Test on-node space indexes using both float and double #1200

aprokop · 2025-01-06T19:44:56Z

Generalized make(), makeIntersects*(), makeNearest*() to work with any kind of geometry
Added testing for on-node space indexes for both float and double

src/spatial/detail/ArborX_BruteForceImpl.hpp

src/spatial/detail/ArborX_Predicates.hpp

aprokop · 2025-01-11T13:36:35Z

@dalg24 Everything passed (except one CUDA build that threw subprocess exceptions). I think it should be fine to merge if you are OK with the changes.

aprokop · 2025-01-11T13:37:16Z

src/spatial/detail/ArborX_Predicates.hpp

@@ -175,6 +175,14 @@ struct PredicateWithAttachment : Predicate
      : Predicate(std::forward<Predicate>(pred))
      , _data(std::forward<Data>(data))
  {}
+#if defined(KOKKOS_ENABLE_CUDA) && defined(KOKKOS_COMPILER_CLANG) &&           \
+    (KOKKOS_COMPILER_CLANG < 1500)


Just being conservative here. No idea what combinations are susceptible.

dalg24 · 2025-01-10T23:34:43Z

test/Search_UnitTestHelpers.hpp

-          typename Coordinate = float>
-auto make(ExecutionSpace const &exec_space,
-          std::vector<ArborX::Box<DIM, Coordinate>> const &b)
+template <typename Tree, typename Geometry, typename ExecutionSpace>


I am not loving the ordering of the template parameters but I understand why you did it this way

dalg24 · 2025-01-10T23:38:09Z

test/Search_UnitTestHelpers.hpp

