Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add 1d support for ArborXWrappers #15720

Merged
merged 3 commits into from Jul 13, 2023
Merged

Conversation

masterleinad
Copy link
Member

Related to #15538.

@Rombur
Copy link
Member

Rombur commented Jul 11, 2023

We now require ArborX 1.3 which has multi-dimensional search support. It's probably cleaner and faster to use the new interface instead of copying everything to 3D data structures.

source/arborx/access_traits.cc Outdated Show resolved Hide resolved
@masterleinad
Copy link
Member Author

We now require ArborX 1.3 which has multi-dimensional search support. It's probably cleaner and faster to use the new interface instead of copying everything to 3D data structures.

That requires incompatible changes to the predicates, though, since they are not templated on the dimension but store 3-dim objects. I would rather do that in a follow-up.

@Rombur
Copy link
Member

Rombur commented Jul 11, 2023

Using CTAD, adding a template on the dimension should be backward compatible because the constructor of the predicates takes a Point or a BoundingBox that is already templated on the dimension.

@masterleinad
Copy link
Member Author

Using CTAD, adding a template on the dimension should be backward compatible because the constructor of the predicates takes a Point or a BoundingBox that is already templated on the dimension.

I had a look at how much it actually takes to use the ExperimentalHyperGeometry namespace. From a user perspective, i.e., in the examples everything works fine without changes due to CTAD. Deriving your own predicates or primitives from the ones provided is not backward-compatible, though, since you have to provide the additional template argument. What's more, is that Morton codes are not defined for dim=1 and the distributed BVH implementation is not templated on the type of the bounding volume hierarchy but uses the BVH class (using ArborX::Box) for both the upper and lower tree.
Thus, I think we need a couple more changes in ArborX before it really makes sense to switch to lower-dimensional data structures here.

@Rombur
Copy link
Member

Rombur commented Jul 13, 2023

clang-format is not happy

Co-authored-by: Wolfgang Bangerth <bangerth@colostate.edu>
@tamiko tamiko merged commit 0c4cd39 into dealii:master Jul 13, 2023
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants