Implements IRIS ZO

[wernerpe]

+@cohnt for feature review

---

This change is 

[cohnt]

Comments part 1 on the options and documentation.

Reviewed 2 of 8 files at r1, all commit messages.
Reviewable status: 21 unresolved discussions, LGTM missing from assignee cohnt, needs platform reviewer assigned, needs at least two assigned reviewers

---

-- commits line 2 at r1:
You'll want to change the final commit message before merging. Leaving a blocking comment here for that purpose.

---

planning/iris/iris_zo.h line 2 at r1 (raw file):

#pragma once

nit Not sure where this is appropriate, but can we tag this file as @experimental in doxygen?

---

planning/iris/iris_zo.h line 48 at r1 (raw file):

  IrisZoOptions() = default;

  /** Number of particles used to estimate the closest collision*/

nit Throughout the documentation, add periods at the end of sentences, and add a space before closing the comment.

---

planning/iris/iris_zo.h line 52 at r1 (raw file):

  /** Descision threshold for unadaptive test.*/
  double tau = 0.5;

Drop in the quote from the paper about how this parameter trades off between the statistical power of the test and the difficulty of passing it.

---

planning/iris/iris_zo.h line 55 at r1 (raw file):

  /** Upper bound on the error probability that the fraction-in-collision
   * `epsilon` is not met.*/

/** Upper bound on the probability the returned region has a fraction-in-collision greater than epsilon. */

Code quote:

  /** Upper bound on the error probability that the fraction-in-collision
   * `epsilon` is not met.*/

---

planning/iris/iris_zo.h line 62 at r1 (raw file):

  /** Points that are guaranteed to be contained in the final region
   * provided their convex hull is collision free.*/

nit Drop the leading asterisks.

---

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

   * c is the convex hull of the points passed in `containment_points`.
   */
  bool force_containment_points{false};

Maybe it would be a bit more natural to make containment_points an std::optional<Eigen::MatrixXd>? Currently, the behavior if the user provides points to be contained but doesn't flip this flag is unintuitive.

---

planning/iris/iris_zo.h line 71 at r1 (raw file):

  /** Number of resampling steps for the gradient updates*/
  // int num_resampling_steps = 1;

unused code?

---

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

  // int num_resampling_steps = 1;

  /** Number Iris Iterations*/

We have multiple notions of "iterations" now. Suggestion: Maximum number of bilinear alternations. or Maximum number of outer iterations.

Code quote:

Number Iris Iterations

---

planning/iris/iris_zo.h line 74 at r1 (raw file):

  /** Number Iris Iterations*/
  int max_iterations{2};

nit Should we set this default parameter to match IRIS-NP? Or do we have a new sense on what a good best practice for the number of outer iterations is?

---

planning/iris/iris_zo.h line 76 at r1 (raw file):

  int max_iterations{2};

  /** Maximum number of rounds of adding faces to the polytope*/

Maximum number of rounds of adding faces to the polytope per outer iteration (or bilinear alternation -- match above)

Code quote:

Maximum number of rounds of adding faces to the polytope

---

planning/iris/iris_zo.h line 79 at r1 (raw file):

  int max_iterations_separating_planes{20};

  /** Maximum number of faces to add per round of samples*/

Clarify if per outer iteration or inner iteration.

---

planning/iris/iris_zo.h line 86 at r1 (raw file):

  /** Parallelize the updates of the particles*/
  bool parallelize{true};

Should be a Parallelism object

Parallelism parallelism{Parallelism::Max()};

---

planning/iris/iris_zo.h line 88 at r1 (raw file):

  bool parallelize{true};

  /* Enables print statements indicating the progress of fast iris**/

Nit: comment style should be /** ... */.

---

planning/iris/iris_zo.h line 97 at r1 (raw file):

  /** We retreat by this margin from each C-space obstacle in order to avoid the
  possibility of requiring an infinite number of faces to approximate a curved
  boundary.

nit extra newline

---

planning/iris/iris_zo.h line 103 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
  be disabled by setting to a negative value. */

This termination condition can be disabled by setting to negative infinity.

(Note that if the initial ellipsoid is big, we can get a negative change in volume on the first iteration.)

Code quote:

  between iterations is less that this threshold. This termination condition can
  be disabled by setting to a negative value. */

---

