Cspace free polytope bindings non dev #19574

AlexandreAmice · 2023-06-11T18:41:27Z

This change is

AlexandreAmice

+@hongkai-dai This is a checkpoint on all the bindings, but its rather large so thought I would give you some extra time to nit pick stuff.

Reviewable status: LGTM missing from assignee hongkai-dai, 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)

hongkai-dai

checkpoint.

Reviewed 3 of 5 files at r1.
Reviewable status: 2 unresolved discussions, LGTM missing from assignee hongkai-dai, 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)

bindings/pydrake/geometry/geometry_py_optimization.cc line 44 at r1 (raw file):

  using BaseClass = geometry::optimization::CSpaceSeparatingPlane<T>;
  constexpr auto& base_cls_doc = doc.CSpaceSeparatingPlane;
  using CIrisClass = geometry::optimization::CIrisSeparatingPlane<T>;

Since we are going to remove CIrisSeparatingPlane very soon, I think it is better not to bind it?

bindings/pydrake/geometry/geometry_py_optimization.cc line 768 at r1 (raw file):

    // Definitions for cspace_separating_plane.h/cc and
    // c_iris_separating_plane.h/cc
    py::enum_<SeparatingPlaneOrder>(

I think we are going to remove this enum soon?

AlexandreAmice

This latest push just rebases to reconcile the bindings with the introduction of CspaceFreePolytopeBase. I still have a lot of tests to write, but I am trying to get out ahead of this so that it does not become a blocker for another collaboration.

Let me know if this is too much to review at once!

Reviewable status: 2 unresolved discussions, LGTM missing from assignee hongkai-dai, 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)

bindings/pydrake/geometry/geometry_py_optimization.cc line 44 at r1 (raw file):

Previously, hongkai-dai (Hongkai Dai) wrote…

Since we are going to remove CIrisSeparatingPlane very soon, I think it is better not to bind it?

I need to leave it as otherwise I cannot bind CspaceFreePolytope::separating_planes

bindings/pydrake/geometry/geometry_py_optimization.cc line 768 at r1 (raw file):

Previously, hongkai-dai (Hongkai Dai) wrote…

I think we are going to remove this enum soon?

I need to leave it until we remove the enum otherwise it is impossible to construct a CspaceFreePolytope object.

hongkai-dai

Checkpoint. I am surprised that binding SortedPair doesn't work. I asked @EricCousineau-TRI to chime in.

Reviewed 1 of 5 files at r2.
Reviewable status: 3 unresolved discussions, LGTM missing from assignee hongkai-dai, 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)

bindings/pydrake/geometry/geometry_py_optimization.cc line 44 at r1 (raw file):

Previously, AlexandreAmice (Alexandre Amice) wrote…

I need to leave it as otherwise I cannot bind CspaceFreePolytope::separating_planes

I see, that makes sense!

BTW, I will file a PR to change the type of separating_planes() to CSpaceSeparatingPlane soon.

bindings/pydrake/geometry/geometry_py_optimization.cc line 768 at r1 (raw file):

Previously, AlexandreAmice (Alexandre Amice) wrote…

I need to leave it until we remove the enum otherwise it is impossible to construct a CspaceFreePolytope object.

I see.

