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

Improve convergence of epa algorithm in case it's stuck #245

Merged
merged 19 commits into from
Aug 12, 2024

Conversation

Ughuuu
Copy link
Contributor

@Ughuuu Ughuuu commented Jul 27, 2024

Improves converges by checking if it's stuck. In such a case return a result, as a better solution will not be found, and most likely they are intersecting but not within the EPS.

Also decrease max iterations from 10000 to 100. It usually converges in 4-5 iterations, no need to ever do as many. Also we never really want a precision that big, we are ok with a lower precision most times.

Possibly fixes:

@Ughuuu
Copy link
Contributor Author

Ughuuu commented Jul 27, 2024

With this fix it actually returns a contact (didn't know how to draw the intersection better):

image

Also it does so in 5 iterations and returns a collision, instead of doing 10000 iterations and returning None.

@Ughuuu Ughuuu marked this pull request as ready for review July 27, 2024 16:26
@Vrixyz
Copy link
Contributor

Vrixyz commented Jul 29, 2024

Thanks! I'd like to add a test for this scenario, would a setup like dimforge/rapier#531 be the correct candidate ?

@Vrixyz Vrixyz added the bug Something isn't working label Jul 29, 2024
@Ughuuu
Copy link
Contributor Author

Ughuuu commented Jul 29, 2024

Sure, thats what I used locally. I didn't put the effort to migrate that sample to the parry repo, but probably that would be next step.

* attempt to make a test for epa convergence hanging

* fix capsule height to be like original bug report

* exact same parameters as original bug + adapt test
@Vrixyz Vrixyz requested a review from sebcrozet August 2, 2024 14:27
Copy link
Member

@sebcrozet sebcrozet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@Vrixyz
Copy link
Contributor

Vrixyz commented Aug 9, 2024

🎉 yay! I'd like to add a changelog line before merging though

@Vrixyz Vrixyz merged commit 837291f into dimforge:master Aug 12, 2024
6 checks passed
@Ughuuu Ughuuu deleted the custom-changes branch October 11, 2024 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EPA convergence failure on capsules
3 participants