Reviewable status: 3 unresolved discussions, needs platform reviewer assigned, needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @cohnt)

planning/iris/iris_entrypoint.h line 20 at r1 (raw file):

  NP,      ///< The IRIS-NP algorithm, which uses nonlinear programming to grow
           ///< regions in configuration space.
  NP2,     ///< The IRIS-NP2 algorithm, an improved version of IRIS-NP.

perhaps "The IRIS-NP2 algorithm, which uses sampling to improve nonlinear search to grow regions in configuration space"

planning/iris/iris_entrypoint.h line 99 at r1 (raw file):

  /** IRIS will terminate if the change in the *volume* of the hyperellipsoid
  between iterations is less that this threshold. This termination condition can

typo: less than

planning/iris/iris_entrypoint.h line 230 at r1 (raw file):

  \b Algorithms: NP, NP2, ZO */
  int mixing_steps{10};

I think 50 would be better!

I am going to advocate for a very different design. Each algorithm should be a class

class IrisRegionBuilder:
public:
  IrisRegionBuilder(const CollisionChecker& checker);
  HPolyhedron BuildRegion();

protected:
  HPolyhedron DoBuildRegion();

I think the differences between the algorithms are too large to be a monolith.

1. Some algorithms take an initial seed and ellipse to start. CliqueInflation take a collection of points. C-Iris takes an initial polytope.
2. There are too many options distinct to each algorithm that just the option file will get very large and unwieldy to navigate.
3. Doing all the necessary pre-condition checking for each algorithm is also going to bloat the single entry point.
4. Most of the time we want to build multiple regions. Having a class-based interface avoids having to run the setup code for each individual call to the build region function.l

Having the single base class however is useful since downstream code which doesn't care how a region is built can just consume an IrisRegionBuilder.

The main design question I have in this setup is finding a clean way to tell different algorithms where to build the next region since this input is different across different algorithms.

Reviewable status: 12 unresolved discussions, needs platform reviewer assigned, needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @cohnt)

planning/iris/iris_entrypoint.h line 11 at r1 (raw file):

#include "drake/geometry/optimization/hyperellipsoid.h"
#include "drake/planning/collision_checker.h"

Tag @experimental so that we can update without deprecation.

planning/iris/iris_entrypoint.h line 16 at r1 (raw file):

enum class IrisAlgorithm {
  Unset,   ///< Default value -- must be changed by the user.

Do you need to have an unset enum?

planning/iris/iris_entrypoint.h line 68 at r1 (raw file):

/**
 */
struct IrisOptions {

I don't like this monolithic IrisOptions object. The approach of solver options is to have a map from options to values that each solver individually can make sense of. Some common options are handled by a specific enum.

My preference is to makeIrisOptions a base class that each algorithm extends. The IrisOptionsImplementation subclasses should each have a virtual method Run() which actually computes the region.

The way one flags which algorithm they which to run in those cases is determined entirely by which set of options they use.

planning/iris/iris_entrypoint.h line 73 at r1 (raw file):

  /** TODO(cohnt): Document */
  IrisRegionSpace region_space{IrisRegionSpace::Unset};

I would put these in the function handle of the entry point. Users must set these so they are not optional kwargs.

Code quote:

  /** TODO(cohnt): Document */
  IrisAlgorithm algorithm{IrisAlgorithm::Unset};

  /** TODO(cohnt): Document */
  IrisRegionSpace region_space{IrisRegionSpace::Unset};

planning/iris/iris_entrypoint.h line 91 at r1 (raw file):

  \b Algorithms: All */
  bool require_sample_point_is_contained{false};

Some algorithsm (C-IRIS, CliqueInflation) can force this condition explicitly so this documentation isn't quite right.

Code quote:

  /** The polytope produced by the first iteration of each algorithm is
  guaranteed to contain the seed point, as long as that point is collision-free.
  However, subsequent iterations do not make this guarantee, so the IRIS paper
  recommends that if containment is a requirement, then the algorithm should
  simply terminate early if alternations would ever cause the set to not contain
  the point.

  \b Algorithms: All */
  bool require_sample_point_is_contained{false};

planning/iris/iris_entrypoint.h line 137 at r1 (raw file):

  \b Algorithms: All */
  geometry::optimization::ConvexSets configuration_obstacles{};

C-Iris currently does not handle Cspace obstacles.

Code quote:

  \b Algorithms: All */
  geometry::optimization::ConvexSets configuration_obstacles{};

planning/iris/iris_entrypoint.h line 144 at r1 (raw file):

  \b Algorithms: All */
  std::optional<geometry::optimization::Hyperellipsoid> starting_ellipse{};

C-Iris does not use an initial ellipse but an initial polytope.

Code quote:

  \b Algorithms: All */
  std::optional<geometry::optimization::Hyperellipsoid> starting_ellipse{};

planning/iris/iris_entrypoint.h line 249 at r1 (raw file):

geometry::optimization::HPolyhedron GrowIrisRegion(
    const CollisionChecker& checker, const IrisOptions& options,
    const Eigen::VectorXd seed);

This isn't exactly what you want. The initial point for C-Iris is actually a region not a seed point. For CliqueInflation it would be collection of points.

Code quote:

    const Eigen::VectorXd seed);

planning/iris/iris_entrypoint.cc line 27 at r1 (raw file):

  } else if (options.algorithm == IrisAlgorithm::Certified) {
    throw std::runtime_error("IrisAlgorithm::Certified is not supported yet.");
  } else if (options.algorithm == IrisAlgorithm::NP) {

This is going to become a massive and messy file as we add more algorithms. It would be good to have a mechanism for separating the different implementations across files.

Code quote:

  if (options.algorithm == IrisAlgorithm::Convex) {
    throw std::runtime_error("IrisAlgorithm::Convex is not supported yet.");
  } else if (options.algorithm == IrisAlgorithm::NP2) {
    throw std::runtime_error("IrisAlgorithm::NP2 is not supported yet.");
  } else if (options.algorithm == IrisAlgorithm::ZO) {
    throw std::runtime_error("IrisAlgorithm::ZO is not supported yet.");
  } else if (options.algorithm == IrisAlgorithm::Certified) {
    throw std::runtime_error("IrisAlgorithm::Certified is not supported yet.");
  } else if (options.algorithm == IrisAlgorithm::NP) {