Add "self collision" accelerated traversal for spatial predicates #801
Conversation
dalg24
force-pushed
the
self_collision
branch
from
124c30b
to
32848b5
Compare
I haven't thought much about the name. But I think the currently proposed implementation is too restrictive. I think what we want here is to allow a user to provide any kind of geometric objects for self-intersections. They could be spheres, boxes, or some custom types. We should not restrict it here. I think we would then want a user to be able to tell us how to expand that geometric object by a given radius. However, we should not rely on the user to construct predicates, I think. Looking in the future, I think we would want to be able to intersect stored values, instead of intersecting bounding volumes. That would require a user to provide us with some kind of lambda anyway. If we want to start really small and be super restrictive, just trying it out for the MD case, then the current interface would be fine except you would want to static assert I should also mention that while I like this order of arguments (offsets, indices) a lot better than (indices, offsets), it differs from the usage in other places which uses (indices, offsets). |
What feedback are you looking for here? Whether the current test is sufficient, or ideas about how to test it? |
|
Also, do you think we should a switch to whether include self-collisions in the results or not? There may be cases to use either. If you are interesting in pair-wise interactions, like force calculations, you would not want them. But if you do something like interpolation from all the points within distance, you would want to include yourself. However, that would make converting half to full a bit more complex to avoid duplication. |
|
I just noticed that there is also no callback option? Is that because it is geared towards Cabana? But it we want to use it in ArborX for DBSCAN, we would want to allow a pure callback option. |
| #ifdef V1 | ||
| if (left_child < i) | ||
| #else | ||
| if (true) |
There was a problem hiding this comment.
One of the benefits of the V2 version that I didn't think about is that it is agnostic to the ordering of the internal nodes. Meaning that as long as the ropes are appropriately constructed, it will find correct answer. But I think V1 would only work with Karras ordering, and not Apetrei or any other.
|
#485 has a few important comments and considerations to keep in mind. |
dalg24
force-pushed
the
self_collision
branch
from
32848b5
to
2eba856
Compare
| @@ -47,7 +47,7 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(iota, DeviceType, ARBORX_DEVICE_TYPES) | |||
| std::vector<int> w_ref = {0, 1, 2}; | |||
| auto w_host = Kokkos::create_mirror_view(w); | |||
| Kokkos::deep_copy(w_host, w); | |||
| BOOST_TEST(w_ref == w_host, tt::per_element()); | |||
| // BOOST_TEST(w_ref == w_host, tt::per_element()); | |||
There was a problem hiding this comment.
Didn't mean to commit that. Please ignore.
|
I think it would make sense to separate this PR into 3:
|
|
Closing in favor of #812 |
This is a draft
I am looking for feedback on:
cabana_proxyfree function (name? signature?)cabana_proxyfunctionI could propose
expandHalfToFull()on its own. The motivation for that function was that I observed it was significant faster to search for half and expand than register both (i,j) and (j,i) from the callback.