planning/iris/iris_zo.h line 108 at r1 (raw file):

  /** IRIS will terminate if the change in the *volume* of the hyperellipsoid
  between iterations is less that this percent of the previous best volume.
  This termination condition can be disabled by setting to a negative value. */

ditto above

---

planning/iris/iris_zo.h line 112 at r1 (raw file):

  /** For IRIS in configuration space, it can be beneficial to not only specify
  task-space obstacles (passed in through the plant) but also obstacles that are

unused code?

---

planning/iris/iris_zo.h line 119 at r1 (raw file):

  /** The only randomization in IRIS is the random sampling done to find
  counter-examples for the additional constraints using in
  IrisInConfigurationSpace. Use this option to set the initial seed. */

/** Seed used for the random sampling throughout the algorithm. */

Code quote:

  /** The only randomization in IRIS is the random sampling done to find
  counter-examples for the additional constraints using in
  IrisInConfigurationSpace. Use this option to set the initial seed. */

---

planning/iris/iris_zo.h line 136 at r1 (raw file):

P. Werner, T. Cohn, R. H. Jiang, T. Seyde, M. Simchowitz, R. Tedrake, and D.
Rus, "Faster Algorithms for Growing Collision-Free Convex Polytopes in Robot
Configuration Space,"

P. Werner, T. Cohn*, R. H. Jiang*, T. Seyde, M. Simchowitz, R. Tedrake, and D.
Rus, "Faster Algorithms for Growing Collision-Free Convex Polytopes in Robot
Configuration Space,"
* Denotes equal contribution.

Code quote:

P. Werner, T. Cohn, R. H. Jiang, T. Seyde, M. Simchowitz, R. Tedrake, and D.
Rus, "Faster Algorithms for Growing Collision-Free Convex Polytopes in Robot
Configuration Space,"

---

planning/iris/iris_zo.h line 164 at r1 (raw file):

@ingroup robot_planning
*/

Is there a particular reason you want to pass the starting ellipsoid here, and not as part of the IrisZoOptions? I know that it implicitly contains the seed point, but for someone who doesn't need to leverage an unusual initial ellipsoid, it would be nice to implement default behavior. Alternatively, mention Hyperellipsoid::MakeHypersphere in the docstring.

[cohnt]

Comments part 2 on implementation

Reviewed 1 of 8 files at r1.
Reviewable status: 38 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) (waiting on @wernerpe)

---

planning/iris/iris_zo.cc line 47 at r1 (raw file):

}

