Add example for 2D triangle-point intersection

[masterleinad]

This should be useful for XGC. In particular, this pull request separates TreeTraversal construction and actually running the search allowing applications to use the TreeTraversal class in their own Kokkos kernels.

[aprokop]

I'm deeply confused about the purpose of this PR and its relationship to XGC. In my understanding, XGC would require calling tree traversal individually from each thread, with no collaboration. This PR seems to still assume that there's a place in XGC where all thread are able to launch the search at the same time.

I thought that instead the API for TreeTraversal should be changed to allow a single thread (like team_member) as an argument.

I may be missing something here.

Also, this PR tries to do too many things simultaneously, and should be split in multiple, like the part touching the core (with the addition of unit testing) and adding an example.

[masterleinad]

Also, this PR tries to do too many things simultaneously, and should be split in multiple, like the part touching the core (with the addition of unit testing) and adding an example.

This is a draft showing my current progress.

[masterleinad]

I'm deeply confused about the purpose of this PR and its relationship to XGC. In my understanding, XGC would require calling tree traversal individually from each thread, with no collaboration. This PR seems to still assume that there's a place in XGC where all thread are able to launch the search at the same time.

I'm happy to discuss why this approach might or might not work.

[masterleinad]

This should now be in decent shape to be reviewed again. As opposed to the initial approach, we now run a "classical" batched query.

[masterleinad]

We only see SYCL warnings that are being addressed in #923.

[aprokop]

Global comments:

• Template on MemorySpace, not DeviceType
• Use _blah instead of blah_ for class member variables
• Prefix all views and kernels with Example::

I'd like to think whether #915 makes this easier. If we construct a hierarchy on triangles, how do we get the performance out of it? Would not having Mapping stored significantly affect the performance? It seems to do a few calculations, is it worth storing?

[aprokop]

I think it would be nice if we have a parameter n in main() for the number of triangles. Could be constexpr.

[aprokop]

#915 will affect this. We would want to construct the hierarchy based on triangles. However, not sure how mapping plays into this. Ideally, would want to do an intersection of a triangle with a point within ArborX without and extra data. Is that much slower?

[masterleinad]

It shouldn't be very difficult to derive a variant or adapt once #915 is merged.

[aprokop]

Provide a parameter with the number of points.

[aprokop]

How do you distinguish whether intersection was found or not? The default values for these views are indistinguishable for the wrong ones.

[masterleinad]

In Debug mode we run a full test checking that we detected the expected intersections and get the correct coefficients.

[aprokop]

I'd like to have NDEBUG (except for the final one checking the results) and print outs removed (print outs may be replaced by leading comments).
I think it would work well if you have them in one of the commits in history to be able to easily restore the code, but they add a bulk to the example. If we want a user to read easily, we want to make it as short as possible.

[aprokop]

Why do we store coefficients? Is that because the problem we consider is interpolation? I don't remember what XGC or the like want.

[masterleinad]

Yes. In my opinion, this is one of the points of this example.

[aprokop]

How was this eps chosen? What happens with smaller values?

[masterleinad]

IIRC, this was about the magnitude I needed to pass CI.

[masterleinad]

I'd like to have NDEBUG (except for the final one checking the results) and print outs removed (print outs may be replaced by leading comments).
I think it would work well if you have them in one of the commits in history to be able to easily restore the code, but they add a bulk to the example. If we want a user to read easily, we want to make it as short as possible.

Here you go!

[aprokop]

Overall, this seems ok

There's one thing that I'm pretty uncomfortable with in the current version. Suppose, I'm a user that wants to solve this problem. Why should I care about the mapping? What I really want is to have a set of triangles, and a set of points, and get the results. I wouldn't want to be concerned about constructing some mapping, passing it in, or something. Ideally, it would be as simple as

  auto bvh = BVH(space, triangles); // for example, a Kokkos::View<Triangle*, MemorySpace>
  bvh.query(space, points_intersect, callback); // with callback being called only on the positive match

So I wonder about it. How much performance do we lose if we compute coefficients alpha and beta inside the callback? Is there a way for us to avoid that?

Currently, while it may be efficient, the example's interface is not self contained, and requires a lot of work from the user. I think it is important to keep simple things simple, while allowing the flexibility for a user to get the performance out with some effort.

[aprokop]

masterleinad#7

I also noticed that det and inv_det are actually equal to 1.f for this configuration. So, if you accidentally miss using them in any of the formulas, the check would still succeed. It may be better to shift some of the nodes slightly during the triangle generation so that it doesn't happen.

[aprokop]

LGTM with the understanding of major simplification after APIv2 is finalized.

[aprokop]

I made a crack at what the example would look like in APIv2 (without mapping) here. I think it would be much cleaner and shorter. There's a question of how to not repeat the calculations, though.

[masterleinad]

LGTM with the understanding of major simplification after APIv2 is finalized.

Sure.