ArborX icon indicating copy to clipboard operation
ArborX copied to clipboard

Improve quality of life for APIv2

Open aprokop opened this issue 1 year ago • 2 comments

attach_indices

attach_indices should work the same for both primitives and predicates:

BVH<blah> tree(space, ArborX::attach_indices<long>(primitives));
tree.query(space, ArborX::attach_indices<unsigned>(queries));

This requires to somehow unify/ignore the tags.

Addressed in #1036.

Simplify using APIv2 in APIv1 regime

  1. If a user a) does not specify a callback, b) the output value_type is int, and c) index is constructed on PairValueIndex, then automatically use LegacyDefaultCallback underneath.
  2. Provide template deduction guides to avoid specifying PairValueIndex template argument.

Addressed in #1051.

Improve constructing queries from the set of primitives

  • Construct queries as primitives with radius
  • Construct queries as primitives with k nearest

Addressed in #1038.

Goal(-like)

Would be nice to write:

Kokkos::View<ArborX::Point*, MemorySpace> points = <blah>;
BVH bvh(space, ArborX::attach_indices(points));
bvh.query(space, ArborX::attach_indices(ArborX::within_predicates(points, r)), Callback{});

or

Kokkos::View<ArborX::Point*, MemorySpace> points = <blah>;
BVH bvh(space, ArborX::attach_indices(points));
Kokkos::View<int*, MemorySpace> offsets("offsets", 0);
Kokkos::View<int*, MemorySpace> indices("indices", 0);
bvh.query(space, ArborX::nearest_predicates(points, 10)), offsets, indices);

Wonder if the latter should be more like

BVH bvh(space, Iota{n}, RangeIndexableGetter{points});

or something similar.

aprokop avatar Jan 30 '24 04:01 aprokop

Other than CTAD, everything else has been implemented.

aprokop avatar Apr 11 '24 16:04 aprokop

We still have some inconvenient and confusing stuff.

If a user is really only interested in indices (aka, APIv1), we have them hopping through:

  • Having to call attach_indices
  • Having the hierarchy being template on PairValueIndex
  • Having to decouple index from the pair inside a callback

A user wants value_type to be int. But due to the performance, we currently don't allow that.

I think right now we couple performance related stuff with the user visible stuff. From the performance perspective, we want to pack value and geometry next to each other so as to reduce the number of memory transactions. But that should be hidden from a user.

One way to do that is to make leaf nodes store value, rope and additional stuff (like, cached indexable getter results). The problem is that we don't know the type of what's stored there until we call the constructor, as it wouldn't be a BVH template parameter. We also don't want to duplicate the geometry if it is already a part of the value that's being used by indexable getter.

So, we need to think more how to make APIv1 work seemlessly in APIv2.

aprokop avatar Oct 01 '24 17:10 aprokop