Introduce PairValueIndex and AttachIndices

There are two use cases that are currently mixed when using LegacyValues: 1) Legacy classes use LegacyValues to pass to the new interface 2) User wants to attach indices to the passed primitives without constructing them themselves The difference is that 1) should not be exposed to a user, while 2) is intended to. With the introduction of the RangeTraits in the future, the use cases diverge: 1) will still use AccessTraits for backwards compatibility, while 2) will switch to using RangeTraits. This patch separates the two. They still share a lot of commonalities at the moment, but that will change with RangeTraits. In addition, the order is swapped between value and index in the struct. The reason being is that it is easer to align an index (likely 4-byte int) than a value (which could be a 12-byte point, 24-byte box, or whatever).
arborx · Dec 7, 2023 · 615c70b · 615c70b
1 parent 5a8d82e
commit 615c70b
Show file tree

Hide file tree

Showing 14 changed files with 114 additions and 56 deletions.
diff --git a/benchmarks/brute_force_vs_bvh/brute_force_vs_bvh_timpl.hpp b/benchmarks/brute_force_vs_bvh/brute_force_vs_bvh_timpl.hpp
@@ -90,9 +90,8 @@ static void run_fp(int nprimitives, int nqueries, int nrepeats)
     {
       Kokkos::Timer timer;
       ArborX::BasicBoundingVolumeHierarchy<
-          MemorySpace, ArborX::Details::PairIndexVolume<Point>>
-          bvh{space, ArborX::Details::LegacyValues<decltype(primitives), Point>{
-                         primitives}};
+          MemorySpace, ArborX::Details::PairValueIndex<Point>>
+          bvh{space, ArborX::Details::AttachIndices{primitives}};
 
       Kokkos::View<int *, ExecutionSpace> indices("Benchmark::indices_ref", 0);
       Kokkos::View<int *, ExecutionSpace> offset("Benchmark::offset_ref", 0);
@@ -111,10 +110,8 @@ static void run_fp(int nprimitives, int nqueries, int nrepeats)
     {
       Kokkos::Timer timer;
       ArborX::BasicBruteForce<MemorySpace,
-                              ArborX::Details::PairIndexVolume<Point>>
-          brute{space,
-                ArborX::Details::LegacyValues<decltype(primitives), Point>{
-                    primitives}};
+                              ArborX::Details::PairValueIndex<Point>>
+          brute{space, ArborX::Details::AttachIndices{primitives}};
 
       Kokkos::View<int *, ExecutionSpace> indices("Benchmark::indices", 0);
       Kokkos::View<int *, ExecutionSpace> offset("Benchmark::offset", 0);

diff --git a/benchmarks/dbscan/ArborX_DBSCANVerification.hpp b/benchmarks/dbscan/ArborX_DBSCANVerification.hpp
@@ -13,7 +13,6 @@
 #define ARBORX_DETAILSDBSCANVERIFICATION_HPP
 
 #include <ArborX_DetailsUtils.hpp>
-#include <ArborX_HyperBox.hpp>
 #include <ArborX_LinearBVH.hpp>
 
 #include <Kokkos_Core.hpp>
@@ -313,8 +312,8 @@ bool verifyDBSCAN(ExecutionSpace exec_space, Primitives const &primitives,
   static_assert(GeometryTraits::is_point<Point>{});
 
   ArborX::BasicBoundingVolumeHierarchy<MemorySpace,
-                                       ArborX::Details::PairIndexVolume<Point>>
-      bvh(exec_space, ArborX::Details::LegacyValues<Points, Point>{points});
+                                       ArborX::Details::PairValueIndex<Point>>
+      bvh(exec_space, ArborX::Details::AttachIndices{points});
 
   auto const predicates = Details::PrimitivesWithRadius<Points>{points, eps};
 

diff --git a/examples/triangle_intersection/triangle_intersection.cpp b/examples/triangle_intersection/triangle_intersection.cpp
@@ -322,9 +322,9 @@ int main()
 
   // Create BVH tree
   ArborX::BasicBoundingVolumeHierarchy<
-      MemorySpace, ArborX::Details::PairIndexVolume<Box>> const
+      MemorySpace, ArborX::Details::PairValueIndex<Box>> const
       tree(execution_space,
-           ArborX::Details::LegacyValues<decltype(triangles), Box>{triangles});
+           ArborX::Details::AttachIndices<decltype(triangles)>{triangles});
 
   // Create the points used for queries
   Points<MemorySpace> points(execution_space);

diff --git a/src/ArborX_BruteForce.hpp b/src/ArborX_BruteForce.hpp
@@ -93,11 +93,11 @@ class BasicBruteForce
 
 template <typename MemorySpace, typename BoundingVolume = Box>
 class BruteForce
-    : public BasicBruteForce<MemorySpace, Details::PairIndexVolume<Box>,
+    : public BasicBruteForce<MemorySpace, Details::PairValueIndex<Box>,
                              Details::DefaultIndexableGetter, BoundingVolume>
 {
   using base_type =
-      BasicBruteForce<MemorySpace, Details::PairIndexVolume<Box>,
+      BasicBruteForce<MemorySpace, Details::PairValueIndex<Box>,
                       Details::DefaultIndexableGetter, BoundingVolume>;
 
 public:

diff --git a/src/ArborX_DBSCAN.hpp b/src/ArborX_DBSCAN.hpp
@@ -283,8 +283,8 @@ dbscan(ExecutionSpace const &exec_space, Primitives const &primitives,
     // Build the tree
     Kokkos::Profiling::pushRegion("ArborX::DBSCAN::tree_construction");
     ArborX::BasicBoundingVolumeHierarchy<MemorySpace,
-                                         Details::PairIndexVolume<Point>>
-        bvh(exec_space, Details::LegacyValues<Points, Point>{points});
+                                         Details::PairValueIndex<Point>>
+        bvh(exec_space, Details::AttachIndices{points});
     Kokkos::Profiling::popRegion();
 
     Kokkos::Profiling::pushRegion("ArborX::DBSCAN::clusters");
@@ -416,9 +416,8 @@ dbscan(ExecutionSpace const &exec_space, Primitives const &primitives,
                          permute};
 
     ArborX::BasicBoundingVolumeHierarchy<MemorySpace,
-                                         Details::PairIndexVolume<Box>>
-        bvh(exec_space, Details::LegacyValues<decltype(mixed_primitives), Box>{
-                            mixed_primitives});
+                                         Details::PairValueIndex<Box>>
+        bvh(exec_space, Details::AttachIndices{mixed_primitives});
 
     Kokkos::Profiling::popRegion();
 

diff --git a/src/ArborX_LinearBVH.hpp b/src/ArborX_LinearBVH.hpp
@@ -138,11 +138,11 @@ class BasicBoundingVolumeHierarchy
 template <typename MemorySpace>
 class BoundingVolumeHierarchy
     : public BasicBoundingVolumeHierarchy<MemorySpace,
-                                          Details::PairIndexVolume<Box>,
+                                          Details::PairValueIndex<Box>,
                                           Details::DefaultIndexableGetter, Box>
 {
   using base_type =
-      BasicBoundingVolumeHierarchy<MemorySpace, Details::PairIndexVolume<Box>,
+      BasicBoundingVolumeHierarchy<MemorySpace, Details::PairValueIndex<Box>,
                                    Details::DefaultIndexableGetter, Box>;
 
 public:

diff --git a/src/details/ArborX_DetailsLegacy.hpp b/src/details/ArborX_DetailsLegacy.hpp
@@ -13,7 +13,7 @@
 #define ARBORX_DETAILS_LEGACY_HPP
 
 #include <ArborX_AccessTraits.hpp>
-#include <ArborX_DetailsNode.hpp>
+#include <ArborX_PairValueIndex.hpp>
 
 namespace ArborX::Details
 {
@@ -26,7 +26,7 @@ class LegacyValues
 
 public:
   using memory_space = typename Access::memory_space;
-  using value_type = Details::PairIndexVolume<BoundingVolume>;
+  using value_type = Details::PairValueIndex<BoundingVolume>;
   using size_type =
       Kokkos::detected_t<Details::AccessTraitsSizeArchetypeExpression, Access,
                          Primitives>;
@@ -41,13 +41,13 @@ class LegacyValues
     if constexpr (std::is_same_v<BoundingVolume,
                                  typename AccessTraitsHelper<Access>::type>)
     {
-      return value_type{(unsigned)i, Access::get(_primitives, i)};
+      return value_type{Access::get(_primitives, i), (unsigned)i};
     }
     else
     {
       BoundingVolume bounding_volume{};
       expand(bounding_volume, Access::get(_primitives, i));
-      return value_type{(unsigned)i, bounding_volume};
+      return value_type{bounding_volume, (unsigned)i};
     }
   }
 
@@ -60,16 +60,16 @@ struct LegacyCallbackWrapper
 {
   Callback _callback;
 
-  template <typename Predicate, typename Geometry>
+  template <typename Predicate, typename Value>
   KOKKOS_FUNCTION auto operator()(Predicate const &predicate,
-                                  PairIndexVolume<Geometry> const &value) const
+                                  PairValueIndex<Value> const &value) const
   {
     return _callback(predicate, value.index);
   }
 
-  template <typename Predicate, typename Geometry, typename Output>
+  template <typename Predicate, typename Value, typename Output>
   KOKKOS_FUNCTION auto operator()(Predicate const &predicate,
-                                  PairIndexVolume<Geometry> const &value,
+                                  PairValueIndex<Value> const &value,
                                   Output const &out) const
   {
     return _callback(predicate, value.index, out);
@@ -78,9 +78,9 @@ struct LegacyCallbackWrapper
 
 struct LegacyDefaultCallback
 {
-  template <typename Query, typename Geometry, typename OutputFunctor>
+  template <typename Query, typename Value, typename OutputFunctor>
   KOKKOS_FUNCTION void operator()(Query const &,
-                                  PairIndexVolume<Geometry> const &value,
+                                  PairValueIndex<Value> const &value,
                                   OutputFunctor const &output) const
   {
     output(value.index);

diff --git a/src/details/ArborX_DetailsNode.hpp b/src/details/ArborX_DetailsNode.hpp
@@ -23,13 +23,6 @@ namespace ArborX::Details
 
 constexpr int ROPE_SENTINEL = -1;
 
-template <class BoundingVolume>
-struct PairIndexVolume
-{
-  unsigned index;
-  BoundingVolume bounding_volume;
-};
-
 template <class Value>
 struct LeafNode
 {

diff --git a/src/details/ArborX_IndexableGetter.hpp b/src/details/ArborX_IndexableGetter.hpp
@@ -13,8 +13,8 @@
 #define ARBORX_INDEXABLE_GETTER_HPP
 
 #include <ArborX_AccessTraits.hpp>
-#include <ArborX_DetailsNode.hpp> // PairIndexVolume
 #include <ArborX_GeometryTraits.hpp>
+#include <ArborX_PairValueIndex.hpp>
 
 namespace ArborX::Details
 {
@@ -32,17 +32,17 @@ struct DefaultIndexableGetter
     return geometry;
   }
 
-  template <typename Geometry>
-  KOKKOS_FUNCTION Geometry const &
-  operator()(PairIndexVolume<Geometry> const &pair) const
+  template <typename Value>
+  KOKKOS_FUNCTION Value const &
+  operator()(PairValueIndex<Value> const &pair) const
   {
-    return pair.bounding_volume;
+    return pair.value;
   }
 
-  template <typename Geometry>
-  KOKKOS_FUNCTION Geometry operator()(PairIndexVolume<Geometry> &&pair) const
+  template <typename Value>
+  KOKKOS_FUNCTION Value operator()(PairValueIndex<Value> &&pair) const
   {
-    return pair.bounding_volume;
+    return pair.value;
   }
 };
 

diff --git a/src/details/ArborX_MinimumSpanningTree.hpp b/src/details/ArborX_MinimumSpanningTree.hpp
@@ -55,8 +55,8 @@ struct MinimumSpanningTree
     auto const n = points.size();
 
     Kokkos::Profiling::pushRegion("ArborX::MST::construction");
-    BasicBoundingVolumeHierarchy<MemorySpace, PairIndexVolume<Point>> bvh(
-        space, Details::LegacyValues<Points, Point>{points});
+    BasicBoundingVolumeHierarchy<MemorySpace, PairValueIndex<Point>> bvh(
+        space, Details::AttachIndices{points});
     Kokkos::Profiling::popRegion();
 
     if (k > 1)

diff --git a/src/details/ArborX_PairValueIndex.hpp b/src/details/ArborX_PairValueIndex.hpp
@@ -0,0 +1,72 @@
+/****************************************************************************
+ * Copyright (c) 2023 by the ArborX authors                                 *
+ * All rights reserved.                                                     *
+ *                                                                          *
+ * This file is part of the ArborX library. ArborX is                       *
+ * distributed under a BSD 3-clause license. For the licensing terms see    *
+ * the LICENSE file in the top-level directory.                             *
+ *                                                                          *
+ * SPDX-License-Identifier: BSD-3-Clause                                    *
+ ****************************************************************************/
+
+#ifndef ARBORX_PAIR_VALUE_INDEX_HPP
+#define ARBORX_PAIR_VALUE_INDEX_HPP
+
+#include <ArborX_AccessTraits.hpp>
+
+#include <Kokkos_Macros.hpp>
+
+namespace ArborX::Details
+{
+
+template <class Value, typename Index = unsigned>
+struct PairValueIndex
+{
+  static_assert(std::is_integral_v<Index>);
+
+  Value value;
+  Index index;
+};
+
+template <typename Values, typename Index = unsigned>
+class AttachIndices
+{
+private:
+  using Data = AccessValues<Values>;
+
+public:
+  Data _data;
+
+  using memory_space = typename Data::memory_space;
+  using value_type = Details::PairValueIndex<typename Data::value_type>;
+
+  AttachIndices(Values const &values)
+      : _data{values}
+  {}
+
+  KOKKOS_FUNCTION
+  auto operator()(int i) const { return value_type{_data(i), Index(i)}; }
+
+  KOKKOS_FUNCTION
+  auto size() const { return _data.size(); }
+};
+
+} // namespace ArborX::Details
+
+template <typename Values, typename Index>
+struct ArborX::AccessTraits<ArborX::Details::AttachIndices<Values, Index>,
+                            ArborX::PrimitivesTag>
+{
+  using Self = ArborX::Details::AttachIndices<Values, Index>;
+
+  using memory_space = typename Self::memory_space;
+  using value_type = typename Self::value_type;
+
+  KOKKOS_FUNCTION static auto size(Self const &values) { return values.size(); }
+  KOKKOS_FUNCTION static decltype(auto) get(Self const &values, int i)
+  {
+    return values(i);
+  }
+};
+
+#endif
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
@@ -76,7 +76,7 @@ foreach(_test Callbacks Degenerate ManufacturedSolution ComparisonWithBoost)
     "template <class MemorySpace>\n"
     "using ArborX_Legacy_BasicBVH_Box =\n"
     "    LegacyTree<ArborX::BasicBoundingVolumeHierarchy<\n"
-    "        MemorySpace, ArborX::Details::PairIndexVolume<ArborX::Box>,\n"
+    "        MemorySpace, ArborX::Details::PairValueIndex<ArborX::Box>,\n"
     "        ArborX::Details::DefaultIndexableGetter, ArborX::Box>>;\n"
     "#define ARBORX_TEST_TREE_TYPES Tuple<ArborX::BVH, ArborX_Legacy_BasicBVH_Box>\n"
     "#define ARBORX_TEST_DEVICE_TYPES std::tuple<${ARBORX_DEVICE_TYPES}>\n"
@@ -96,7 +96,7 @@ foreach(_test Callbacks Degenerate ManufacturedSolution ComparisonWithBoost)
     "template <class MemorySpace>\n"
     "using ArborX_Legacy_BasicBruteForce_Box =\n"
     "    LegacyTree<ArborX::BasicBruteForce<\n"
-    "        MemorySpace, ArborX::Details::PairIndexVolume<ArborX::Box>,\n"
+    "        MemorySpace, ArborX::Details::PairValueIndex<ArborX::Box>,\n"
     "        ArborX::Details::DefaultIndexableGetter, ArborX::Box>>;\n"
     "#define ARBORX_TEST_TREE_TYPES Tuple<ArborX_BruteForce_Box, ArborX_Legacy_BasicBruteForce_Box>\n"
     "#define ARBORX_TEST_DEVICE_TYPES std::tuple<${ARBORX_DEVICE_TYPES}>\n"
@@ -121,7 +121,7 @@ foreach(_test Callbacks Degenerate ManufacturedSolution ComparisonWithBoost)
       "template <class MemorySpace>\n"
       "using ArborX_Legacy_BasicBVH_${_bounding_volume} =\n"
       "    LegacyTree<ArborX::BasicBoundingVolumeHierarchy<\n"
-      "        MemorySpace, ArborX::Details::PairIndexVolume<${_bounding_volume}>,\n"
+      "        MemorySpace, ArborX::Details::PairValueIndex<${_bounding_volume}>,\n"
       "        ArborX::Details::DefaultIndexableGetter, ${_bounding_volume}>>;\n"
       "#define ARBORX_TEST_TREE_TYPES Tuple<ArborX_Legacy_BasicBVH_${_bounding_volume}>\n"
       "#define ARBORX_TEST_DEVICE_TYPES std::tuple<${ARBORX_DEVICE_TYPES}>\n"

diff --git a/test/tstDetailsMutualReachabilityDistance.cpp b/test/tstDetailsMutualReachabilityDistance.cpp
@@ -33,10 +33,8 @@ auto compute_core_distances(ExecutionSpace exec_space,
   ARBORX_ASSERT(points.extent_int(0) >= k);
   using MemorySpace = typename ExecutionSpace::memory_space;
   ArborX::BasicBoundingVolumeHierarchy<
-      MemorySpace, ArborX::Details::PairIndexVolume<ArborX::Point>>
-      bvh{exec_space,
-          ArborX::Details::LegacyValues<decltype(points), ArborX::Point>{
-              points}};
+      MemorySpace, ArborX::Details::PairValueIndex<ArborX::Point>>
+      bvh{exec_space, ArborX::Details::AttachIndices{points}};
   Kokkos::View<float *, MemorySpace> distances(
       Kokkos::view_alloc(Kokkos::WithoutInitializing, "Test::core_distances"),
       bvh.size());

diff --git a/test/tstDetailsTreeConstruction.cpp b/test/tstDetailsTreeConstruction.cpp
@@ -250,7 +250,7 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(example_tree_construction, DeviceType,
   BOOST_TEST_MESSAGE("ref = " << ref.str());
 
   using LeafNode = ArborX::Details::LeafNode<
-      ArborX::Details::PairIndexVolume<Test::FakeBoundingVolume>>;
+      ArborX::Details::PairValueIndex<Test::FakeBoundingVolume>>;
   using InternalNode = ArborX::Details::InternalNode<Test::FakeBoundingVolume>;
 
   Kokkos::View<LeafNode *, DeviceType> leaf_nodes("Testing::leaf_nodes", 0);