bindings/pydrake/geometry/geometry_py_optimization.cc line 808 at r2 (raw file):

  }
  {
    using PolytopeBaseClass = CspaceFreePolytopeBase;

Here we bind some of the method in the base class (such as CspaceFreePolytopeBase::map_geometries_to_separating_planes() as child class method. Could you explain why not to bind it as a base class method? Later when we add python binding for CspaceFreeBox which is also derived from CspaceFreePolytopeBase, we will need to define the binding for these method again if they are not in the base class.

bindings/pydrake/geometry/geometry_py_optimization.cc line 826 at r2 (raw file):

                 const Class::Options&>(),
            py::arg("plant"), py::arg("scene_graph"), py::arg("plane_order"),
            py::arg("q_star"), py::arg("options") = Class::Options(),

Should also have py::keep_alive<1, 2>() which keeps plant alive?

bindings/pydrake/geometry/geometry_py_optimization.cc line 838 at r2 (raw file):

            "map_geometries_to_separating_planes",
            [](const CspaceFreePolytope* self) {
              // Template deduction for drake::SortedPair<GeometryId> does not

Interesting. I saw that you have sored_pair_pybind.h in the include headers already and it still doesn't work? @EricCousineau-TRI I thought sorted_pair_pybind.h should handle this?

EricCousineau-TRI

Reviewable status: 3 unresolved discussions, LGTM missing from assignee hongkai-dai, 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)

bindings/pydrake/geometry/geometry_py_optimization.cc line 838 at r2 (raw file):

Previously, hongkai-dai (Hongkai Dai) wrote…

Interesting. I saw that you have sored_pair_pybind.h in the include headers already and it still doesn't work? @EricCousineau-TRI I thought sorted_pair_pybind.h should handle this?

Working: It should. Let me see.

EricCousineau-TRI

Reviewable status: 4 unresolved discussions, LGTM missing from assignee hongkai-dai, 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)

bindings/pydrake/geometry/geometry_py_optimization.cc line 1032 at r2 (raw file):

            &Class::BilinearAlternationOptions::find_polytope_options,
            cls_doc.BilinearAlternationOptions.find_polytope_options.doc)
        .def_readwrite("find_lagrangian_options",

I believe compilation errors are occurring because this struct does not have operator= for copying.

Some possible solutions:

Define operator= for just the final class (I'm not sure if this would jive w/ styleguie)
Use .def_readonly, where people can access the value, but can't assign over it (same as in C++).

EricCousineau-TRI

Reviewable status: 4 unresolved discussions, LGTM missing from assignee hongkai-dai, 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)

bindings/pydrake/geometry/geometry_py_optimization.cc line 838 at r2 (raw file):

Previously, EricCousineau-TRI (Eric Cousineau) wrote…

Working: It should. Let me see.

Yeah, there was a bug: AlexandreAmice#74

Would you be OK merging this in? I'll timebox an attempt to make a separate PR w/ proper testing.

AlexandreAmice

Reviewable status: 4 unresolved discussions, LGTM missing from assignee hongkai-dai, 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

bindings/pydrake/geometry/geometry_py_optimization.cc line 808 at r2 (raw file):

Previously, hongkai-dai (Hongkai Dai) wrote…

Here we bind some of the method in the base class (such as CspaceFreePolytopeBase::map_geometries_to_separating_planes() as child class method. Could you explain why not to bind it as a base class method? Later when we add python binding for CspaceFreeBox which is also derived from CspaceFreePolytopeBase, we will need to define the binding for these method again if they are not in the base class.

I am not sure I understand. Are you suggesting that I bind CspaceFreePolytopeBase separately? If so, I am not sure that CspaceFreePolytope actually would inherit those bindings. I believe I tried this in the past, but I may have done something wrong.

@EricCousineau-TRI do you know if we can subclass in this manner?

bindings/pydrake/geometry/geometry_py_optimization.cc line 826 at r2 (raw file):

Previously, hongkai-dai (Hongkai Dai) wrote…

Should also have py::keep_alive<1, 2>() which keeps plant alive?

Good call.

bindings/pydrake/geometry/geometry_py_optimization.cc line 838 at r2 (raw file):

Previously, EricCousineau-TRI (Eric Cousineau) wrote…

Yeah, there was a bug: AlexandreAmice#74

Would you be OK merging this in? I'll timebox an attempt to make a separate PR w/ proper testing.

I appreciate the fix thanks!

bindings/pydrake/geometry/geometry_py_optimization.cc line 1032 at r2 (raw file):

Previously, EricCousineau-TRI (Eric Cousineau) wrote…

I believe compilation errors are occurring because this struct does not have operator= for copying.

Some possible solutions:

Define operator= for just the final class (I'm not sure if this would jive w/ styleguie)

Use .def_readonly, where people can access the value, but can't assign over it (same as in C++).

Thanks! Will touch this up soon.

EricCousineau-TRI

Reviewable status: 4 unresolved discussions, LGTM missing from assignee hongkai-dai, 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)

bindings/pydrake/geometry/geometry_py_optimization.cc line 838 at r2 (raw file):

Previously, AlexandreAmice (Alexandre Amice) wrote…

I appreciate the fix thanks!

Working: We should wait until #19753 lands

EricCousineau-TRI

Reviewable status: 4 unresolved discussions, LGTM missing from assignee hongkai-dai, 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)

bindings/pydrake/geometry/geometry_py_optimization.cc line 808 at r2 (raw file):

Previously, AlexandreAmice (Alexandre Amice) wrote…

I am not sure I understand. Are you suggesting that I bind CspaceFreePolytopeBase separately? If so, I am not sure that CspaceFreePolytope actually would inherit those bindings. I believe I tried this in the past, but I may have done something wrong.

@EricCousineau-TRI do you know if we can subclass in this manner?

Yeah, you do py::class_<MyBase>(m, "MyBase"), then later on you do py::class_<MyChild, MyBase>(m, "MyChild"):
https://pybind11.readthedocs.io/en/stable/classes.html#inheritance-and-automatic-downcasting

hongkai-dai

Reviewable status: 5 unresolved discussions, LGTM missing from assignee hongkai-dai, 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)

bindings/pydrake/geometry/geometry_py_optimization.cc line 67 at r2 (raw file):

  {
    auto cls =
        DefineTemplateClassWithDefault<CIrisClass>(

BTW, I filed the PR https://reviewable.io/reviews/RobotLocomotion/drake/19756#- to deprecate this class.

EricCousineau-TRI

Reviewable status: 5 unresolved discussions, LGTM missing from assignee hongkai-dai, 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)

bindings/pydrake/geometry/geometry_py_optimization.cc line 838 at r2 (raw file):

Previously, EricCousineau-TRI (Eric Cousineau) wrote…

Working: We should wait until #19753 lands

This has landed - could you rebase / merge latest master here? You may get merge conflict on the sorted_pair_pybind.h - you should take in master's changes

AlexandreAmice

This one should finally be ready for a complete review.

Reviewable status: 5 unresolved discussions, LGTM missing from assignee hongkai-dai, 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

bindings/pydrake/geometry/geometry_py_optimization.cc line 808 at r2 (raw file):

Previously, EricCousineau-TRI (Eric Cousineau) wrote…

Yeah, you do py::class_<MyBase>(m, "MyBase"), then later on you do py::class_<MyChild, MyBase>(m, "MyChild"):
https://pybind11.readthedocs.io/en/stable/classes.html#inheritance-and-automatic-downcasting

Ah I see. I had originally done the multiple inheritence of <MyChild, MyBase> but got strange errors since I din't actually make the base class.

bindings/pydrake/geometry/geometry_py_optimization.cc line 838 at r2 (raw file):

Previously, EricCousineau-TRI (Eric Cousineau) wrote…

This has landed - could you rebase / merge latest master here? You may get merge conflict on the sorted_pair_pybind.h - you should take in master's changes

Done.

AlexandreAmice

@EricCousineau-TRI I left a few areas I could use a little bit of help with, but please feel free to pass off if you are not the best person to help.

Reviewable status: 9 unresolved discussions, LGTM missing from assignee hongkai-dai, 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)

bindings/pydrake/geometry/BUILD.bazel line 115 at r5 (raw file):

drake_py_unittest(
    name = "optimization_test",
    timeout = "long",

See discussion test_CspaceFreePolytopeMethods

bindings/pydrake/geometry/geometry_py_optimization.cc line 67 at r2 (raw file):

Previously, hongkai-dai (Hongkai Dai) wrote…

BTW, I filed the PR https://reviewable.io/reviews/RobotLocomotion/drake/19756#- to deprecate this class.

Sounds good. Will take these out once that PR lands.

bindings/pydrake/geometry/geometry_py_optimization.cc line 796 at r5 (raw file):

            .def_readonly("result", &SeparationCertificateResultBase::result);

    // TODO(Alexandre.Amice) decide whether to bind this based on a discussion

@hongkai-dai I am incline to not bind this at all. I am not even sure how I would test the binding given that the PlaneSeparatesGeometry objects of CspaceFreePolytope aren't publically accessible at all.

bindings/pydrake/geometry/geometry_py_optimization.cc line 831 at r5 (raw file):

        m, "CspaceFreePolytopeBase", base_cls_doc.doc);
    cspace_free_polytope_base_cls
        // TODO(Alexandre.Amice): Figure out how to bind rational_forward_kin.

@EricCousineau-TRI I could use come help binding this. When I bind it, I get the error "Unable to convert to Python type" which is strange because RationalForwardKinematics is bound. I think it is likely due to the complexities of the interfacing C++ and Python copying.

bindings/pydrake/geometry/test/optimization_test.py line 905 at r5 (raw file):

            mut.CspaceFreePolytope.\
                FindSeparationCertificateGivenPolytopeOptions()
        lagrangian_options.solver_id = CsdpSolver.id()

This makes the test take extremely long. With Mosek, the test is much faster and we do not need to bump up the timeout. I am not sure how to filter this to only run if Mosek is built and whether this is desirable @EricCousineau-TRI

hongkai-dai

Reviewed 2 of 4 files at r5.
Reviewable status: 11 unresolved discussions, LGTM missing from assignee hongkai-dai, 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)

geometry/optimization/cspace_free_structs.h line 44 at r5 (raw file):

struct FindSeparationCertificateOptions {
  DRAKE_NO_COPY_NO_MOVE_NO_ASSIGN(FindSeparationCertificateOptions)

Why removing the protected copy and move operators?

bindings/pydrake/geometry/geometry_py_optimization.cc line 50 at r5 (raw file):

        DefineTemplateClassWithDefault<BaseClass>(
            m, "CSpaceSeparatingPlane", param, base_cls_doc.doc)
            .def_readonly("a", &BaseClass::a, py_rvp::copy, base_cls_doc.a.doc)

Curious why we do copy here py_rvp::copy. Does py_rvp::reference_internal not work here?

bindings/pydrake/geometry/geometry_py_optimization.cc line 51 at r5 (raw file):

            m, "CSpaceSeparatingPlane", param, base_cls_doc.doc)
            .def_readonly("a", &BaseClass::a, py_rvp::copy, base_cls_doc.a.doc)
            .def_readonly("b", &BaseClass::b, base_cls_doc.b.doc)

What is the return strategy here? Also py_rvp::copy?

bindings/pydrake/geometry/geometry_py_optimization.cc line 61 at r5 (raw file):

                base_cls_doc.expressed_body.doc)
            .def_readonly("decision_variables", &BaseClass::decision_variables,
                py_rvp::copy, base_cls_doc.a.doc);

Again, curious why we cannot return by reference.

bindings/pydrake/geometry/geometry_py_optimization.cc line 796 at r5 (raw file):

Previously, AlexandreAmice (Alexandre Amice) wrote…

@hongkai-dai I am incline to not bind this at all. I am not even sure how I would test the binding given that the PlaneSeparatesGeometry objects of CspaceFreePolytope aren't publically accessible at all.

That sounds good to me.

AlexandreAmice

Reviewable status: 11 unresolved discussions, LGTM missing from assignee hongkai-dai, 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)

geometry/optimization/cspace_free_structs.h line 44 at r5 (raw file):

Previously, hongkai-dai (Hongkai Dai) wrote…

Why removing the protected copy and move operators?

I don't believe I changed anything in this file ultimately?

bindings/pydrake/geometry/geometry_py_optimization.cc line 50 at r5 (raw file):

Previously, hongkai-dai (Hongkai Dai) wrote…

Curious why we do copy here py_rvp::copy. Does py_rvp::reference_internal not work here?

Eigen matrices of objects get cast to numpy.ndarray which must be copied. If you use either py_rvp::reference or py_rvp::reference_internal you get a the error RuntimeError: dtype=object arrays must be copied, and cannot be referenced

bindings/pydrake/geometry/geometry_py_optimization.cc line 51 at r5 (raw file):

Previously, hongkai-dai (Hongkai Dai) wrote…

What is the return strategy here? Also py_rvp::copy?

Since b is a Polynomial and not Vector3<symbolic::Polynomial> it can be returned without copying. The default for the return value policy is return_value_policy::automatic which I assume in this case return py_rvp::reference

bindings/pydrake/geometry/geometry_py_optimization.cc line 61 at r5 (raw file):

Previously, hongkai-dai (Hongkai Dai) wrote…

Again, curious why we cannot return by reference.

Ditto. Eigen vectors of objects need to be copied.

hongkai-dai

Reviewable status: 8 unresolved discussions, LGTM missing from assignee hongkai-dai, 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)

geometry/optimization/cspace_free_structs.h line 44 at r5 (raw file):

Previously, AlexandreAmice (Alexandre Amice) wrote…

I don't believe I changed anything in this file ultimately?

My bad, I was looking at the wrong revision. Retracted.

bindings/pydrake/geometry/geometry_py_optimization.cc line 67 at r2 (raw file):

Previously, AlexandreAmice (Alexandre Amice) wrote…

Sounds good. Will take these out once that PR lands.

BTW, the #19756 just lands.

bindings/pydrake/geometry/geometry_py_optimization.cc line 50 at r5 (raw file):

Previously, AlexandreAmice (Alexandre Amice) wrote…

Eigen matrices of objects get cast to numpy.ndarray which must be copied. If you use either py_rvp::reference or py_rvp::reference_internal you get a the error RuntimeError: dtype=object arrays must be copied, and cannot be referenced

I see, thanks!

BTW, could you add the documentation here

use py_rvp::copy here because numpy.ndarray with dtype=object arrays must be copied, and cannot be referenced.

bindings/pydrake/geometry/geometry_py_optimization.cc line 51 at r5 (raw file):

Previously, AlexandreAmice (Alexandre Amice) wrote…

Since b is a Polynomial and not Vector3<symbolic::Polynomial> it can be returned without copying. The default for the return value policy is return_value_policy::automatic which I assume in this case return py_rvp::reference

I see, thanks!

bindings/pydrake/geometry/geometry_py_optimization.cc line 878 at r6 (raw file):

            py::arg("s_center"), py::arg("options"), cls_doc.BinarySearch.doc)
        .def(
            "MakeIsGeometrySeparableProgram",

Do we still need this lambda function here, as SortedPair has been fixed?

bindings/pydrake/geometry/geometry_py_optimization.cc line 947 at r6 (raw file):

          .def(py::init<>())
          .def(
              "prog",

I am not very sure what we should do here.

In

drake/bindings/pydrake/multibody/inverse_kinematics_py.cc

Line 133 in 45b9edf

.def("prog", &Class::prog, py_rvp::reference_internal, cls_doc.prog.doc)

it just calls &Class:prog instead of a lambda function. Should we do the same here?

bindings/pydrake/geometry/test/optimization_test.py line 905 at r5 (raw file):

Previously, AlexandreAmice (Alexandre Amice) wrote…

This makes the test take extremely long. With Mosek, the test is much faster and we do not need to bump up the timeout. I am not sure how to filter this to only run if Mosek is built and whether this is desirable @EricCousineau-TRI

BTW, you could do

mosek_solver = MosekSolver()
if mosek_solver.available():

AlexandreAmice

Reviewable status: 6 unresolved discussions, LGTM missing from assignee hongkai-dai, 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)

bindings/pydrake/geometry/geometry_py_optimization.cc line 878 at r6 (raw file):

Previously, hongkai-dai (Hongkai Dai) wrote…

Do we still need this lambda function here, as SortedPair has been fixed?

Done.

bindings/pydrake/geometry/geometry_py_optimization.cc line 947 at r6 (raw file):

Previously, hongkai-dai (Hongkai Dai) wrote…

I am not very sure what we should do here.

In

drake/bindings/pydrake/multibody/inverse_kinematics_py.cc

Line 133 in 45b9edf

.def("prog", &Class::prog, py_rvp::reference_internal, cls_doc.prog.doc)

it just calls &Class:prog instead of a lambda function. Should we do the same here?

BTW: I moved this to SeparationCertificateProgramBase binding.

The difference is that InverseKinematics::prog returns a const reference to the program while SeparationCertificateProgram::prog returns a copyable unique pointer. The reason to access this program would be to modify it/add constraints. This current code is the only way I could find to get access to the program.

The most similar design pattern I can think of is actually the MultipleShooting class which maintains both a prog_ variable and an owned_prog_ variable. I would need to go deeper to do more.

hongkai-dai

Reviewable status: 5 unresolved discussions, LGTM missing from assignee hongkai-dai, 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)

bindings/pydrake/geometry/geometry_py_optimization.cc line 947 at r6 (raw file):

Previously, AlexandreAmice (Alexandre Amice) wrote…

BTW: I moved this to SeparationCertificateProgramBase binding.

The difference is that InverseKinematics::prog returns a const reference to the program while SeparationCertificateProgram::prog returns a copyable unique pointer. The reason to access this program would be to modify it/add constraints. This current code is the only way I could find to get access to the program.

The most similar design pattern I can think of is actually the MultipleShooting class which maintains both a prog_ variable and an owned_prog_ variable. I would need to go deeper to do more.

@EricCousineau-TRI , would love to hear your suggestion on binding a copyable_unique_ptr.

EricCousineau-TRI

Reviewable status: 4 unresolved discussions, LGTM missing from assignee hongkai-dai, 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)

bindings/pydrake/geometry/BUILD.bazel line 115 at r5 (raw file):

Previously, AlexandreAmice (Alexandre Amice) wrote…

See discussion test_CspaceFreePolytopeMethods

I think the skipping should remove the need for long, maybe?

bindings/pydrake/geometry/geometry_py_optimization.cc line 947 at r6 (raw file):

Previously, hongkai-dai (Hongkai Dai) wrote…

@EricCousineau-TRI , would love to hear your suggestion on binding a copyable_unique_ptr.

I had briefly tried to make binding accessors copyable_unique_ptr<> more convenient, but was unable to get it to work #19636

If you wish to stay with copyable_unique_ptr<>, then I would recommend writing a wrapper functions accepting a pointer to change that to a pointer, e.g.

[](const Class& self) { return self.my_member().get(); }

If you are writing a function / constructor accepting a copyable_unqiue_ptr<T>, you would most likely need to wrap it to accept const T&, then clone it, e.g.

py::init([](const T& my_member) { return MyClass(my_member.Clone()); }

Examples that Russ / Jeremy had worked on a ways back:

drake/bindings/pydrake/trajectories_py.cc

Lines 557 to 566 in 39b4289

    
           .def(py::init([](std::vector<const Trajectory<T>*> py_segments) { 
        
             std::vector<copyable_unique_ptr<Trajectory<T>>> segments; 
        
             segments.reserve(py_segments.size()); 
        
             for (const Trajectory<T>* py_segment : py_segments) { 
        
               segments.emplace_back(py_segment ? py_segment->Clone() : nullptr); 
        
             } 
        
             return std::make_unique<CompositeTrajectory<T>>( 
        
                 std::move(segments)); 
        
           }), 
        
               py::arg("segments"), cls_doc.ctor.doc)

bindings/pydrake/geometry/test/optimization_test.py line 905 at r5 (raw file):

Previously, hongkai-dai (Hongkai Dai) wrote…

BTW, you could do

mosek_solver = MosekSolver()
if mosek_solver.available():

Yup! Alternatively, you can do unittest.skipUnless(), e.g.

drake/bindings/pydrake/multibody/test/optimization_test.py

Lines 123 to 124 in 39b4289

    
           @unittest.skipUnless(mp.SnoptSolver().available(), "Requires Snopt") 
        
           def test_one_box(self):

EricCousineau-TRI

Reviewable status: 4 unresolved discussions, LGTM missing from assignee hongkai-dai, 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)

bindings/pydrake/geometry/geometry_py_optimization.cc line 831 at r5 (raw file):
The proper fix is to have py::module::import("pydrake.multibody.rational") to ensure binding of RationalForwardKinematics is present:
https://drake.mit.edu/doxygen_cxx/group__python__bindings.html#PydrakeModuleDefinitions