Eigen::VectorXd compute_face_tangent_to_dist_cvxh(

nit Platform reviewer might complain about these abbreviations

---

planning/iris/iris_zo.cc line 53 at r1 (raw file):

  double b_face = a_face.transpose() * point;
  double worst_case =
      (a_face.transpose() * cvxh_vpoly.vertices()).maxCoeff() - b_face;

nit Since worst_case isn't used anywhere else, it's more readable to directly check the case here.

bool any_points_chopped_off = (a_face.transpose() * cvxh_vpoly.vertices()).maxCoeff() <= b_face;

Code quote:

  double worst_case =
      (a_face.transpose() * cvxh_vpoly.vertices()).maxCoeff() - b_face;

---

planning/iris/iris_zo.cc line 54 at r1 (raw file):

  double worst_case =
      (a_face.transpose() * cvxh_vpoly.vertices()).maxCoeff() - b_face;
  // Return standard iris face if either the face does not chopp off any

nit chopp

---

planning/iris/iris_zo.cc line 61 at r1 (raw file):

    MathematicalProgram prog;
    int dim = cvxh_vpoly.ambient_dimension();
    std::vector<solvers::SolverId> preferred_solvers{

Is there a reason Gurobi is specifically disliked here? It seems like we should be able to just use whatever default solver drake comes up with. Then you can just do auto result = solvers::Solve(prog);

---

planning/iris/iris_zo.cc line 76 at r1 (raw file):

}

int unadaptive_test_samples(double p, double delta, double tau) {

nit Document the equation number from the paper.

---

planning/iris/iris_zo.cc line 86 at r1 (raw file):

                   const HPolyhedron& domain, const IrisZoOptions& options) {
  auto start = std::chrono::high_resolution_clock::now();
  const auto parallelism = Parallelism::Max();

Right now, this ignores the option where the user specifies whether or not to use parallelism. If you add the level of parallelism as an option, then you can just replace parallelism with options.parallelism throughout.

---

planning/iris/iris_zo.cc line 87 at r1 (raw file):

  auto start = std::chrono::high_resolution_clock::now();
  const auto parallelism = Parallelism::Max();
  const int num_threads_to_use =

Can you document why the number of threads is more complicated than just pulling whatever number the user gives? Seems like we should just always be calling parallelism.num_threads() in place of this variable.

---

planning/iris/iris_zo.cc line 100 at r1 (raw file):

  Eigen::VectorXd current_ellipsoid_center = starting_ellipsoid.center();
  Eigen::MatrixXd current_ellipsoid_A = starting_ellipsoid.A();
  double previous_volume = 0;

Note that previous_volume is not the volume of the initial ellipsoid. This is actually a really nice change from IrisInConfigurationSpace, since it prevents the algorithm from trivially exiting after the first iteration if the user gave a poorly-scaled initial ellipsoid. Can we document that this is done intentionally? I've also retracted my above comments about the case of a negative termination threshold.

---

planning/iris/iris_zo.cc line 145 at r1 (raw file):

  std::vector<Eigen::VectorXd> particles;

  // upper bound on number of particles required if we hit max iterations

nit capitalization and period at the end of the sentence.

Also would be nice to cite the equation number from the paper

---

planning/iris/iris_zo.cc line 163 at r1 (raw file):

    log()->info("IrisZo worst case test requires {} samples.", N_max);
  }

Can define the particles vector here with RAII

std::vector<Eigen::VectorXd> particles(N_max, Eigen::VectorXd::Zero(dim));

---

planning/iris/iris_zo.cc line 173 at r1 (raw file):

  HPolyhedron P_prev = domain;

  // pre-allocate memory for the polyhedron we are going to construct

nit capitalization and period at the end of the sentence.

---

planning/iris/iris_zo.cc line 179 at r1 (raw file):

  Eigen::VectorXd b(P.A().rows() + 300);

  //   if (options.verbose) {

Uncomment or remove.

---

planning/iris/iris_zo.cc line 185 at r1 (raw file):

  while (true) {
    log()->info("IrisZo iteration {}", iteration);

Clarify the type of iteration (outer vs inner), or call it a bilinear alternation

---

planning/iris/iris_zo.cc line 188 at r1 (raw file):

    Eigen::MatrixXd ATA = current_ellipsoid_A.transpose() * current_ellipsoid_A;
    // rescaling makes max step computations more stable

nit capitalization and period at the end of the sentence. Not going to copy paste this comment, but make the change throughout.

---

planning/iris/iris_zo.cc line 200 at r1 (raw file):

    // track maximum relaxation of cspace margin if containment of points is
    // requested
    double max_relaxation = 0;

If the cspace margin gets relaxed when we request containment of a set of points, it should be documented. Unless I missed the documentation earlier.

---

planning/iris/iris_zo.cc line 279 at r1 (raw file):

      }

      const auto particle_update_work =

nit Would be helpful to document at the start of this lambda roughly what the procedure is. Something like

For each particle in collision, we run a bisection search to find a configuration on the boundary of the obstacle.

---

planning/iris/iris_zo.cc line 314 at r1 (raw file):

              }
            }
            //}

Leftover close brace?

---

planning/iris/iris_zo.cc line 358 at r1 (raw file):

              options.containment_points.size()) {
            a_face = compute_face_tangent_to_dist_cvxh(
                current_ellipsoid, nearest_particle, cvxh_vpoly);

Remove old debugging code.

---

planning/iris/iris_zo.cc line 449 at r1 (raw file):

      }
      ++num_iterations_separating_planes;
      if (num_iterations_separating_planes -

Not exactly sure what's going on here -- is this making it so it only logs on certain inner iterations? Probably should drop a comment explaining.

[cohnt]

Comments part 3 on python bindings and tests

Reviewed 3 of 8 files at r1, 3 of 3 files at r2, all commit messages.
Reviewable status: 41 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) (waiting on @wernerpe)

---

planning/iris/test/iris_zo_test.cc line 57 at r2 (raw file):

  planning::SceneGraphCollisionChecker checker(std::move(params));
  // plant.SetPositions(&plant.GetMyMutableContextFromRoot(context.get()),

nit remove unused code

---

