search

adamantine-sim / adamantine

Software to simulate heat transfer for additive manufacturing
https://adamantine-sim.github.io/adamantine/
Other
34 stars 10 forks source link

Use Nearest predicate instead of Spatial predicate to compute the ray-box intersection #163

Closed Rombur closed 2 years ago

Rombur commented 2 years ago

This is the bug I talked about this morning. I switched from Intersect to Nearest. It's a little bit weird but the BoundingBox nearest to a Ray is the first BoundingBox that the Ray intersects. If a Ray misses a BoundingBox, the distance between the Ray and the BoundingBox is infinite.

Rombur commented 2 years ago

Retest

Rombur commented 2 years ago

Retest this please

Rombur commented 2 years ago

There are a couple things on this PR:

  1. I had to increase the tolerance on the MechanicalPhysics test because otherwise I would get some errors. I think it's fine because the test uses CG so, there are some extra round off errors
  2. clang-sanitizer is acting once again. I propose to disable the sanitizers for now. I will come back to it later.
  3. the reason the PR is failing for one configuration is because of the flag --fast-math. When using the flag, ArborX misses a cell.

I am not sure what to do with this PR. I think 1. and 2. should get merged but I am not sure about this PR itself. We can remove --fast-math and have the CI passes or just hold on the PR for now. I do want to understand what is going on in ArborX but I don't know how long it will take (I will do that as part of the Alexa project not the LDRD).

stvdwtt commented 2 years ago

@Rombur, my thought is to disable --fast-math and merge the whole PR. At this point, I think getting the ray intersections correct is more important than performance (within reason), especially since we need to start exercising the the ray-tracing in the DA workflow.

Rombur commented 2 years ago

I think I know what is going on. The ray hits an edge and it is parallel to the face. In the distributed case that face is at the limit between the two domains and because of a small round off error the ray misses the face. If I move the ray slightly to hit the face and not the edge the test passes. In practice, we would need to be very unlucky for this to happen since most rays won't be parallel to a face, hit exactly the edge, and hit just between different domains. I can modified the test or remove the fast-math. Note that the current code could potentially have the same problem but it is just not triggered by the test.

stvdwtt commented 2 years ago

In that case, I think modifying the test is the best way to go. I agree that the issue of hitting or missing an edge with a parallel ray is enough of an edge case (literally, sorry) that we can ignore it without practical repercussions.