Any symbol referenced in a module binding (even as function/method parameters) must either be bound in that compilation unit (with the binding evaluated before to the reference), or the module must import the pydrake module in which it is bound (e.g., py::module::import("pydrake.foo"))). Failure to do so can cause errors (unable to cast unregistered types from Python to C++) and can cause the generated docstring from pybind11 to render these types by their C++ typeid rather than the Python type name.

However, this showed that there was a dependency cycle from pydrake.geometry (and relatedly, pydrake.geometry.optimization, since it is imported at the same time), ->pydrake.multibody.rational -> pydrake.multibody -> pydrake.geometry.

I have shown this in the following meta PR:
AlexandreAmice#75

This is presently defective, in that we can't import pydrake.multibody.rational here, thus we can't guarantee that a user won't see this error unless they explicitly import pydrake.multibody.rational themselves before they call .rational_forward_kin() as you've seen.

Possible solutions:

(1) Make pydrake.geometry.optimization it's own drake_pybind_library target, //bindings/pydrake/geometry:optimization_py. This goes against current efforts to consolidate shared libraries, but should remove the dependency cycle (I think).
(2) Some other organization of code that avoids this cross-package dependency.

@jwnimmer-tri Might you have suggestions / guidance here?

jwnimmer-tri

Reviewable status: 4 unresolved discussions, LGTM missing from assignee hongkai-dai, 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)

bindings/pydrake/geometry/geometry_py_optimization.cc line 831 at r5 (raw file):

Previously, EricCousineau-TRI (Eric Cousineau) wrote…

The proper fix is to have py::module::import("pydrake.multibody.rational") to ensure binding of RationalForwardKinematics is present:
https://drake.mit.edu/doxygen_cxx/group__python__bindings.html#PydrakeModuleDefinitions

Any symbol referenced in a module binding (even as function/method parameters) must either be bound in that compilation unit (with the binding evaluated before to the reference), or the module must import the pydrake module in which it is bound (e.g., py::module::import("pydrake.foo"))). Failure to do so can cause errors (unable to cast unregistered types from Python to C++) and can cause the generated docstring from pybind11 to render these types by their C++ typeid rather than the Python type name.

However, this showed that there was a dependency cycle from pydrake.geometry (and relatedly, pydrake.geometry.optimization, since it is imported at the same time), ->pydrake.multibody.rational -> pydrake.multibody -> pydrake.geometry.

I have shown this in the following meta PR:
AlexandreAmice#75

This is presently defective, in that we can't import pydrake.multibody.rational here, thus we can't guarantee that a user won't see this error unless they explicitly import pydrake.multibody.rational themselves before they call .rational_forward_kin() as you've seen.

Possible solutions:

(1) Make pydrake.geometry.optimization it's own drake_pybind_library target, //bindings/pydrake/geometry:optimization_py. This goes against current efforts to consolidate shared libraries, but should remove the dependency cycle (I think).
(2) Some other organization of code that avoids this cross-package dependency.

@jwnimmer-tri Might you have suggestions / guidance here?

The only sound fix is the endgame where we have pydrake.so as a single shared library, where we have helper functions like void DefineGeometryOptimization(py::module m); and void DefineMultibodyRationaly(py::module m); and the init function for the shared library calls those individual functions one by one in exactly the right order. I'm slowly working on making that happen.

In the meantime, the only thing you can do is choose the lesser of whatever evils exist.

(IIRC there is already one import cycle between multibody and geometry, maybe the default friction class or something?)

EricCousineau-TRI

Reviewable status: 4 unresolved discussions, LGTM missing from assignee hongkai-dai, 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)

bindings/pydrake/geometry/geometry_py_optimization.cc line 831 at r5 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

The only sound fix is the endgame where we have pydrake.so as a single shared library, where we have helper functions like void DefineGeometryOptimization(py::module m); and void DefineMultibodyRationaly(py::module m); and the init function for the shared library calls those individual functions one by one in exactly the right order. I'm slowly working on making that happen.

In the meantime, the only thing you can do is choose the lesser of whatever evils exist.

(IIRC there is already one import cycle between multibody and geometry, maybe the default friction class or something?)

Working: Gotcha; I will see what our existing dependency cycle looks like.
Most likely, the lesser of the two evils in the short term is to split this out into its own shared module for now, which will then become moot once everything is rolled up.

jwnimmer-tri

Reviewable status: 4 unresolved discussions, LGTM missing from assignee hongkai-dai, 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)

bindings/pydrake/geometry/geometry_py_optimization.cc line 831 at r5 (raw file):

... if what you mean is CoulombFriction ...

Mostly yes. Here's what I mean, precisely: #17520 (comment). See how the docs for pydrake.geometry.AddContactMaterial list the C++ type instead of the Python type? The *.pyi file will be wrong (missing the type) as well.

AlexandreAmice

Reviewable status: 3 unresolved discussions, LGTM missing from assignee hongkai-dai, 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

bindings/pydrake/geometry/geometry_py_optimization.cc line 831 at r5 (raw file):

Previously, EricCousineau-TRI (Eric Cousineau) wrote…

Here's my attempt to split it, but it seems to have its own import issue:
#19852

I probably won't be able to get back to this until late next week (or later), unfortunately.

Perhaps I can submit this PR and get back to how to bind rational_forward_kinematics later? Access to the rational_forward_kinematics object is fairly minor to the end-users I believe and this seems like it would require a larger overhaul of the bindings layout.

bindings/pydrake/geometry/geometry_py_optimization.cc line 947 at r6 (raw file):

Previously, EricCousineau-TRI (Eric Cousineau) wrote…

I had briefly tried to make binding accessors copyable_unique_ptr<> more convenient, but was unable to get it to work #19636

