Bug fix: avoid using caching in SurfaceSpatialIndex as this results i… #392
Conversation
…n different values for the same point when ran in parallel
There was a problem hiding this comment.
Thanks for the PR. I cannot assess the implications of this change and need to defer to @Andreas-Forster. If it is fine for Andreas, it is okay for me.
| @@ -188,7 +188,7 @@ private[mesh] class TriangleMesh3DSpatialIndex(private val bs: BoundingSphere, | |||
| val p = point.toVector | |||
|
|
|||
| // last triangle might be a good candidate | |||
There was a problem hiding this comment.
I guess this comment does not fit the code anymore and should be updated.
There was a problem hiding this comment.
I agree with the PR. It fixes the issue we have right now. While it might be that the PR makes the algorithm slightly slower, it is definitively deterministic.
When we need the last bit of performance we could later on check if there is a way to fix this issue, by improving the algorithm.
So I am fine with merging it.
|
@Andreas-Forster I realized only now that I don't understand the issue very well. Could you quickly comment on why taking the last points as a starting point makes it non-deterministic, while taking the head does not? Is this also due to numerical instabilities or is there a bug hidden somewhere that we should fix? |
|
I did not debug it in detail. But I guess the tree traversal could be different for a point when you have a different initialization there. Just the order of visiting the nodes. I guess that sometimes then are two points found that have the (nearly) same distance and hence the order matters. Then starting always at the same point, makes the tree traversal agnostic of the last found point. |
…n different values for the same point when ran in parallel. Fixes issue #367