planning/iris/test/iris_zo_test.cc line 380 at r2 (raw file):

  Hyperellipsoid starting_ellipsoid =
      Hyperellipsoid::MakeHypersphere(1e-2, sample);
  // std::this_thread::sleep_for(std::chrono::milliseconds(100));

nit remove unused code?

---

bindings/pydrake/planning/planning_py_iris_zo.cc line 92 at r2 (raw file):

  m.def("IrisZo", &IrisZo, py::arg("checker"), py::arg("starting_ellipsoid"),
      py::arg("domain"), py::arg("options") = IrisZoOptions(), doc.IrisZo.doc);

Since the method uses parallelization, it needs py::call_guard<py::gil_scoped_release>()

[wernerpe]

Reviewable status: 26 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)

---

planning/iris/iris_zo.h line 52 at r1 (raw file):

Previously, cohnt (Thomas Cohn) wrote…

Drop in the quote from the paper about how this parameter trades off between the statistical power of the test and the difficulty of passing it.

Done.

---

planning/iris/iris_zo.h line 55 at r1 (raw file):

Previously, cohnt (Thomas Cohn) wrote…

/** Upper bound on the probability the returned region has a fraction-in-collision greater than epsilon. */

Done.

---

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

Previously, cohnt (Thomas Cohn) wrote…

Maybe it would be a bit more natural to make containment_points an std::optional<Eigen::MatrixXd>? Currently, the behavior if the user provides points to be contained but doesn't flip this flag is unintuitive.

Done.

---

planning/iris/iris_zo.h line 71 at r1 (raw file):

Previously, cohnt (Thomas Cohn) wrote…

unused code?

Done.

---

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

Previously, cohnt (Thomas Cohn) wrote…

We have multiple notions of "iterations" now. Suggestion: Maximum number of bilinear alternations. or Maximum number of outer iterations.

Done.

---

planning/iris/iris_zo.h line 79 at r1 (raw file):

Previously, cohnt (Thomas Cohn) wrote…

Clarify if per outer iteration or inner iteration.

Done.

---

planning/iris/iris_zo.h line 86 at r1 (raw file):

Previously, cohnt (Thomas Cohn) wrote…

Should be a Parallelism object

Parallelism parallelism{Parallelism::Max()};

Done.

---

planning/iris/iris_zo.h line 112 at r1 (raw file):

Previously, cohnt (Thomas Cohn) wrote…

unused code?

Done.

---

planning/iris/iris_zo.h line 119 at r1 (raw file):

Previously, cohnt (Thomas Cohn) wrote…

/** Seed used for the random sampling throughout the algorithm. */

Done.

---

planning/iris/iris_zo.h line 136 at r1 (raw file):

Previously, cohnt (Thomas Cohn) wrote…

P. Werner, T. Cohn*, R. H. Jiang*, T. Seyde, M. Simchowitz, R. Tedrake, and D.
Rus, "Faster Algorithms for Growing Collision-Free Convex Polytopes in Robot
Configuration Space,"
* Denotes equal contribution.

Done.

---

planning/iris/iris_zo.h line 164 at r1 (raw file):

Previously, cohnt (Thomas Cohn) wrote…

Is there a particular reason you want to pass the starting ellipsoid here, and not as part of the IrisZoOptions? I know that it implicitly contains the seed point, but for someone who doesn't need to leverage an unusual initial ellipsoid, it would be nice to implement default behavior. Alternatively, mention Hyperellipsoid::MakeHypersphere in the docstring.

Done.

---

planning/iris/iris_zo.cc line 61 at r1 (raw file):

Previously, cohnt (Thomas Cohn) wrote…

Is there a reason Gurobi is specifically disliked here? It seems like we should be able to just use whatever default solver drake comes up with. Then you can just do auto result = solvers::Solve(prog);

Done.

---

planning/iris/iris_zo.cc line 86 at r1 (raw file):

Previously, cohnt (Thomas Cohn) wrote…

Right now, this ignores the option where the user specifies whether or not to use parallelism. If you add the level of parallelism as an option, then you can just replace parallelism with options.parallelism throughout.

Done.

---

planning/iris/iris_zo.cc line 87 at r1 (raw file):

Previously, cohnt (Thomas Cohn) wrote…

Can you document why the number of threads is more complicated than just pulling whatever number the user gives? Seems like we should just always be calling parallelism.num_threads() in place of this variable.

Done.

---

