Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Interpretation of Assertions #380

Open
cologne86 opened this issue Mar 13, 2023 · 8 comments
Open

Interpretation of Assertions #380

cologne86 opened this issue Mar 13, 2023 · 8 comments

Comments

@cologne86
Copy link

cologne86 commented Mar 13, 2023

Hi,
Using this library without NDEBUG set i am experiencing crashes caused by assert calls.

I am getting assertions using two hpp::fcl::BVHModel<hpp::fcl::OBBRSS> objects as input
calling hpp::fcl::collide. Depending on transformations those would sometimes cause one of the following asserts:

https://github.com/humanoid-path-planner/hpp-fcl/blob/893d57c6e08434fd482a656977c5376cb360e6b4/src/narrowphase/gjk.cpp#L331
https://github.com/humanoid-path-planner/hpp-fcl/blob/893d57c6e08434fd482a656977c5376cb360e6b4/src/narrowphase/gjk.cpp#L558
https://github.com/humanoid-path-planner/hpp-fcl/blob/893d57c6e08434fd482a656977c5376cb360e6b4/src/narrowphase/gjk.cpp#L1264

However, when looking at the surrounding code here:

       if (la > lb) {
          assert(false);
          w0 = b0;
          w1 = b1;

it seems that these assertions are not explicitly error states,but the algorithm does handle the input in that case. Constraints in other cases also seem to me quite strict.

When building the software with NDEBUG, results still look valid and i have not experienced any crashes.
The question is:
Should the assertions never be triggered with a valid input, or is it OK to go on without debug information?

@jcarpent
Copy link
Contributor

I guess this is related to an ill-formated shape with degenerated faces (e.g., aligned points belonging to a triangle).
@lmontaut WHat is your thoughts?

@lmontaut
Copy link
Contributor

Hi @cologne86,
Could you please provide an example of code to replicate and fix the bugs?

The asserts correspond to degenerate cases that can happen with a valid input.

https://github.com/humanoid-path-planner/hpp-fcl/blob/893d57c6e08434fd482a656977c5376cb360e6b4/src/narrowphase/gjk.cpp#L331

This assert failing is due to GJK currently using a relative convergence criterion making the value 1e-6 arbitrary. This will be fixed in hppfcl3x notably by switching to an absolute convergence criterion.

https://github.com/humanoid-path-planner/hpp-fcl/blob/893d57c6e08434fd482a656977c5376cb360e6b4/src/narrowphase/gjk.cpp#L558

This assert signals a rare behavior when computing nearest points between two shapes, related to how GJK constructs the simplex leading to the computation of nearest points. The code after the assert is the fallback behavior of hppfcl. The computed nearest points remain valid however.

https://github.com/humanoid-path-planner/hpp-fcl/blob/893d57c6e08434fd482a656977c5376cb360e6b4/src/narrowphase/gjk.cpp#L1264

This assert can trigger if the simplex is degenerate (a tetrahedron with 0 volume).

@cologne86
Copy link
Author

Thanks a lot for your quick replies, code and mesh generation is included in a larger context.
I will work on a small example test to trigger these asserts, will need a bit of time.

@cologne86
Copy link
Author

cologne86 commented Mar 14, 2023

It seems i was wrong about:
https://github.com/humanoid-path-planner/hpp-fcl/blob/893d57c6e08434fd482a656977c5376cb360e6b4/src/narrowphase/gjk.cpp#L558
I was not able to trigger that assert but another one:
https://github.com/humanoid-path-planner/hpp-fcl/blob/893d57c6e08434fd482a656977c5376cb360e6b4/src/collision_node.cpp#L58

Created a pull request that adds a test cpp. However i did not include it as it would result in tests failing.

cologne86 pushed a commit to cologne86/hpp-fcl that referenced this issue Mar 14, 2023
cologne86 pushed a commit to cologne86/hpp-fcl that referenced this issue Mar 14, 2023
@cologne86
Copy link
Author

I have another question regarding this statement:

This assert failing is due to GJK currently using a relative convergence criterion making the value 1e-6 arbitrary. This will be fixed in hppfcl3x notably by switching to an absolute convergence criterion.

Is there a risk in the current implementation, that the algorithm wont converge?

@jcarpent
Copy link
Contributor

It will converge as it is a convex optimization problem, and we have a limit on the number of iterations. It is just that the check values are not properly aligned together and they are mostly here for developing purposes.

@lmontaut
Copy link
Contributor

No, the algorithm will converge. At the end of the day GJK is an algorithm with solves a convex optimization problem, up to a certain tolerance w.r.t the optimal solution of this optimization problem.
Switching from a relative to an absolute convergence criterion simply changes the mathematical guarantees of the algorithm in terms of distance to the optimal solution.
The reason why this is not already done is because if we switch from a relative to an absolute convergence criterion, then the tolerance of the algo (the default is 1e-6) does not have the same geometrical interpretation.

At the end of the day using an absolute convergence criterion rather than a relative one makes it much easier to work with in the code and it is also much simpler to interpret geometrically, I have already done some tests in the past and this seems to resolve a lot of the asserts I have encountered.

jcarpent pushed a commit to cologne86/hpp-fcl that referenced this issue Apr 17, 2023
@diarodriguezva
Copy link

diarodriguezva commented Feb 3, 2024

Similar as @cologne86 I am getting these assertions when the library is compiled in debug mode. Specifically from the collide function inside collision_node.cpp when enable_contact is set to true

assert(result.distance_lower_bound * result.distance_lower_bound -
                   sqrDistLowerBound <
               1e-8);

when is it expected that result.distance_lower_bound * result.distance_lower_bound does not match sqrDistLowerBound ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants