Refactor space filling curves surrounding code #1180
Conversation
If a passed function argument is a prvalue, it will be destroyed once we return, as returning a reference to its member does not extend its lifetime.
| template <typename Box, typename Geometry, | ||
| std::enable_if_t<GeometryTraits::is_box_v<Box> && | ||
| !GeometryTraits::is_point_v<Geometry>> * = nullptr> | ||
| template <typename Box, class Geometry> |
There was a problem hiding this comment.
Do not mix typename and class
| Kokkos::View<LinearOrderingValueType *, typename Values::memory_space> | ||
| linear_ordering_indices( | ||
| Kokkos::view_alloc(space, Kokkos::WithoutInitializing, | ||
| "ArborX::SpaceFillingCurve::linear_ordering"), | ||
| n); |
There was a problem hiding this comment.
I don't think that moving the allocation into that function and returning the Morton codes is a good change
There was a problem hiding this comment.
I don't like it much either. However, our boilerplate code for determining the type of indices is really several lines at every call site.
Alternatively, we could introduce a requirement for a space filling curve to provide index_type that it will return. It would simplify things a lot.
| template <typename ExecutionSpace, typename Values, typename SpaceFillingCurve, | ||
| typename Box> | ||
| inline auto computeSpaceFillingCurvePermutation(ExecutionSpace const &space, | ||
| Values const &values, | ||
| SpaceFillingCurve const &curve, | ||
| Box const &scene_bounding_box) | ||
| { | ||
| auto linear_ordering_indices = | ||
| projectOntoSpaceFillingCurve(space, values, curve, scene_bounding_box); | ||
| return sortObjects(space, linear_ordering_indices); | ||
| } |
There was a problem hiding this comment.
Saves little code to bundle two distinct operations.
I am not convinced it is a good idea to introduce it.
There was a problem hiding this comment.
That's fair. It does a bit more than that in that it also reduces the amount of used memory as linear_ordering_indices are deallocated automatically once this function returns. This behavior is similar to the original sortPredicatesAlongSpaceFillingCurve.
getGeometryfor prvaluesIf a passed function argument is a prvalue, it will be destroyed once we return, as returning a reference to its member does not extend its lifetime.
projectOntoSpaceFillingCurvefromArborX_TreeConstruction.hpptoArborX_SpaceFillingCurve.hppIt will also be used by the tree traversal now
projectOntoSpaceFillingCurveto return linear orderingThis avoids a lot of boilerplate code to determine the type. We currently have zero benefit from passing indices as the last argument.
computeSpaceFillingCurvePermutationthat combinesprojectOntoSpaceFillingCurvewith sorting to further reduce the amount of codePredicateIndexablesto easily iterate over predicate geometries