Resolve a new GJK/EPA real world error #446
Conversation
box-box regression 6 in signed distance test.
There was a problem hiding this comment.
+@hongkai-dai for feature review, please.
Reviewable status: 0 of 2 files reviewed, all discussions resolved (waiting on @hongkai-dai)
There was a problem hiding this comment.
Reviewed 2 of 2 files at r1.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @sherm1)
SeanCurtis-TRI
force-pushed
the
PR_more_epa_regression_tests
branch
from
d6ed943 to
ac40c37
Compare
There was a problem hiding this comment.
CI hopefully resolved by loosening the threshold in the new test parameter.
Reviewable status: 1 of 2 files reviewed, all discussions resolved (waiting on @hongkai-dai and @sherm1)
There was a problem hiding this comment.
Reviewed 1 of 2 files at r1, 1 of 1 files at r2.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @SeanCurtis-TRI)
include/fcl/narrowphase/detail/convexity_based_algorithm/gjk_libccd-inl.h, line 374 at r2 (raw file):
// Compares the given `value` against a _squared epsilon_. This is particularly // important when testing some squared quantity (e.g., distance) to see if it
nit: (e.g., distance²)
include/fcl/narrowphase/detail/convexity_based_algorithm/gjk_libccd-inl.h, line 376 at r2 (raw file):
// important when testing some squared quantity (e.g., distance) to see if it // is zero. Comparing distance squared directly against zero means that the // distance is considered zero with only half the precision (e.g., it is
minor: I'm not sure what you mean by "comparing directly against zero". Did you mean "comparing directly against epsilon"? That would make more sense in this context as I understand it.
This specifically addresses issues when converting a 3-simplex into a tetrahedron. It was inspired by a real-world failure case which revealed that a conversion was performed that broke EPA assumptions. The changes consist of: 1. Superficial formatting changes. 2. Added documentation. 3. Testing squared distance against epsilon _squared_ (instead of epsilon). (This last led to the example failure case to pass.) 4. Eliminate redundant degeneracy tests; if the origin lies on one face, simply demote to that triangle and continue.
SeanCurtis-TRI
force-pushed
the
PR_more_epa_regression_tests
branch
from
ac40c37 to
b4cda6a
Compare
There was a problem hiding this comment.
Comments addressed. I did some more significant rephrasing of the isAbsValueLessThanEpsSquared() function.
Reviewable status: 1 of 2 files reviewed, all discussions resolved (waiting on @hongkai-dai and @sherm1)
There was a problem hiding this comment.
Reviewed 1 of 1 files at r2, 1 of 1 files at r3.
Reviewable status:
There was a problem hiding this comment.
Reviewed 1 of 1 files at r3.
Reviewable status:
This PR has two commits -- the failing test in the first commit and the "fix" in the second commit. They will be squashed upon merging.
The solution to this particular failure was cleaning up one aspect of the conversion from simplex to polytope. Specifically, the conversion of a 3-simplex into a tetrahedron.
resolves #445
This change is