From 1b469532c39b6465ae7a1f82c463c25a517ed0d1 Mon Sep 17 00:00:00 2001 From: Andrey Prokopenko Date: Fri, 11 Oct 2024 23:49:52 -0400 Subject: [PATCH 1/3] Add CTAD for Indexables --- src/cluster/ArborX_DBSCAN.hpp | 4 +--- src/spatial/ArborX_BruteForce.hpp | 4 +--- src/spatial/ArborX_LinearBVH.hpp | 3 +-- src/spatial/detail/ArborX_IndexableGetter.hpp | 8 ++++++++ test/tstDetailsTreeConstruction.cpp | 5 ++--- test/tstIndexableGetter.cpp | 6 ++---- 6 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/cluster/ArborX_DBSCAN.hpp b/src/cluster/ArborX_DBSCAN.hpp index 94dc2e6d9..035be5c87 100644 --- a/src/cluster/ArborX_DBSCAN.hpp +++ b/src/cluster/ArborX_DBSCAN.hpp @@ -302,9 +302,7 @@ dbscan(ExecutionSpace const &exec_space, Primitives const &primitives, Box bounds; Details::TreeConstruction::calculateBoundingBoxOfTheScene( exec_space, - Details::Indexables{ - points, Details::DefaultIndexableGetter{}}, - bounds); + Details::Indexables{points, Details::DefaultIndexableGetter{}}, 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. diff --git a/src/spatial/ArborX_BruteForce.hpp b/src/spatial/ArborX_BruteForce.hpp index eefd8c4ea..ab9085fc0 100644 --- a/src/spatial/ArborX_BruteForce.hpp +++ b/src/spatial/ArborX_BruteForce.hpp @@ -198,9 +198,7 @@ void BruteForce::query( Details::BruteForceImpl::query( Tag{}, space, predicates, _values, - Details::Indexables{ - _values, _indexable_getter}, - callback); + Details::Indexables{_values, _indexable_getter}, callback); } } // namespace ArborX diff --git a/src/spatial/ArborX_LinearBVH.hpp b/src/spatial/ArborX_LinearBVH.hpp index e6f2cafff..538c73ae8 100644 --- a/src/spatial/ArborX_LinearBVH.hpp +++ b/src/spatial/ArborX_LinearBVH.hpp @@ -238,8 +238,7 @@ BoundingVolumeHierarchy:: return; } - Details::Indexables indexables{values, - indexable_getter}; + Details::Indexables indexables{values, indexable_getter}; Kokkos::Profiling::pushRegion( "ArborX::BVH::BVH::calculate_scene_bounding_box"); diff --git a/src/spatial/detail/ArborX_IndexableGetter.hpp b/src/spatial/detail/ArborX_IndexableGetter.hpp index d11bca5fd..3c27ef988 100644 --- a/src/spatial/detail/ArborX_IndexableGetter.hpp +++ b/src/spatial/detail/ArborX_IndexableGetter.hpp @@ -68,6 +68,14 @@ struct Indexables KOKKOS_FUNCTION auto size() const { return _values.size(); } }; +template +#if KOKKOS_VERSION >= 40400 +KOKKOS_DEDUCTION_GUIDE +#else +KOKKOS_FUNCTION +#endif + Indexables(Values, IndexableGetter) -> Indexables; + } // namespace ArborX::Details #endif diff --git a/test/tstDetailsTreeConstruction.cpp b/test/tstDetailsTreeConstruction.cpp index f093d4136..6abff5160 100644 --- a/test/tstDetailsTreeConstruction.cpp +++ b/test/tstDetailsTreeConstruction.cpp @@ -80,9 +80,8 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(assign_morton_codes, DeviceType, scene_host, {{{0.0, 0.0, 0.0}}, {{(float)N, (float)N, (float)N}}})); LegacyValues> values{boxes}; - ArborX::Details::Indexables - indexables{values, ArborX::Details::DefaultIndexableGetter{}}; + ArborX::Details::Indexables indexables{ + values, ArborX::Details::DefaultIndexableGetter{}}; // Test for a bug where missing move ref operator() in DefaultIndexableGetter // results in a default initialized indexable used in scene box calucation. diff --git a/test/tstIndexableGetter.cpp b/test/tstIndexableGetter.cpp index b36c6af46..2e92cc35e 100644 --- a/test/tstIndexableGetter.cpp +++ b/test/tstIndexableGetter.cpp @@ -110,8 +110,7 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(indexables, DeviceType, ARBORX_DEVICE_TYPES) ArborX::PrimitivesTag>; Primitives primitives(points_cloud); - ArborX::Details::Indexables indexables{ - primitives, indexable_getter}; + ArborX::Details::Indexables indexables{primitives, indexable_getter}; ArborX::Box<3> box; calculateBoundingBoxOfTheScene(ExecutionSpace{}, indexables, box); @@ -126,8 +125,7 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(indexables, DeviceType, ARBORX_DEVICE_TYPES) ArborX::PrimitivesTag>; Primitives primitives(points_cloud); - ArborX::Details::Indexables indexables{ - primitives, indexable_getter}; + ArborX::Details::Indexables indexables{primitives, indexable_getter}; ArborX::Box<3> box; calculateBoundingBoxOfTheScene(ExecutionSpace{}, indexables, box); From d2f6ac9c10a6465ee94b4d3c82fbece7bf5757e7 Mon Sep 17 00:00:00 2001 From: Andrey Prokopenko Date: Fri, 8 Nov 2024 12:15:47 -0500 Subject: [PATCH 2/3] Move IndexablesGetter from Details to Experimental --- src/cluster/ArborX_DBSCAN.hpp | 3 ++- src/distributed/ArborX_DistributedTree.hpp | 5 +++-- src/spatial/ArborX_BruteForce.hpp | 2 +- src/spatial/ArborX_LinearBVH.hpp | 4 ++-- src/spatial/detail/ArborX_IndexableGetter.hpp | 14 ++++++++++++-- test/CMakeLists.txt | 6 +++--- test/tstCompileOnlyTypeRequirements.cpp | 7 +++---- test/tstDetailsTreeConstruction.cpp | 4 ++-- test/tstIndexableGetter.cpp | 2 +- 9 files changed, 29 insertions(+), 18 deletions(-) diff --git a/src/cluster/ArborX_DBSCAN.hpp b/src/cluster/ArborX_DBSCAN.hpp index 035be5c87..493c9385c 100644 --- a/src/cluster/ArborX_DBSCAN.hpp +++ b/src/cluster/ArborX_DBSCAN.hpp @@ -302,7 +302,8 @@ dbscan(ExecutionSpace const &exec_space, Primitives const &primitives, Box bounds; Details::TreeConstruction::calculateBoundingBoxOfTheScene( exec_space, - Details::Indexables{points, Details::DefaultIndexableGetter{}}, bounds); + Details::Indexables{points, Experimental::DefaultIndexableGetter{}}, + 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. diff --git a/src/distributed/ArborX_DistributedTree.hpp b/src/distributed/ArborX_DistributedTree.hpp index 5d0431fee..0466cd2ec 100644 --- a/src/distributed/ArborX_DistributedTree.hpp +++ b/src/distributed/ArborX_DistributedTree.hpp @@ -37,7 +37,8 @@ class DistributedTreeBase using BoundingVolume = typename BottomTree::bounding_volume_type; using TopTree = BoundingVolumeHierarchy, - Details::DefaultIndexableGetter, BoundingVolume>; + Experimental::DefaultIndexableGetter, + BoundingVolume>; using bottom_tree_type = BottomTree; using top_tree_type = TopTree; @@ -119,7 +120,7 @@ class DistributedTreeBase // NOTE: query() must be called as collective over all processes in the // communicator passed to the constructor template >>, diff --git a/src/spatial/ArborX_BruteForce.hpp b/src/spatial/ArborX_BruteForce.hpp index ab9085fc0..a375e46b8 100644 --- a/src/spatial/ArborX_BruteForce.hpp +++ b/src/spatial/ArborX_BruteForce.hpp @@ -31,7 +31,7 @@ namespace ArborX { template >>, diff --git a/src/spatial/ArborX_LinearBVH.hpp b/src/spatial/ArborX_LinearBVH.hpp index 538c73ae8..3b1888f05 100644 --- a/src/spatial/ArborX_LinearBVH.hpp +++ b/src/spatial/ArborX_LinearBVH.hpp @@ -49,7 +49,7 @@ struct HappyTreeFriends; } // namespace Details template >>, @@ -179,7 +179,7 @@ KOKKOS_FUNCTION typename Details::AccessValues::value_type>; template >>, diff --git a/src/spatial/detail/ArborX_IndexableGetter.hpp b/src/spatial/detail/ArborX_IndexableGetter.hpp index 3c27ef988..4aec2ab36 100644 --- a/src/spatial/detail/ArborX_IndexableGetter.hpp +++ b/src/spatial/detail/ArborX_IndexableGetter.hpp @@ -16,7 +16,10 @@ #include #include -namespace ArborX::Details +namespace ArborX +{ + +namespace Experimental { struct DefaultIndexableGetter @@ -52,6 +55,11 @@ struct DefaultIndexableGetter } }; +} // namespace Experimental + +namespace Details +{ + template struct Indexables { @@ -76,6 +84,8 @@ KOKKOS_FUNCTION #endif Indexables(Values, IndexableGetter) -> Indexables; -} // namespace ArborX::Details +} // namespace Details + +} // namespace ArborX #endif diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 7a3f3ca32..6fdde05a8 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -88,7 +88,7 @@ foreach(_test Callbacks Degenerate ManufacturedSolution ComparisonWithBoost) "using ArborX_Legacy_BVH_Box =\n" " LegacyTree>,\n" - " ArborX::Details::DefaultIndexableGetter, ArborX::Box<3, float>>>;\n" + " ArborX::Experimental::DefaultIndexableGetter, ArborX::Box<3, float>>>;\n" "#define ARBORX_TEST_TREE_TYPES Tuple\n" "#define ARBORX_TEST_DEVICE_TYPES std::tuple<${ARBORX_DEVICE_TYPES}>\n" "#include \n" @@ -106,7 +106,7 @@ foreach(_test Callbacks Degenerate ManufacturedSolution ComparisonWithBoost) "using ArborX_Legacy_BruteForce_Box =\n" " LegacyTree>,\n" - " ArborX::Details::DefaultIndexableGetter, ArborX::Box<3, float>>>;\n" + " ArborX::Experimental::DefaultIndexableGetter, ArborX::Box<3, float>>>;\n" "#define ARBORX_TEST_TREE_TYPES Tuple\n" "#define ARBORX_TEST_DEVICE_TYPES std::tuple<${ARBORX_DEVICE_TYPES}>\n" "#define ARBORX_TEST_DISABLE_CALLBACK_EARLY_EXIT\n" @@ -130,7 +130,7 @@ foreach(_test Callbacks Degenerate ManufacturedSolution ComparisonWithBoost) "using ArborX_Legacy_BVH_${_bounding_volume} =\n" " LegacyTree,\n" - " ArborX::Details::DefaultIndexableGetter, ${_bounding_volume}>>;\n" + " ArborX::Experimental::DefaultIndexableGetter, ${_bounding_volume}>>;\n" "#define ARBORX_TEST_TREE_TYPES Tuple\n" "#define ARBORX_TEST_DEVICE_TYPES std::tuple<${ARBORX_DEVICE_TYPES}>\n" "#define ARBORX_TEST_DISABLE_NEAREST_QUERY\n" diff --git a/test/tstCompileOnlyTypeRequirements.cpp b/test/tstCompileOnlyTypeRequirements.cpp index ce028c974..4d5260b13 100644 --- a/test/tstCompileOnlyTypeRequirements.cpp +++ b/test/tstCompileOnlyTypeRequirements.cpp @@ -63,10 +63,9 @@ void check_bounding_volume_and_predicate_geometry_type_requirements() { using ExecutionSpace = Kokkos::DefaultExecutionSpace; using MemorySpace = ExecutionSpace::memory_space; - using Tree = - ArborX::BoundingVolumeHierarchy; + using Tree = ArborX::BoundingVolumeHierarchy< + MemorySpace, Test::PrimitivePointOrBox, + ArborX::Experimental::DefaultIndexableGetter, Test::FakeBoundingVolume>; Kokkos::View primitives( "primitives", 0); diff --git a/test/tstDetailsTreeConstruction.cpp b/test/tstDetailsTreeConstruction.cpp index 6abff5160..d0c6322d3 100644 --- a/test/tstDetailsTreeConstruction.cpp +++ b/test/tstDetailsTreeConstruction.cpp @@ -81,7 +81,7 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(assign_morton_codes, DeviceType, LegacyValues> values{boxes}; ArborX::Details::Indexables indexables{ - values, ArborX::Details::DefaultIndexableGetter{}}; + values, ArborX::Experimental::DefaultIndexableGetter{}}; // Test for a bug where missing move ref operator() in DefaultIndexableGetter // results in a default initialized indexable used in scene box calucation. @@ -171,7 +171,7 @@ void generateHierarchy(Primitives primitives, MortonCodes sorted_morton_codes, BoundingVolume bounds; ArborX::Details::TreeConstruction::generateHierarchy( space, LegacyValues{primitives}, - ArborX::Details::DefaultIndexableGetter{}, permutation_indices, + ArborX::Experimental::DefaultIndexableGetter{}, permutation_indices, sorted_morton_codes, leaf_nodes, internal_nodes, bounds); } diff --git a/test/tstIndexableGetter.cpp b/test/tstIndexableGetter.cpp index 2e92cc35e..1e66c702f 100644 --- a/test/tstIndexableGetter.cpp +++ b/test/tstIndexableGetter.cpp @@ -100,7 +100,7 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(indexables, DeviceType, ARBORX_DEVICE_TYPES) ArborX::Box scene_bounding_box{{-1.f, -1.f, -1.f}, {1.f, 1.f, 1.f}}; - using IndexableGetter = ArborX::Details::DefaultIndexableGetter; + using IndexableGetter = ArborX::Experimental::DefaultIndexableGetter; IndexableGetter indexable_getter; { From 53442f6248f1536cb82acdaed261be50dcf9908c Mon Sep 17 00:00:00 2001 From: Andrey Prokopenko Date: Sun, 10 Nov 2024 10:09:07 -0500 Subject: [PATCH 3/3] Enable the deduction guide only for C++17 --- src/spatial/detail/ArborX_IndexableGetter.hpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/spatial/detail/ArborX_IndexableGetter.hpp b/src/spatial/detail/ArborX_IndexableGetter.hpp index 4aec2ab36..84909e426 100644 --- a/src/spatial/detail/ArborX_IndexableGetter.hpp +++ b/src/spatial/detail/ArborX_IndexableGetter.hpp @@ -76,6 +76,7 @@ struct Indexables KOKKOS_FUNCTION auto size() const { return _values.size(); } }; +#ifdef KOKKOS_ENABLE_CXX17 template #if KOKKOS_VERSION >= 40400 KOKKOS_DEDUCTION_GUIDE @@ -83,6 +84,7 @@ KOKKOS_DEDUCTION_GUIDE KOKKOS_FUNCTION #endif Indexables(Values, IndexableGetter) -> Indexables; +#endif } // namespace Details