Add header-only cuspatial::join_quadtree_and_bounding_boxes
#861
Conversation
…/header-only-join-quadtree-and-bounding-boxes
trxcllnt
added
3 - Ready for Review
| rmm::device_uvector<uint32_t> out_node_idxs(num_pairs, stream, mr); | ||
| rmm::device_uvector<uint32_t> out_bbox_idxs(num_pairs, stream, mr); | ||
|
|
||
| auto make_current_level_iter = [&]() { |
There was a problem hiding this comment.
Any reason to use a lambda here? It seems we should create the iterator as a variable instead.
There was a problem hiding this comment.
Yes, this lambda closes over variables that are updated each iteration of the loop.
There was a problem hiding this comment.
Do you mean that the iterators gets invalidated after every iteration of the loop?
There was a problem hiding this comment.
make_current_level_iter() returns a zip iterator over the cur_{types,levels,node_idxs,bbox_idxs} vectors, which represent the current level in the tree descent. The descend_quadtree() call walks one level in the tree and returns vectors representing the next "current level."
Similarly, make_output_values_iter() returns a zip iterator of num_results + out_{node,bbox}_idxs. num_results is updated by the detail::find_intersections() call.
The iterators returned by these functions are used both in setting up the initial state of the the top-level quadrants, and in the loop that does the tree traversal. An earlier implementation tracked and updated these two iterators each iteration of the loop, but they were refactored into these lambdas in a cleanup PR.
Codecov ReportBase: 92.55% // Head: 92.58% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## branch-23.02 #861 +/- ##
================================================
+ Coverage 92.55% 92.58% +0.02%
================================================
Files 24 24
Lines 1008 1011 +3
================================================
+ Hits 933 936 +3
Misses 75 75
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
harrism
requested review from
harrism
and removed request for
cwharris and
zhangjianting
There was a problem hiding this comment.
Looks good, thanks Paul! Please update all copyrights in new / modified files to 2023.
Couple of minor comments. There are some places with really long lambdas that you could consider turning into functor classes for tidyness, but I'm not going to press the issue.
cpp/include/cuspatial/experimental/detail/join/intersection.cuh
Outdated
Show resolved
Hide resolved
| rmm::device_uvector<uint32_t> out_node_idxs(num_pairs, stream, mr); | ||
| rmm::device_uvector<uint32_t> out_bbox_idxs(num_pairs, stream, mr); | ||
|
|
||
| auto make_current_level_iter = [&]() { |
There was a problem hiding this comment.
Do you mean that the iterators gets invalidated after every iteration of the loop?
Co-authored-by: Mark Harris <mharris@nvidia.com>
…/header-only-join-quadtree-and-bounding-boxes
trxcllnt
force-pushed
the
fea/header-only-join-quadtree-and-bounding-boxes
branch
from
a7fc335
to
57a7a1e
Compare
|
/merge |
Contributes to #565. Add header-only
cuspatial::join_quadtree_and_bounding_boxes.