If you wish to stay with copyable_unique_ptr<>, then I would recommend writing a wrapper functions accepting a pointer to change that to a pointer, e.g.
[](const Class& self) { return self.my_member().get(); }
If you are writing a function / constructor accepting a copyable_unqiue_ptr<T>, you would most likely need to wrap it to accept const T&, then clone it, e.g.
py::init([](const T& my_member) { return MyClass(my_member.Clone()); }
Examples that Russ / Jeremy had worked on a ways back:

drake/bindings/pydrake/trajectories_py.cc

Lines 557 to 566 in 39b4289

.def(py::init([](std::vector<const Trajectory<T>*> py_segments) {

std::vector<copyable_unique_ptr<Trajectory<T>>> segments;

segments.reserve(py_segments.size());

for (const Trajectory<T>* py_segment : py_segments) {

segments.emplace_back(py_segment ? py_segment->Clone() : nullptr);

}

return std::make_unique<CompositeTrajectory<T>>(

std::move(segments));

}),

py::arg("segments"), cls_doc.ctor.doc)

I am not sure if I should change the code or if what I have currently is acceptable?

hongkai-dai

Reviewed 1 of 5 files at r7, 1 of 4 files at r10, 1 of 3 files at r12.
Reviewable status: 4 unresolved discussions, LGTM missing from assignee hongkai-dai, 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)

geometry/optimization/cspace_free_polytope.h line 293 at r12 (raw file):

    }

    /** Each plane index is mapped to a vector of polynomial, */

BTW, the plane index is mapped to a single polynomial instead of a vector of polynomial.

bindings/pydrake/geometry/geometry_py_optimization.cc line 947 at r6 (raw file):

Previously, AlexandreAmice (Alexandre Amice) wrote…

I am not sure if I should change the code or if what I have currently is acceptable?

I think we can accept the current code. I mention the copyable_unique_ptr in case you want to access SeparationCertificateProgram::prog. As you have bind the accessor function through a lambda in the base class, I think we are good here.

EricCousineau-TRI

Reviewable status: 3 unresolved discussions, LGTM missing from assignee hongkai-dai, 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)

bindings/pydrake/geometry/geometry_py_optimization.cc line 831 at r5 (raw file):

Previously, AlexandreAmice (Alexandre Amice) wrote…

Perhaps I can submit this PR and get back to how to bind rational_forward_kinematics later? Access to the rational_forward_kinematics object is fairly minor to the end-users I believe and this seems like it would require a larger overhaul of the bindings layout.

Yup, that works for me! Leaving the TODO sounds great for now.

bindings/pydrake/geometry/geometry_py_optimization.cc line 947 at r6 (raw file):

Previously, hongkai-dai (Hongkai Dai) wrote…

I think we can accept the current code. I mention the copyable_unique_ptr in case you want to access SeparationCertificateProgram::prog. As you have bind the accessor function through a lambda in the base class, I think we are good here.

OK Yup, works for me!

hongkai-dai

+@ggould-tri for platform review please, thanks!

Reviewable status: 2 unresolved discussions, LGTM missing from assignee ggould-tri(platform), commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on @AlexandreAmice)

ggould-tri

; formatting and comment quibbles.

Reviewed 1 of 4 files at r10, 2 of 3 files at r12, 2 of 2 files at r13, all commit messages.
Reviewable status: 12 unresolved discussions, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on @AlexandreAmice)

geometry/optimization/cspace_free_structs.h line 23 at r13 (raw file):

 */
// TODO(Alexandre.Amice) consider moving to internal since this is never
// accessed publically by cspace_free_polytope

I recommend that you do this soon -- in this PR or immediately after -- or else remove this TODO, since this struct will become part of Drake's public API subject to deprecation rules after a month.

bindings/pydrake/geometry/geometry_py_optimization.cc line 49 at r13 (raw file):

            m, "CSpaceSeparatingPlane", param, base_cls_doc.doc)
            // use py_rvp::copy here because numpy.ndarray with dtype=object
            // arrays must be copied, and cannot be referenced.

typo: Capitalize complete sentences.

Suggestion:

            // Use py_rvp::copy here because numpy.ndarray with dtype=object
            // arrays must be copied, and cannot be referenced.

bindings/pydrake/geometry/geometry_py_optimization.cc line 62 at r13 (raw file):

            .def_readonly("plane_degree", &Class::plane_degree)
            // use py_rvp::copy here because numpy.ndarray with dtype=object
            // arrays must be copied, and cannot be referenced.

typo: Capitalize complete sentences.

Suggestion:

            // Use py_rvp::copy here because numpy.ndarray with dtype=object
            // arrays must be copied, and cannot be referenced.

bindings/pydrake/geometry/geometry_py_optimization.cc line 847 at r13 (raw file):

        m, "CspaceFreePolytopeBase", base_cls_doc.doc);
    cspace_free_polytope_base_cls
        // TODO(Alexandre.Amice): Bind rational_forward_kinematics once

Since this TODO has complex logic associated with it, it would be better to create an issue for it and reference that issue number here. The issue would also give you a place to put relevant code.

bindings/pydrake/geometry/geometry_py_optimization.cc line 857 at r13 (raw file):

        // &BaseClass::rational_forward_kin,
        // py_rvp::reference_internal,
        // base_cls_doc.rational_forward_kin.doc)

minor: Since you're committing commented-out code here, generally a no-no, this should be a bit prettier and more clearly set out with justification.

Suggestion:

        // -> pydrake.geometry.optimizaiton.
        // The missing binding would look something like:
        //      .def("rational_forward_kin",
        //           &BaseClass::rational_forward_kin,
        //           py_rvp::reference_internal,
        //           base_cls_doc.rational_forward_kin.doc)

bindings/pydrake/geometry/geometry_py_optimization.cc line 927 at r13 (raw file):

            cls_doc.SeparatingPlaneLagrangians.GetSolution.doc)
        // use py_rvp::copy here because numpy.ndarray with dtype=object arrays
        // must be copied, and cannot be referenced.

typo: Capitalize complete sentences.

Suggestion:

        // Use py_rvp::copy here because numpy.ndarray with dtype=object arrays
        // must be copied, and cannot be referenced.

bindings/pydrake/geometry/geometry_py_optimization.cc line 949 at r13 (raw file):

                  .negative_side_rational_lagrangians.doc)
          // use py_rvp::copy here because numpy.ndarray with dtype=object
          // arrays must be copied, and cannot be referenced.

typo: Capitalize complete sentences.

Suggestion:

          // Use py_rvp::copy here because numpy.ndarray with dtype=object
          // arrays must be copied, and cannot be referenced.

bindings/pydrake/geometry/geometry_py_optimization.cc line 956 at r13 (raw file):

          .def_readonly("result", &SepCertClass::result)
          // use py_rvp::copy here because numpy.ndarray with dtype=object
          // arrays must be copied, and cannot be referenced.

typo: Capitalize complete sentences.

Suggestion:

          // Use py_rvp::copy here because numpy.ndarray with dtype=object
          // arrays must be copied, and cannot be referenced.

bindings/pydrake/geometry/geometry_py_optimization.cc line 960 at r13 (raw file):

              &SepCertClass::plane_decision_var_vals, py_rvp::copy);
    }
    py::class_<Class::SeparationCertificate>(cspace_free_polytope_cls,

I don't see why some of these py::class_ statements are in {} stanzas and some aren't. Is there a reason to it? There seem to be a mix of both styles in here, and consistency would be better.

Suggestion:

    }
      py::class_<Class::SeparationCertificate>(cspace_free_polytope_cls,

bindings/pydrake/geometry/test/optimization_test.py line 743 at r13 (raw file):

    def test_CspaceFreePolytope_SeparatingPlaneOrder(self):
        # access the separating plane orders

minor: Throughout this file, please capitalize and punctuate complete sentence comments.

AlexandreAmice

Reviewable status: 2 unresolved discussions, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes

geometry/optimization/cspace_free_structs.h line 23 at r13 (raw file):

Previously, ggould-tri wrote…

I recommend that you do this soon -- in this PR or immediately after -- or else remove this TODO, since this struct will become part of Drake's public API subject to deprecation rules after a month.

I will address this in a separate PR.

bindings/pydrake/geometry/geometry_py_optimization.cc line 947 at r6 (raw file):

Previously, EricCousineau-TRI (Eric Cousineau) wrote…

OK Yup, works for me!

Done.

bindings/pydrake/geometry/geometry_py_optimization.cc line 857 at r13 (raw file):

Previously, ggould-tri wrote…

minor: Since you're committing commented-out code here, generally a no-no, this should be a bit prettier and more clearly set out with justification.

Done. I removed this and added an issue number.

bindings/pydrake/geometry/geometry_py_optimization.cc line 960 at r13 (raw file):

Previously, ggould-tri wrote…

I don't see why some of these py::class_ statements are in {} stanzas and some aren't. Is there a reason to it? There seem to be a mix of both styles in here, and consistency would be better.

Done. Thanks for catching that.

ggould-tri

Reviewed 2 of 2 files at r14, all commit messages.
Reviewable status: 1 unresolved discussion, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on @AlexandreAmice)

AlexandreAmice

+(status: squashing now)
+(release notes: feature)

@ggould-tri are you okay with the release note tag?

Reviewable status: complete! all discussions resolved, LGTM from assignees ggould-tri(platform),hongkai-dai

ggould-tri

Yes; it will get sorted to the bindings section during the release engineering process.

Reviewable status: complete! all discussions resolved, LGTM from assignees ggould-tri(platform),hongkai-dai

AlexandreAmice assigned hongkai-dai Jun 11, 2023

AlexandreAmice commented Jun 11, 2023

View reviewed changes

hongkai-dai reviewed Jun 12, 2023

View reviewed changes

AlexandreAmice force-pushed the cspace_free_polytope_bindings_non_dev branch from 8a4b85b to cc38ede Compare July 10, 2023 02:40

AlexandreAmice commented Jul 10, 2023

View reviewed changes

hongkai-dai reviewed Jul 11, 2023

View reviewed changes

EricCousineau-TRI reviewed Jul 11, 2023

View reviewed changes

EricCousineau-TRI mentioned this pull request Jul 11, 2023

fixup! fix sorted_pair_pybind bug AlexandreAmice/drake#74

Merged

EricCousineau-TRI reviewed Jul 11, 2023

View reviewed changes

AlexandreAmice commented Jul 11, 2023

View reviewed changes

EricCousineau-TRI mentioned this pull request Jul 11, 2023

[bindings] Fix bugs for sorted_pair_pybind; add test #19753

Merged

EricCousineau-TRI reviewed Jul 11, 2023

View reviewed changes

hongkai-dai reviewed Jul 12, 2023

View reviewed changes

EricCousineau-TRI reviewed Jul 12, 2023

View reviewed changes

AlexandreAmice force-pushed the cspace_free_polytope_bindings_non_dev branch from 2c67a1b to dd50b52 Compare July 15, 2023 17:31

AlexandreAmice commented Jul 15, 2023

View reviewed changes

hongkai-dai reviewed Jul 16, 2023

View reviewed changes

AlexandreAmice commented Jul 17, 2023

View reviewed changes

hongkai-dai reviewed Jul 17, 2023

View reviewed changes

AlexandreAmice force-pushed the cspace_free_polytope_bindings_non_dev branch from 9e9c51c to d75086d Compare July 18, 2023 02:29

AlexandreAmice commented Jul 18, 2023

View reviewed changes

hongkai-dai reviewed Jul 18, 2023

View reviewed changes

EricCousineau-TRI reviewed Jul 19, 2023

View reviewed changes

EricCousineau-TRI mentioned this pull request Jul 19, 2023

make test pass, but defect dependency cycle AlexandreAmice/drake#75

Closed

EricCousineau-TRI reviewed Jul 19, 2023

View reviewed changes

jwnimmer-tri reviewed Jul 19, 2023

View reviewed changes

EricCousineau-TRI reviewed Jul 26, 2023

View reviewed changes

EricCousineau-TRI and others added 11 commits July 29, 2023 09:37

fixup! fix sorted_pair_pybind bug

95a0a2a

options still need tweaking

28dd997

update tests

b72f1f1

branch to figure out why binary search fails

071ae6a

bound binary search lagrangian options

67c658d

update

a81ec12

fix some linting

69b3de5

update some linting and address part of initial review

56ea52f

address some review

513d619

clean up some commented out code

c1b6b70

rebase again

1d9427f

AlexandreAmice force-pushed the cspace_free_polytope_bindings_non_dev branch from d64a962 to 1d9427f Compare July 29, 2023 20:41

jwnimmer-tri reviewed Jul 30, 2023

View reviewed changes

AlexandreAmice commented Jul 30, 2023

View reviewed changes

hongkai-dai reviewed Jul 31, 2023

View reviewed changes

EricCousineau-TRI reviewed Aug 1, 2023

View reviewed changes

finish addressing review

7c5df24

hongkai-dai assigned ggould-tri Aug 14, 2023

hongkai-dai reviewed Aug 14, 2023

View reviewed changes

ggould-tri reviewed Aug 14, 2023

View reviewed changes

AlexandreAmice mentioned this pull request Aug 19, 2023

Bind rational_forward_kinematics method in CspaceFreePolytope #20025

Open

address platform review

47d80e1

AlexandreAmice commented Aug 19, 2023

View reviewed changes

ggould-tri reviewed Aug 21, 2023

View reviewed changes

AlexandreAmice added release notes: feature This pull request contains a new feature status: squashing now https://drake.mit.edu/reviewable.html#curated-commits labels Aug 21, 2023

AlexandreAmice commented Aug 21, 2023

View reviewed changes

ggould-tri reviewed Aug 21, 2023

View reviewed changes

AlexandreAmice merged commit 95be711 into RobotLocomotion:master Aug 21, 2023

jwnimmer-tri mentioned this pull request Sep 13, 2023

[py optimization] Add missing repr and docs for polytope options #20181

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cspace free polytope bindings non dev #19574

Cspace free polytope bindings non dev #19574

AlexandreAmice commented Jun 11, 2023 •

edited by sherm1

Loading

AlexandreAmice left a comment

hongkai-dai left a comment

AlexandreAmice left a comment

hongkai-dai left a comment

EricCousineau-TRI left a comment

EricCousineau-TRI left a comment

EricCousineau-TRI left a comment

AlexandreAmice left a comment

EricCousineau-TRI left a comment

EricCousineau-TRI left a comment

hongkai-dai left a comment

EricCousineau-TRI left a comment

AlexandreAmice left a comment

AlexandreAmice left a comment

hongkai-dai left a comment

AlexandreAmice left a comment

hongkai-dai left a comment

AlexandreAmice left a comment

hongkai-dai left a comment

EricCousineau-TRI left a comment

EricCousineau-TRI left a comment

jwnimmer-tri left a comment

EricCousineau-TRI left a comment

jwnimmer-tri left a comment

AlexandreAmice left a comment

hongkai-dai left a comment

EricCousineau-TRI left a comment

hongkai-dai left a comment

ggould-tri left a comment

AlexandreAmice left a comment

ggould-tri left a comment

AlexandreAmice left a comment

ggould-tri left a comment

	.def(py::init([](std::vector<const Trajectory<T>*> py_segments) {
	std::vector<copyable_unique_ptr<Trajectory<T>>> segments;
	segments.reserve(py_segments.size());
	for (const Trajectory<T>* py_segment : py_segments) {
	segments.emplace_back(py_segment ? py_segment->Clone() : nullptr);
	}
	return std::make_unique<CompositeTrajectory<T>>(
	std::move(segments));
	}),
	py::arg("segments"), cls_doc.ctor.doc)

	@unittest.skipUnless(mp.SnoptSolver().available(), "Requires Snopt")
	def test_one_box(self):

Cspace free polytope bindings non dev #19574

Cspace free polytope bindings non dev #19574

Conversation

AlexandreAmice commented Jun 11, 2023 • edited by sherm1 Loading

AlexandreAmice left a comment

Choose a reason for hiding this comment

hongkai-dai left a comment

Choose a reason for hiding this comment

AlexandreAmice left a comment

Choose a reason for hiding this comment

hongkai-dai left a comment

Choose a reason for hiding this comment

EricCousineau-TRI left a comment

Choose a reason for hiding this comment

EricCousineau-TRI left a comment

Choose a reason for hiding this comment

EricCousineau-TRI left a comment

Choose a reason for hiding this comment

AlexandreAmice left a comment

Choose a reason for hiding this comment

EricCousineau-TRI left a comment

Choose a reason for hiding this comment

EricCousineau-TRI left a comment

Choose a reason for hiding this comment

hongkai-dai left a comment

Choose a reason for hiding this comment

EricCousineau-TRI left a comment

Choose a reason for hiding this comment

AlexandreAmice left a comment

Choose a reason for hiding this comment

AlexandreAmice left a comment

Choose a reason for hiding this comment

hongkai-dai left a comment

Choose a reason for hiding this comment

AlexandreAmice left a comment

Choose a reason for hiding this comment

hongkai-dai left a comment

Choose a reason for hiding this comment

AlexandreAmice left a comment

Choose a reason for hiding this comment

hongkai-dai left a comment

Choose a reason for hiding this comment

EricCousineau-TRI left a comment

Choose a reason for hiding this comment

EricCousineau-TRI left a comment

Choose a reason for hiding this comment

jwnimmer-tri left a comment

Choose a reason for hiding this comment

EricCousineau-TRI left a comment

Choose a reason for hiding this comment

jwnimmer-tri left a comment

Choose a reason for hiding this comment

AlexandreAmice left a comment

Choose a reason for hiding this comment

hongkai-dai left a comment

Choose a reason for hiding this comment

EricCousineau-TRI left a comment

Choose a reason for hiding this comment

hongkai-dai left a comment

Choose a reason for hiding this comment

ggould-tri left a comment

Choose a reason for hiding this comment

AlexandreAmice left a comment

Choose a reason for hiding this comment

ggould-tri left a comment

Choose a reason for hiding this comment

AlexandreAmice left a comment

Choose a reason for hiding this comment

ggould-tri left a comment

Choose a reason for hiding this comment

AlexandreAmice commented Jun 11, 2023 •

edited by sherm1

Loading