Skip to content

Iris from clique cover allows the use of iris variants #23104

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

AlexandreAmice
Copy link
Contributor

@AlexandreAmice AlexandreAmice commented Jun 17, 2025

This change is Reviewable

Copy link
Contributor Author

@AlexandreAmice AlexandreAmice left a comment

Choose a reason for hiding this comment

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

+@cohnt for feature review.

cc @wernerpe if you want to chime in

Reviewable status: LGTM missing from assignee cohnt, needs platform reviewer assigned, needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on @AlexandreAmice)

Copy link
Contributor Author

@AlexandreAmice AlexandreAmice left a comment

Choose a reason for hiding this comment

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

@cohnt If you could suggest fixes to the docs. I don't really have time to comb through and explain the new usage. Also you would know better than me the pitfalls.

Also, there is a footgun in this sample PR where the subspace machinery almost certainly doesn't work since we don't sample from the subspace.

Reviewable status: LGTM missing from assignee cohnt, needs platform reviewer assigned, needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on @AlexandreAmice)

@SeanCurtis-TRI
Copy link
Contributor

planning/iris/iris_from_clique_cover.cc line 246 at r1 (raw file):

    checker.UpdatePositions(clique_ellipse.center(), builder_id);
    log()->debug("Iris builder thread {} is constructing a set.", builder_id);
    std::visit(

BTW You'll want to look at common/overloaded.h and generally such for overloaded to see how we handle variants in drake.

Copy link
Contributor

@cohnt cohnt left a comment

Choose a reason for hiding this comment

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

Reviewed 6 of 6 files at r1, all commit messages.
Reviewable status: 3 unresolved discussions, LGTM missing from assignee cohnt, needs platform reviewer assigned, needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on @AlexandreAmice)


planning/iris/iris_from_clique_cover.h line 40 at r1 (raw file):

   * allow more than 1 thread, then the debug visualizations of internal Iris
   * calls will be disabled. This is due to a limitation of drawing to meshcat
   * from outside the main thread.

I would add something like

@throws if the user specifies a parameterization in IrisZoOptions or IrisNp2Options.

Then add a check in IrisFromCliqueCover like the following:

if (!(options.parameterization.get_parameterization()(VectorXd::Zero(ndim)) == VectorXd::Zero(ndim))) {
  throw std::runtime_error("IrisFromCliqueCover does not yet support growing regions on a parameterized subspace.");
}

A test would be something like this test from IrisNp2:

VectorX<Variable> varable_vector(1);
VectorX<Expression> expression_vector(1);
expression_vector[0] = varable_vector[0] + 1;
options.parameterization =
    IrisParameterizationFunction(expression_vector, varable_vector);
DRAKE_EXPECT_THROWS_MESSAGE(
    IrisNp2(*sgcc_ptr, starting_ellipsoid_, domain_, options),
    ".*parameterized subspace.*");
options.parameterization = IrisParameterizationFunction();

And also a

// TODO(cohnt): Support user-specified parameterizations contained in IrisZoOptions and IrisNp2Options.

planning/iris/iris_from_clique_cover.cc line 441 at r1 (raw file):

    // Show the samples used in build cliques. Debugging visualization.
    auto meshcat_ptr = std::visit(

btw This snippet appears twice, and could conceivably be needed again elsewhere? Maybe an internal linkage function like GetMeshcatPtrFromOptions (or similarly-named) would be nice.

@cohnt cohnt linked an issue Jun 18, 2025 that may be closed by this pull request
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

Successfully merging this pull request may close these issues.

IRIS from Clique Cover Using IrisZo or IrisNp2
3 participants