Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing erase after remove_if #966

Merged
merged 1 commit into from
Sep 21, 2023
Merged

Conversation

mglisse
Copy link
Member

@mglisse mglisse commented Sep 18, 2023

Fix #964.
I don't know how I managed to miss that... It was in a place where the code was still valid, and the slowdown was limited enough not to make the runtime seem strange. Good thing one compiler decided to warn about it.
It does not seem to impact the result (good), but it does improve performance (something like 20%, depending on the input).

@@ -76,7 +76,16 @@ int main(int argc, char**argv) {
<< " vs metric " << std::chrono::duration_cast<std::chrono::milliseconds>((time_stop2 - time_start2)).count()
<< " (Boost version " << BOOST_VERSION << ")\n";
if (dists != dists2 || results != results2) {
// Note that with many points, it often happens that 2 points with the same distance are swapped in the output.
std::cerr << "Results differ\n";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it happen that results differ ? I was not able to reach this code.

Copy link
Member Author

@mglisse mglisse Sep 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With 100k points and seed 0, I get 1 swap. With 50k points, I need to try several seeds before I get a swap. (I don't remember if this was dimension 2 or 4)

@VincentRouvreau VincentRouvreau added the 3.9.0 GUDHI version 3.9.0 label Sep 21, 2023
@VincentRouvreau VincentRouvreau merged commit 9c5e382 into GUDHI:master Sep 21, 2023
6 checks passed
@mglisse mglisse deleted the remove_if branch March 3, 2024 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.9.0 GUDHI version 3.9.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

remove_if unused result
2 participants