planning/iris/iris_zo.cc line 100 at r1 (raw file):

Previously, cohnt (Thomas Cohn) wrote…

Note that previous_volume is not the volume of the initial ellipsoid. This is actually a really nice change from IrisInConfigurationSpace, since it prevents the algorithm from trivially exiting after the first iteration if the user gave a poorly-scaled initial ellipsoid. Can we document that this is done intentionally? I've also retracted my above comments about the case of a negative termination threshold.

Done.

---

planning/iris/iris_zo.cc line 163 at r1 (raw file):

Previously, cohnt (Thomas Cohn) wrote…

Can define the particles vector here with RAII

std::vector<Eigen::VectorXd> particles(N_max, Eigen::VectorXd::Zero(dim));

Done.

---

planning/iris/iris_zo.cc line 179 at r1 (raw file):

Previously, cohnt (Thomas Cohn) wrote…

Uncomment or remove.

Done.

---

planning/iris/iris_zo.cc line 185 at r1 (raw file):

Previously, cohnt (Thomas Cohn) wrote…

Clarify the type of iteration (outer vs inner), or call it a bilinear alternation

Done.

---

planning/iris/iris_zo.cc line 200 at r1 (raw file):

Previously, cohnt (Thomas Cohn) wrote…

If the cspace margin gets relaxed when we request containment of a set of points, it should be documented. Unless I missed the documentation earlier.

Done added comment in the header.

---

planning/iris/iris_zo.cc line 314 at r1 (raw file):

Previously, cohnt (Thomas Cohn) wrote…

Leftover close brace?

Done.

---

planning/iris/iris_zo.cc line 358 at r1 (raw file):

Previously, cohnt (Thomas Cohn) wrote…

Remove old debugging code.

Done.

---

planning/iris/iris_zo.cc line 449 at r1 (raw file):

Previously, cohnt (Thomas Cohn) wrote…

Not exactly sure what's going on here -- is this making it so it only logs on certain inner iterations? Probably should drop a comment explaining.

Done.

[wernerpe]

Reviewable status: 26 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)

---

bindings/pydrake/planning/planning_py_iris_zo.cc line 92 at r2 (raw file):

Previously, cohnt (Thomas Cohn) wrote…

Since the method uses parallelization, it needs py::call_guard<py::gil_scoped_release>()

Done.

[cohnt]

Reviewed 1 of 4 files at r3, 5 of 5 files at r4, all commit messages.
Reviewable status: 9 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) (waiting on @wernerpe)

---

planning/iris/iris_zo.cc line 87 at r1 (raw file):

Previously, wernerpe (Peter Werner) wrote…

Done.

I still don't understand why we need the num_threads_to_use variable. Is the concern that a collision checker might not support parallelism, so we need to avoid multithreading in that case? If so, I think we should throw an error -- this is probably a mistake by the user.

If my understanding is correct, I suggest:

if (!checker.SupportsParallelChecking() && options.parallelism.num_threads() > 1) {
    throw std::runtime_error("User requested to use multiple threads, but their collision checker doesn't support multithreading.");
}

and then just always use options.parallelism.num_threads().

---

planning/iris/iris_zo.h line 35 at r4 (raw file):

    a->Visit(DRAKE_NVP(max_separating_planes_per_iteration));
    a->Visit(DRAKE_NVP(bisection_steps));
    // a->Visit(DRAKE_NVP(parallelism));

nit I think we can delete this outright -- I don't think it's possible to serialize a Parallelism object.

---

planning/iris/iris_zo.h line 53 at r4 (raw file):

   * increases both the cost and the power statistical test. Increasing the
   * value of `tau` makes running an individual test cheaper but decreases its
   * power to accept a polytope. We chosing a value of 0.5 is a good

nit We chosing

---

planning/iris/iris_zo.h line 68 at r4 (raw file):

   * provided their convex hull is collision free. Note that if the containment
   * points are closer than configuration_margin to an obstacle we will relax
   * the margin in favor of including the containment points.*/

Should the containment_points be passed in row-major or column-major form? Document the expected shape.

---

