-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Upgrade IrisInConfigurationSpace to IRIS-NP2 #21822
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
Comments
Two quetsions from me...
|
|
Extending IRIS-NP to support CollisionChecker seems somewhat orthogonal to upgrading to IRIS-NP2. A lot of the features that CollisionChecker supports need to be taken into account by the counterexample search programs. Negative padding seems particularly complicated to implement. Maybe we can keep these as separate TODOs? |
My understanding is that @rhjiang already has a version that works, except not for the blocker discussed in the slack thread I linked above. If that's not the case, then i agree we can/should separate them. |
The version I have uses CollisionChecker only for querying points in collision. Not sure I fully understand Tommy's comment, but, the version I have formulates the closest collision program the same way IRIS-NP did, not involving the CollisionChecker. |
In that case, I'm ok deferring #18830. But I guess you construct the SceneGraphCollisionChecker inside the iris call? (the user doesn't have to pass it in?) |
Plan is to not upgrade in place, but to make a new entry point (behind the new common interface from #21821). |
In our latest paper submission, @cohnt , @rhjiang , and @wernerpe made major improvements to IRIS-NP (the algorithm currently implemented in IrisInConfigurationSpace). Regions can be computed order(s) of magnitude faster, they tend to be bigger, and tend to have less faces!
We should upgrade our Drake implementation to this latest version of the algorithm.
One question is whether we should make the new code use the CollisionChecker interface (#18830). My understanding is that there was one remaining sticking point, which was discussed in this slack thread.
The text was updated successfully, but these errors were encountered: