RobotLocomotion / drake

Model-based design and verification for robotics.
https://drake.mit.edu
Other
3.25k stars 1.25k forks source link

InverseKinematics for MultiBodyTree #7173

Closed hongkai-dai closed 5 years ago

hongkai-dai commented 6 years ago

As @amcastro-tri is writing MultiBodyTree, it is probably a good time to re-write our IK code. The major reason why I want to re-write this code are

  1. RigidBodyConstraint is not really a Constraint sub-class. It also has this property tspan, which is confusing and not necessary.
  2. InverseKinematics is not a MathematicalProgram sub-class. It only accepts q as decision variables, and its cost is fixed to (q- q*)ᵀ Q (q- q*). We would like to add additional cost, and maybe additional variables.

Plan

@avalenzu @amcastro-tri @RussTedrake

amcastro-tri commented 6 years ago

This is just awesome @hongkai-dai, please let me know anyway I can be of help with this. As I expressed to you before, from just a purely OOP design perspective, I think InverseKinematics is not a MathematicalProgram but has a (or uses) MathematicalProgram internally. I tell you more, what if one day we want to try out someone else's solver? we could change it under the hood (so swap the MathematicalProgram solver) and still have a reasonable IK. Are you sure you want to make public all off MathematicalProgram's API's through IK?

amcastro-tri commented 6 years ago

Another suggestion, why not to make the solvers to take a KinematicsProblem? Take as an analogy a linear algebra problem. You have the problem being described by your matrix object A (the KinematicsProblem) and then you have a series of solvers that can solve A (InverseKinematics, InverseKinematicsSequence). So what about:

              KinematicsSolver
                     |
           ----------------------
           |                    |
InverseKinematicsSolver     InverseKinematicsSequenceSolver

Then you could do:

MultibodyTree<T> model;
// Add stuff to your model...
KinematicsProblem problem(model);
// Add constraints and costs to your problem...
InverseKinematicsSolver ik_solver(problem);
InverseKinematicsSolution solution = ik_solver.solve();
jwnimmer-tri commented 6 years ago

I would also suggest avoiding KinematicsProgram inheriting from MathematicalProgram. Having bespoke subclasses of MathematicalProgram will only make it more difficult to maintain the MP class over time. (We should probably mark MP as final now to enforce this.) Also, using inheritance for "narrowing" violates the subtyping principle.

If you want to give KP users access to MP helper methods, then give KP an accessor that returns the MP, or add selective forwarding methods if you don't want to expose everything.

hongkai-dai commented 6 years ago

Thanks @amcastro-tri and @jwnimmer-tri for the suggestion. @avalenzu and I had a discussion, and we decide to take the following steps

RussTedrake commented 6 years ago

@jwnimmer-tri -- fyi, the trajectory optimization classes already subclass mathematical program (and I think that is a very natural thing)