-template <typename DeviceType, int DIM = 3, typename Coordinate = float>
-auto makeIntersectsBoxQueries(
-    std::vector<ArborX::Box<DIM, Coordinate>> const &boxes,
+template <typename DeviceType, typename Geometry>


We still want to template on DeviceType?

I plan to switch it to templating on the ExecutionSpace in the future.

dalg24 · 2025-01-15T13:19:37Z

test/CMakeLists.txt

-  list(APPEND ARBORX_TEST_QUERY_TREE_SOURCES "${CMAKE_CURRENT_BINARY_DIR}/tstQueryTree${_test}_BF.cpp")
-  foreach(_bounding_volume KDOP14 KDOP18) # purposefully ommitting KDOP6 and KDOP26 to reduce the number of instantiations
-    file(WRITE "${CMAKE_CURRENT_BINARY_DIR}/tstQueryTree${_test}_BVH_${_bounding_volume}.cpp.tmp"
+  foreach (_precision float double)


By how much are we increasing compile time? runtime?
I am interested both in these numbers for the QueryTree exe and total times

test/Search_UnitTestHelpers.hpp

dalg24 · 2025-01-15T13:26:15Z

test/tstDistributedTreeNearest.cpp

+  using Point = ArborX::Point<3>;
+  using Box = ArborX::Box<3>;


Do we want to be explicit here and say float?

Not right now. I'll do that once we switch distributed test to run both.

test/tstDistributedTreeSpatial.cpp

aprokop · 2025-01-15T16:00:17Z

@dalg24 Addressed all your comments.

aprokop · 2025-01-17T23:58:27Z

By how much are we increasing compile time? runtime?
I am interested both in these numbers for the QueryTree exe and total times

On my Mac in Debug configuration with only the Serial backend:

master, making just the tree query test:

$ time make -j1 ArborX_Test_QueryTree.exe
[25/25] Linking CXX executable test/ArborX_Test_QueryTree.exe
real    1m35.138s
user    1m29.668s
sys     0m6.000s

master, making all tests:

$ cd test
$ time make -j1
[99/99] Linking CXX executable test/ArborX_Test_InterpMovingLeastSquares.exe
real    3m16.225s
user    3m4.549s
sys     0m13.047s

branch, making just the tree query test:

$ time make -j1 ArborX_TestQueryTree.exe
[41/41] Linking CXX executable test/ArborX_Test_QueryTree.exe
real    2m56.503s
user    2m46.213s
sys     0m11.336s

So, ~2x slower than float, as expected.
branch, making all tests:

$ cd test
$ time make -j1
[114/114] Linking CXX executable test/ArborX_Test_InterpMovingLeastSquares.exe
real    4m37.494s
user    4m20.802s
sys     0m18.160s

So, around ~40% slower than just float.

Runtime is in the epsilon, as on my Mac running query test with both float and double only takes 14s.

Co-authored-by: Damien L-G <dalg24+github@gmail.com>

aprokop · 2025-01-18T00:04:20Z

Rebased to fix conflicts with #1108.

aprokop added enhancement New feature or request testing Anything to do with tests and CI labels Jan 6, 2025

aprokop marked this pull request as ready for review January 6, 2025 21:45

aprokop requested a review from dalg24 January 6, 2025 21:57

aprokop force-pushed the test_double branch from 66ebada to de60f7a Compare January 7, 2025 02:48

aprokop added the bug Something isn't working label Jan 7, 2025

dalg24 reviewed Jan 7, 2025

View reviewed changes

src/spatial/detail/ArborX_BruteForceImpl.hpp Outdated Show resolved Hide resolved

dalg24 reviewed Jan 10, 2025

View reviewed changes

src/spatial/detail/ArborX_Predicates.hpp Outdated Show resolved Hide resolved

aprokop force-pushed the test_double branch from 563d976 to 25c334d Compare January 10, 2025 23:02

aprokop commented Jan 11, 2025

View reviewed changes

dalg24 reviewed Jan 15, 2025

View reviewed changes

aprokop and others added 9 commits January 17, 2025 19:02

Generalize make() in SearchUnitHelpers

b194a37

Test on-node space indexes for both float and double

a25460f

Clean make() usage

e952d84

Clean makeIntersects*() and makeNearest*()

b6c917e

bugfix: make sure we don't exceed scratch size capacity in BF::query

0bf60d6

Better solution to figure out alignment correction

0b67932

Co-authored-by: Damien L-G <dalg24+github@gmail.com>

Workaround for misaligned address in Clang+Cuda

f560776

Run all most query tests for both float and double

c932833

Uncomment and fix a test in spatial distributed

2737c55

aprokop force-pushed the test_double branch from 072c77b to 2737c55 Compare January 18, 2025 00:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Test on-node space indexes using both float and double #1200

Test on-node space indexes using both float and double #1200

aprokop commented Jan 6, 2025 •

edited

Loading

aprokop commented Jan 11, 2025

aprokop Jan 11, 2025

dalg24 Jan 10, 2025

dalg24 Jan 10, 2025

aprokop Jan 15, 2025

dalg24 Jan 15, 2025

dalg24 Jan 15, 2025

aprokop Jan 15, 2025

aprokop commented Jan 15, 2025

aprokop commented Jan 17, 2025 •

edited

Loading

aprokop commented Jan 18, 2025

Test on-node space indexes using both float and double #1200

Are you sure you want to change the base?

Test on-node space indexes using both float and double #1200

Conversation

aprokop commented Jan 6, 2025 • edited Loading

aprokop commented Jan 11, 2025

aprokop Jan 11, 2025

Choose a reason for hiding this comment

dalg24 Jan 10, 2025

Choose a reason for hiding this comment

dalg24 Jan 10, 2025

Choose a reason for hiding this comment

aprokop Jan 15, 2025

Choose a reason for hiding this comment

dalg24 Jan 15, 2025

Choose a reason for hiding this comment

dalg24 Jan 15, 2025

Choose a reason for hiding this comment

aprokop Jan 15, 2025

Choose a reason for hiding this comment

aprokop commented Jan 15, 2025

aprokop commented Jan 17, 2025 • edited Loading

aprokop commented Jan 18, 2025

aprokop commented Jan 6, 2025 •

edited

Loading

aprokop commented Jan 17, 2025 •

edited

Loading