Move NN search back to the VoxelHashMap #388
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tizianoGuadagnino
changed the title
|
I can see us in 3 months going back to the old implementation. I guess we changed this five times already 😛 haha Depending on how I wake up, I believe there is no optimal place for this function. I love or hate this change, so let's take it! This is also a follow-up on #290 (comment) I linked the related PR #290 in the description |
Co-authored-by: Ignacio Vizzo <[email protected]>
nachovizzo
approved these changes
Aug 8, 2024
benemer
approved these changes
Aug 8, 2024
tizianoGuadagnino
added a commit
that referenced
this pull request
Aug 8, 2024
* Now we can search neighbors from the voxel hash * Integrate Nacho's Comment Co-authored-by: Ignacio Vizzo <[email protected]> --------- Co-authored-by: Ignacio Vizzo <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I simply moved the functions related to the Nearest Neighbor search from
RegistrationtoVoxelHashMap, as this is the data structure that is used to compute the neighbors. This allows other people to use our same strategy of NN search without copy-pasting code from an unnamed namespace. I know that this is something we did a long time ago, but I changed my mind, and this makes much more sense, as now you can use our NN search decoupled from the registration module. Of course, performances are completely unaffected, as I can show here.Ps: this change is propedeutic (io non parlo inglese) to a slightly more significant change that will remove a useless copy of points from the voxels during the NN search.
Related PRs
This PR basically shadows what we did in #290 when we refactored a bit the system to support different number of threads in #252