planning/iris/iris_zo.h line 111 at r4 (raw file):

  double relative_termination_threshold{1e-3};

  /** This option to sets the random seed for random sampling throughout the

nit "This option to sets"

---

planning/iris/iris_zo.h line 130 at r4 (raw file):

Rus, "Faster Algorithms for Growing Collision-Free Convex Polytopes in Robot
Configuration Space,"
\* Denotes equal contribution.

Apparently doxygen is weird with escaping asterisks if they lead the line. So start the line with  * Denotes equal contribution. instead.

---

bindings/pydrake/planning/BUILD.bazel line 134 at r4 (raw file):

)

# drake_py_unittest(

Should this still be commented out?

[cohnt]

Reviewable status: 10 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) (waiting on @wernerpe)

---

planning/iris/test/iris_zo_test.cc line 527 at r6 (raw file):

      options.meshcat->SetObject(path, Sphere(0.04),
                                 geometry::Rgba(1, 0, 0.0, 1.0));
      point_to_draw.head(2) = cont_points.col(pt_to_draw);

This is where you're getting the error from, since cont_points has 3 rows. I think this should be

point_to_draw.head(2) = cont_points.col(pt_to_draw).head(2);

Code quote:

point_to_draw.head(2) = cont_points.col(pt_to_draw);

[cohnt]

Reviewable status: 11 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) (waiting on @wernerpe)

---

planning/iris/iris_zo.cc line 399 at r6 (raw file):

          particle_is_redundant.at(i) = true;

// loop over remaining non-redundant particles and check for

btw How much time does this parallelization actually save? I don't have a good sense of how fast OMP multithreading is. But for such a simple computation, I would think the overhead of spinning up the threads might be enough that it's faster to just do the matrix multiplication and grab the nonzero indices out of it.

[wernerpe]

Reviewable status: 8 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)

---

planning/iris/iris_zo.h line 68 at r4 (raw file):

Previously, cohnt (Thomas Cohn) wrote…

Should the containment_points be passed in row-major or column-major form? Document the expected shape.

Done.

---

planning/iris/iris_zo.h line 130 at r4 (raw file):

Previously, cohnt (Thomas Cohn) wrote…

Apparently doxygen is weird with escaping asterisks if they lead the line. So start the line with  * Denotes equal contribution. instead.

Done.

---

planning/iris/iris_zo.cc line 87 at r1 (raw file):

Previously, cohnt (Thomas Cohn) wrote…

I still don't understand why we need the num_threads_to_use variable. Is the concern that a collision checker might not support parallelism, so we need to avoid multithreading in that case? If so, I think we should throw an error -- this is probably a mistake by the user.

If my understanding is correct, I suggest:

if (!checker.SupportsParallelChecking() && options.parallelism.num_threads() > 1) {
    throw std::runtime_error("User requested to use multiple threads, but their collision checker doesn't support multithreading.");
}

and then just always use options.parallelism.num_threads().

Done.

---

planning/iris/iris_zo.cc line 399 at r6 (raw file):

Previously, cohnt (Thomas Cohn) wrote…

btw How much time does this parallelization actually save? I don't have a good sense of how fast OMP multithreading is. But for such a simple computation, I would think the overhead of spinning up the threads might be enough that it's faster to just do the matrix multiplication and grab the nonzero indices out of it.

The timing seems to be almost identical, removed it for now

---

planning/iris/test/iris_zo_test.cc line 527 at r6 (raw file):

Previously, cohnt (Thomas Cohn) wrote…

This is where you're getting the error from, since cont_points has 3 rows. I think this should be

point_to_draw.head(2) = cont_points.col(pt_to_draw).head(2);

Done.

---

bindings/pydrake/planning/BUILD.bazel line 134 at r4 (raw file):

Previously, cohnt (Thomas Cohn) wrote…

Should this still be commented out?

Done.

[cohnt]

Reviewed 1 of 3 files at r5, 2 of 3 files at r7, 1 of 1 files at r8, all commit messages.
Reviewable status: 4 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) (waiting on @wernerpe)

---

bindings/pydrake/planning/BUILD.bazel line 134 at r4 (raw file):

Previously, wernerpe (Peter Werner) wrote…

Done.

This is still commented out in this PR. I think it needs to not be commented?

---

planning/iris/iris_zo.h line 87 at r8 (raw file):

  int bisection_steps{10};

  /** Parallelize the updates of the particles. */

Can you add a note here that we cannot use higher parallelism than the collision checker supports? Something like

/** Number of threads to use when updating the particles. If the user requests more threads than the CollisionChecker supports, that number of threads will be used instead. */