Skip to content

Only skip first result of radiusSearch if results are sorted - #5997

Merged
mvieth merged 1 commit into
PointCloudLibrary:masterfrom
mvieth:radius_search_sorted_results
Apr 9, 2024
Merged

Only skip first result of radiusSearch if results are sorted#5997
mvieth merged 1 commit into
PointCloudLibrary:masterfrom
mvieth:radius_search_sorted_results

Conversation

@mvieth

@mvieth mvieth commented Apr 2, 2024

Copy link
Copy Markdown
Member

The same was done for extractEuclideanClusters years ago: #109
For ConditionalEuclideanClustering, I tested whether it is faster to require sorted results (and skip first entry), or not (and iterate over all results). The second option is much faster (took roughly 2/3 of the time of the first option in my test)

The same was done for `extractEuclideanClusters` years ago: PointCloudLibrary#109
For ConditionalEuclideanClustering, I tested whether it is faster to require sorted results (and skip first entry), or not (and iterate over all results). The second option is much faster (took roughly 2/3 of the time of the first option in my test)
@mvieth mvieth added the changelog: fix Meta-information for changelog generation label Apr 2, 2024
@larshg

larshg commented Apr 8, 2024

Copy link
Copy Markdown
Contributor

Should we add localmaximum filter as well? Ie. its the same problem discussed in #4999?

In #5057 Kunal did skip, if the point is the query point it self for the our_cvfh algorithm? I haven't looked into the algorithm, so I'm not sure if it matters. Just remembered this discussion...

@mvieth

mvieth commented Apr 8, 2024

Copy link
Copy Markdown
Member Author

Should we add localmaximum filter as well? Ie. its the same problem discussed in #4999?

In the localmaximum filter, this is already fixed: #5572

In #5057 Kunal did skip, if the point is the query point it self for the our_cvfh algorithm? I haven't looked into the algorithm, so I'm not sure if it matters. Just remembered this discussion...

Ah right, I kind of forgot about that pull request. For the classes I touched in this PR, it is not necessary to explicitly check whether the point is the query point (if(seed_queue[sq_idx] == nn_indices[j])). The query point comes from seed_queue, and whenever a point is put into seed_queue, it is marked as processed[i] = true. So the query point is marked as already processed. And when looping over the search results, the first check is whether the point has already been processed (if(processed[nn_indices[j]])), skipping over the point if yes. So no additional check necessary

@mvieth
mvieth merged commit fa9d6ad into PointCloudLibrary:master Apr 9, 2024
@mvieth
mvieth deleted the radius_search_sorted_results branch April 9, 2024 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog: fix Meta-information for changelog generation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants