Merge pull request #915 from aprokop/9-constructor_values

Finalize BasicBoundingVolumeHierarchy

benchmarks/brute_force_vs_bvh/brute_force_vs_bvh_timpl.hpp

@@ -83,6 +83,7 @@ static void run_fp(int nprimitives, int nqueries, int nrepeats)
   Placeholder<DIM, FloatingPoint> primitives{nprimitives};
   Placeholder<DIM, FloatingPoint> predicates{nqueries};
 
+  using Point = ArborX::ExperimentalHyperGeometry::Point<DIM, FloatingPoint>;
   using Box = ArborX::ExperimentalHyperGeometry::Box<DIM, FloatingPoint>;
 
   for (int i = 0; i < nrepeats; i++)
@@ -91,8 +92,9 @@ static void run_fp(int nprimitives, int nqueries, int nrepeats)
     {
       Kokkos::Timer timer;
       ArborX::BasicBoundingVolumeHierarchy<
-          MemorySpace, ArborX::Details::PairIndexVolume<Box>>
-          bvh{space, primitives};
+          MemorySpace, ArborX::Details::PairIndexVolume<Point>>
+          bvh{space, ArborX::Details::LegacyValues<decltype(primitives), Point>{
+                         primitives}};
 
       Kokkos::View<int *, ExecutionSpace> indices("Benchmark::indices_ref", 0);
       Kokkos::View<int *, ExecutionSpace> offset("Benchmark::offset_ref", 0);

benchmarks/dbscan/ArborX_DBSCANVerification.hpp

@@ -307,12 +307,14 @@ bool verifyDBSCAN(ExecutionSpace exec_space, Primitives const &primitives,
   ARBORX_ASSERT(eps > 0);
   ARBORX_ASSERT(core_min_size >= 2);
 
-  constexpr int dim = GeometryTraits::dimension_v<
-      typename Details::AccessTraitsHelper<Access>::type>;
+  using Point = typename Details::AccessTraitsHelper<Access>::type;
+  static_assert(GeometryTraits::is_point<Point>{});
+  constexpr int dim = GeometryTraits::dimension_v<Point>;
   using Box = ExperimentalHyperGeometry::Box<dim>;
   ArborX::BasicBoundingVolumeHierarchy<MemorySpace,
                                        ArborX::Details::PairIndexVolume<Box>>
-      bvh(exec_space, primitives);
+      bvh(exec_space,
+          ArborX::Details::LegacyValues<Primitives, Box>{primitives});
 
   auto const predicates =
       Details::PrimitivesWithRadius<Primitives>{primitives, eps};

examples/triangle_intersection/triangle_intersection.cpp

@@ -38,6 +38,7 @@ constexpr float hx = Lx / (nx - 1);
 constexpr float hy = Ly / (ny - 1);
 
 using Point = ArborX::ExperimentalHyperGeometry::Point<2>;
+using Box = ArborX::ExperimentalHyperGeometry::Box<2>;
 using Triangle = ArborX::ExperimentalHyperGeometry::Triangle<2>;
 
 #ifdef PRECOMPUTE_MAPPING
@@ -321,9 +322,9 @@ int main()
 
   // Create BVH tree
   ArborX::BasicBoundingVolumeHierarchy<
-      MemorySpace, ArborX::Details::PairIndexVolume<
-                       ArborX::ExperimentalHyperGeometry::Box<2>>> const
-      tree(execution_space, triangles);
+      MemorySpace, ArborX::Details::PairIndexVolume<Box>> const
+      tree(execution_space,
+           ArborX::Details::LegacyValues<decltype(triangles), Box>{triangles});
 
   // Create the points used for queries
   Points<MemorySpace> points(execution_space);

src/ArborX_BruteForce.hpp

@@ -91,7 +91,9 @@ BruteForce<MemorySpace, BoundingVolume>::BruteForce(
 {
   static_assert(
       KokkosExt::is_accessible_from<MemorySpace, ExecutionSpace>::value);
-  Details::check_valid_access_traits(PrimitivesTag{}, primitives);
+  // FIXME for now, do not check the return type of get()
+  Details::check_valid_access_traits<Primitives>(
+      PrimitivesTag{}, primitives, Details::DoNotCheckGetReturnType());
   using Access = AccessTraits<Primitives, PrimitivesTag>;
   static_assert(KokkosExt::is_accessible_from<typename Access::memory_space,
                                               ExecutionSpace>::value,

src/ArborX_DBSCAN.hpp

@@ -60,15 +60,14 @@ struct WithinRadiusGetter
 {
   float _r;
 
-  template <typename Box>
-  KOKKOS_FUNCTION auto operator()(Box const &box) const
+  template <typename Point>
+  KOKKOS_FUNCTION auto operator()(Point const &point) const
   {
-    static_assert(GeometryTraits::is_box<Box>::value);
+    static_assert(GeometryTraits::is_point<Point>::value);
 
-    constexpr int dim = GeometryTraits::dimension_v<Box>;
+    constexpr int dim = GeometryTraits::dimension_v<Point>;
     auto const &hyper_point =
-        reinterpret_cast<ExperimentalHyperGeometry::Point<dim> const &>(
-            box.minCorner());
+        reinterpret_cast<ExperimentalHyperGeometry::Point<dim> const &>(point);
     using ArborX::intersects;
     return intersects(ExperimentalHyperGeometry::Sphere<dim>{hyper_point, _r});
   }
@@ -98,6 +97,22 @@ struct MixedBoxPrimitives
   Permutation _permute;
 };
 
+template <typename Primitives>
+struct PrimitivesIndexables
+{
+  Primitives _primitives;
+
+  using Access = AccessTraits<Primitives, PrimitivesTag>;
+  using memory_space = typename Access::memory_space;
+
+  KOKKOS_FUNCTION decltype(auto) operator()(int i) const
+  {
+    return Access::get(_primitives, i);
+  }
+
+  KOKKOS_FUNCTION auto size() const { return Access::size(_primitives); }
+};
+
 } // namespace Details
 
 template <typename Primitives>
@@ -266,8 +281,9 @@ dbscan(ExecutionSpace const &exec_space, Primitives const &primitives,
   using UnionFind = Details::UnionFind<MemorySpace>;
 #endif
 
-  constexpr int dim = GeometryTraits::dimension_v<
-      typename Details::AccessTraitsHelper<Access>::type>;
+  using Point = typename Details::AccessTraitsHelper<Access>::type;
+  static_assert(GeometryTraits::is_point<Point>{});
+  constexpr int dim = GeometryTraits::dimension_v<Point>;
   using Box = ExperimentalHyperGeometry::Box<dim>;
 
   bool const is_special_case = (core_min_size == 2);
@@ -290,8 +306,8 @@ dbscan(ExecutionSpace const &exec_space, Primitives const &primitives,
     // Build the tree
     Kokkos::Profiling::pushRegion("ArborX::DBSCAN::tree_construction");
     ArborX::BasicBoundingVolumeHierarchy<MemorySpace,
-                                         Details::PairIndexVolume<Box>>
-        bvh(exec_space, primitives);
+                                         Details::PairIndexVolume<Point>>
+        bvh(exec_space, Details::LegacyValues<Primitives, Point>{primitives});
     Kokkos::Profiling::popRegion();
 
     Kokkos::Profiling::pushRegion("ArborX::DBSCAN::clusters");
@@ -352,7 +368,8 @@ dbscan(ExecutionSpace const &exec_space, Primitives const &primitives,
     Kokkos::Profiling::pushRegion("ArborX::DBSCAN::dense_cells");
     Box bounds;
     Details::TreeConstruction::calculateBoundingBoxOfTheScene(
-        exec_space, Details::Indexables<Primitives>{primitives}, bounds);
+        exec_space, Details::PrimitivesIndexables<Primitives>{primitives},
+        bounds);
 
     // The cell length is chosen to be eps/sqrt(dimension), so that any two
     // points within the same cell are within eps distance of each other.
@@ -411,14 +428,16 @@ dbscan(ExecutionSpace const &exec_space, Primitives const &primitives,
 
     // Build the tree
     Kokkos::Profiling::pushRegion("ArborX::DBSCAN::tree_construction");
+    Details::MixedBoxPrimitives<Primitives, decltype(dense_cell_offsets),
+                                decltype(cell_indices), decltype(permute)>
+        mixed_primitives{primitives,          grid,
+                         dense_cell_offsets,  num_points_in_dense_cells,
+                         sorted_cell_indices, permute};
+
     ArborX::BasicBoundingVolumeHierarchy<MemorySpace,
                                          Details::PairIndexVolume<Box>>
-        bvh(exec_space,
-            Details::MixedBoxPrimitives<
-                Primitives, decltype(dense_cell_offsets),
-                decltype(cell_indices), decltype(permute)>{
-                primitives, grid, dense_cell_offsets, num_points_in_dense_cells,
-                sorted_cell_indices, permute});
+        bvh(exec_space, Details::LegacyValues<decltype(mixed_primitives), Box>{
+                            mixed_primitives});
 
     Kokkos::Profiling::popRegion();
 

src/ArborX_LinearBVH.hpp

@@ -65,10 +65,11 @@ class BasicBoundingVolumeHierarchy
 
   BasicBoundingVolumeHierarchy() = default; // build an empty tree
 
-  template <typename ExecutionSpace, typename Primitives,
+  template <typename ExecutionSpace, typename Values,
             typename SpaceFillingCurve = Experimental::Morton64>
   BasicBoundingVolumeHierarchy(
-      ExecutionSpace const &space, Primitives const &primitives,
+      ExecutionSpace const &space, Values const &values,
+      IndexableGetter const &indexable_getter = IndexableGetter(),
       SpaceFillingCurve const &curve = SpaceFillingCurve());
 
   KOKKOS_FUNCTION
@@ -147,14 +148,22 @@ class BoundingVolumeHierarchy
 public:
   using legacy_tree = void;
 
+  using bounding_volume_type = typename base_type::bounding_volume_type;
+
   BoundingVolumeHierarchy() = default; // build an empty tree
 
   template <typename ExecutionSpace, typename Primitives,
             typename SpaceFillingCurve = Experimental::Morton64>
   BoundingVolumeHierarchy(ExecutionSpace const &space,
                           Primitives const &primitives,
                           SpaceFillingCurve const &curve = SpaceFillingCurve())
-      : base_type(space, primitives, curve)
+      : base_type(
+            space,
+            // Validate the primitives before calling the base constructor
+            (Details::check_valid_access_traits(PrimitivesTag{}, primitives),
+             Details::LegacyValues<Primitives, bounding_volume_type>{
+                 primitives}),
+            Details::DefaultIndexableGetter(), curve)
   {}
 
   template <typename ExecutionSpace, typename Predicates, typename Callback>
@@ -194,28 +203,32 @@ using BVH = BoundingVolumeHierarchy<MemorySpace>;
 
 template <typename MemorySpace, typename Value, typename IndexableGetter,
           typename BoundingVolume>
-template <typename ExecutionSpace, typename Primitives,
-          typename SpaceFillingCurve>
+template <typename ExecutionSpace, typename Values, typename SpaceFillingCurve>
 BasicBoundingVolumeHierarchy<MemorySpace, Value, IndexableGetter,
                              BoundingVolume>::
     BasicBoundingVolumeHierarchy(ExecutionSpace const &space,
-                                 Primitives const &primitives,
+                                 Values const &user_values,
+                                 IndexableGetter const &indexable_getter,
                                  SpaceFillingCurve const &curve)
-    : _size(AccessTraits<Primitives, PrimitivesTag>::size(primitives))
+    : _size(AccessTraits<Values, PrimitivesTag>::size(user_values))
     , _leaf_nodes(Kokkos::view_alloc(space, Kokkos::WithoutInitializing,
                                      "ArborX::BVH::leaf_nodes"),
                   _size)
     , _internal_nodes(Kokkos::view_alloc(space, Kokkos::WithoutInitializing,
                                          "ArborX::BVH::internal_nodes"),
                       _size > 1 ? _size - 1 : 0)
+    , _indexable_getter(indexable_getter)
 {
   static_assert(
       KokkosExt::is_accessible_from<MemorySpace, ExecutionSpace>::value);
-  Details::check_valid_access_traits(PrimitivesTag{}, primitives);
-  using Access = AccessTraits<Primitives, PrimitivesTag>;
+  // FIXME redo with RangeTraits
+  Details::check_valid_access_traits<Values>(
+      PrimitivesTag{}, user_values, Details::DoNotCheckGetReturnType());
+  using Access = AccessTraits<Values, PrimitivesTag>;
   static_assert(KokkosExt::is_accessible_from<typename Access::memory_space,
                                               ExecutionSpace>::value,
-                "Primitives must be accessible from the execution space");
+                "Values must be accessible from the execution space");
+
   constexpr int DIM = GeometryTraits::dimension_v<BoundingVolume>;
 
   Details::check_valid_space_filling_curve<DIM>(curve);
@@ -227,39 +240,40 @@ BasicBoundingVolumeHierarchy<MemorySpace, Value, IndexableGetter,
     return;
   }
 
+  Details::AccessValues<Values> values{user_values};
+
   if (size() == 1)
   {
     Details::TreeConstruction::initializeSingleLeafTree(
-        space, Details::LegacyValues<Primitives, indexable_type>{primitives},
-        _indexable_getter, _leaf_nodes, _bounds);
+        space, values, _indexable_getter, _leaf_nodes, _bounds);
     return;
   }
 
+  Details::Indexables<decltype(values), IndexableGetter> indexables{
+      values, indexable_getter};
+
   Kokkos::Profiling::pushRegion(
       "ArborX::BVH::BVH::calculate_scene_bounding_box");
 
   // determine the bounding box of the scene
   ExperimentalHyperGeometry::Box<
       DIM, typename GeometryTraits::coordinate_type<BoundingVolume>::type>
       bbox{};
-  Details::TreeConstruction::calculateBoundingBoxOfTheScene(
-      space, Details::Indexables<Primitives>{primitives}, bbox);
-
+  Details::TreeConstruction::calculateBoundingBoxOfTheScene(space, indexables,
+                                                            bbox);
   Kokkos::Profiling::popRegion();
   Kokkos::Profiling::pushRegion("ArborX::BVH::BVH::compute_linear_ordering");
 
   // Map indexables from multidimensional domain to one-dimensional interval
   using LinearOrderingValueType = Kokkos::detected_t<
       Details::SpaceFillingCurveProjectionArchetypeExpression,
-      SpaceFillingCurve, decltype(bbox),
-      std::decay_t<decltype(Access::get(primitives, 0))>>;
+      SpaceFillingCurve, decltype(bbox), indexable_type>;
   Kokkos::View<LinearOrderingValueType *, MemorySpace> linear_ordering_indices(
       Kokkos::view_alloc(space, Kokkos::WithoutInitializing,
                          "ArborX::BVH::BVH::linear_ordering"),
       size());
   Details::TreeConstruction::projectOntoSpaceFillingCurve(
-      space, Details::Indexables<Primitives>{primitives}, curve, bbox,
-      linear_ordering_indices);
+      space, indexables, curve, bbox, linear_ordering_indices);
 
   Kokkos::Profiling::popRegion();
   Kokkos::Profiling::pushRegion("ArborX::BVH::BVH::sort_linearized_order");
@@ -273,9 +287,8 @@ BasicBoundingVolumeHierarchy<MemorySpace, Value, IndexableGetter,
 
   // Generate bounding volume hierarchy
   Details::TreeConstruction::generateHierarchy(
-      space, Details::LegacyValues<Primitives, indexable_type>{primitives},
-      _indexable_getter, permutation_indices, linear_ordering_indices,
-      _leaf_nodes, _internal_nodes, _bounds);
+      space, values, _indexable_getter, permutation_indices,
+      linear_ordering_indices, _leaf_nodes, _internal_nodes, _bounds);
 
   Kokkos::Profiling::popRegion();
 }

src/details/ArborX_AccessTraits.hpp

@@ -153,8 +153,12 @@ void check_valid_access_traits(PredicatesTag, Predicates const &)
                 "Invalid tag for the predicates");
 }
 
-template <typename Primitives>
-void check_valid_access_traits(PrimitivesTag, Primitives const &)
+struct DoNotCheckGetReturnType : std::false_type
+{};
+
+template <typename Primitives, typename CheckGetReturnType = std::true_type>
+void check_valid_access_traits(PrimitivesTag, Primitives const &,
+                               CheckGetReturnType = {})
 {
   using Access = AccessTraits<Primitives, PrimitivesTag>;
   static_assert(
@@ -187,11 +191,32 @@ void check_valid_access_traits(PrimitivesTag, Primitives const &)
       "member function");
   using T = std::decay_t<Kokkos::detected_t<AccessTraitsGetArchetypeExpression,
                                             Access, Primitives>>;
-  static_assert(GeometryTraits::is_point<T>{} || GeometryTraits::is_box<T>{},
-                "AccessTraits<Primitives,PrimitivesTag>::get() return type "
-                "must decay to a point or a box type");
+  if constexpr (CheckGetReturnType())
+  {
+    static_assert(GeometryTraits::is_point<T>{} || GeometryTraits::is_box<T>{},
+                  "AccessTraits<Primitives,PrimitivesTag>::get() return type "
+                  "must decay to a point or a box type");
+  }
 }
 
+template <typename Values>
+class AccessValues
+{
+private:
+  using Access = AccessTraits<Values, PrimitivesTag>;
+
+public:
+  Values _values;
+
+  using memory_space = typename Access::memory_space;
+
+  KOKKOS_FUNCTION
+  decltype(auto) operator()(int i) const { return Access::get(_values, i); }
+
+  KOKKOS_FUNCTION
+  auto size() const { return Access::size(_values); }
+};
+
 } // namespace Details
 
 namespace Traits

src/details/ArborX_DetailsLegacy.hpp

@@ -36,7 +36,7 @@ class LegacyValues
   {}
 
   KOKKOS_FUNCTION
-  decltype(auto) operator()(size_type i) const
+  auto operator()(size_type i) const
   {
     if constexpr (std::is_same_v<BoundingVolume,
                                  typename AccessTraitsHelper<Access>::type>)
@@ -70,4 +70,25 @@ struct LegacyCallbackWrapper
 
 } // namespace ArborX::Details
 
+template <typename Primitives, typename BoundingVolume>
+struct ArborX::AccessTraits<
+    ArborX::Details::LegacyValues<Primitives, BoundingVolume>,
+    ArborX::PrimitivesTag>
+{
+  using Values = ArborX::Details::LegacyValues<Primitives, BoundingVolume>;
+
+  using memory_space = typename Values::memory_space;
+  using size_type = typename Values::size_type;
+  using value_type = typename Values::value_type;
+
+  KOKKOS_FUNCTION static size_type size(Values const &values)
+  {
+    return values.size();
+  }
+  KOKKOS_FUNCTION static decltype(auto) get(Values const &values, size_type i)
+  {
+    return values(i);
+  }
+};
+
 #endif