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

Fix insertion bug in node_query_ #13

Merged
merged 2 commits into from
Jul 9, 2024
Merged

Conversation

kn1cht
Copy link
Contributor

@kn1cht kn1cht commented Jun 22, 2024

Fix #12

Fixed a bug in KDTree::node_query_ that k_nearest_buffer.insert() might not be executed in certain environments when the next of insert_it is k_nearest_buffer.end().

Copy link
Owner

@crvs crvs left a comment

Choose a reason for hiding this comment

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

Perfect!

KDTree.cpp Outdated
@@ -135,6 +135,7 @@ void KDTree::node_query_(
std::upper_bound(k_nearest_buffer.begin(), k_nearest_buffer.end(),
node_distance, detail::compare_node_distance);
if (insert_it == k_nearest_buffer.end() ||
std::next(insert_it) == k_nearest_buffer.end() ||
insert_it->first != std::next(insert_it)->first) {
Copy link
Owner

Choose a reason for hiding this comment

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

this condition was wrong to begin with, it just doesn't make a difference in GCC for the reason you pointed out in the bug report, instead the insertion should happen when:

  • insert_it is NOT the end(), or,
  • insert_it is the end() pointer, but we still have not filled the buffer with num_nearest elements.

So the final insertion condition should be:
(insert_it != k_nearest_buffer.end() || k_nearest_buffer.size() < num_nearest)

@crvs crvs merged commit 21d5a30 into crvs:master Jul 9, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

KDTree built with MSVC (Microsoft Visual C++) does not achieve 100% accuracy
2 participants