Add more description on InverseKinematics::solve()

[jslee02]

This comment might be a bit helpful to understand how IK problem is solved internally especially about what is the initial guess and how seeds are used.

[mxgrey]

Looks good to me 👍

[mkoval]

This looks great. 👍

It would be helpful if you could clarify a few additional points:

1. Does getProblem()->setInitialSeed() have an effect?
2. How about getProblem->addSeed()?
3. What's the difference between addSeed() and setInitialSeed()?
4. What attributes on getProblem() are safe to modify? Which are overwritten by solve()?
5. What random number generator and seed are used for random restarts?
6. How do you generate seeds for unbounded DOFs (e.g. cyclic joints)?
7. How do I set the number of attempts? Where is this logic implemented?

[mxgrey]

Those are all really good questions, but the answers to those questions are a teeny bit complicated:

1. InverseKinematics::getProblem()->setInitialGuess() will have an effect if you use InverseKinematics::getSolver()->solve(), but not if you just use InverseKinematics::solve(). The function InverseKinematics::solve() wraps up the call to getSolver()->solve() with a few extra steps to make sure that the IK Problem is configured in a sane way. Whatever you set the Problem's "initial guess" to will get overwritten by InverseKinematics::solve() if that's the function that you decide to call instead of calling Solver::solve() directly.
2. getProblem->addSeed() will do the expected thing of adding a seed configuration to the Problem that the IK solver is using. The effects of that will actually depend on what type of Solver you're having your InverseKinematics module use. The default Solver type is GradientDescentSolver. That solver type will start to use each seed after the first attempt at a solution failed if GradientDescentSolver::setMaxAttempts(~) is given a value greater than 1. By default, it is given only one attempt. After all seeds have been used it will start randomly perturbing configurations until it has reached its maximum number of attempts.
3. To be technical, there is no setInitialSeed function. setInitialGuess will decide what configuration the solver starts from. For the effect of addSeed, see (2).
4. Strictly speaking, it's meant to be safe to modify absolutely anything in the Problem. But there's definitely an important question to be asked about how easy or difficult it is to modify things in the Problem without experiencing unintended negative consequences. I tried to design the whole InverseKinematics module to be both (a) easy to use and highly effective out-of-the-box and (b) 100% configurable to anyone's needs. This means it should (a) be useful to a beginner without requiring any effort while also (b) completely customizable for an expert user. But there's an important question about the middle ground: What if (c) a casual (but not beginner) user wants to make a small adjustment? Is the current system reasonable for that purpose? As the person who designed and implemented all of it, I can't answer that question; I'll need feedback from users to figure that out. Maybe we'll need to do some redesigning in order to accommodate use case (c). Or maybe we just need documentation and tutorials. I eagerly invite opinions on this matter.
5. This is decided by the Solver that you're using. Technically, only one of the currently existing Solvers even uses seeds at all, and that's the GradientDescentSolver. In fact, I added the seed concept to the Problem class for the sake of the GradientDescentSolver. As it currently stands, I think the GradientDescentSolver is the best suited for the IK module (it was actually created specifically for the sake of the IK module), but that could easily change in the future if we start offering more third-party Solver interfaces or more native solver types. But the way the GradientDescentSolver does it is determined by its Properties settings, and information on that is available in its header.
6. DART doesn't generate seeds. generating seeds is exclusively at the discretion of the user. The randomization that occur after seeds no longer exist is determined by the GradientDescentSolver::Properties. For joints with infinite bounds, the effect will be that each joint will take a step the size of mMaxRandomizationStep in a random direction (up or down).
7. The "number of attempts" concept is unique to the GradientDescentSolver. You can use any solver type in your IK module, so you'll have to cast the return of getSolver() to std::shared_ptr<GradientDescentSolver> and then you can access its interface for changing the maximum number of attempts.

So as I laid out in (3), there's an important question to keep in mind as we move forward: Is the current implementation suitable for use case (c)? Are there things we can do to improve usability without sacrificing generalizability? Do we just need to make sure we have really good documentation? If so, what kind of documentation should we have? I think the upcoming tech report will be nice to have, but I don't think it can be as exhaustive as something like a wiki. We might want to consider having a user-driven wiki for DART/KIDO down the road.

[mkoval]

@mxgrey thank you for the excellent explanation.

After some more time working with the API, I have a few concrete suggestions:

• The Doxygen comment associated with InverseKinematics::solve() should explicitly state what configuration it does and which properties of Solver will be overwritten. From quickly skimming the source code, this includes at least:
  • setDimension
  • setInitialGuess
  • setLowerBounds
  • setUpperBounds
• Local optimizers that do not support random restarts should print a warning if asked to solve a Problem that has getSeeds().size() > 1. Otherwise, the user may be baffled why adding more seeds to their Problem doesn't improve the success rate of the optimization.
• Document the difference between an "initial guess" and a "seed." Does the input to setInitialGuess() become a seed? What happens if I have already added seeds to my Problem?

Here is some pontification on use case (c):

• Using random restarts for IK is common. It's awkward that I have to dynamic_cast the Solver to a GradientDescentSolver, manually setup the Solver, and generate my own seeds to do this.
• It's also common to inject a "filter" between the IK solver and the random restart logic, e.g. to reject solutions that are in collision with the environment. It's awkward to do this if the random retry logic is part of the optimizer.
• With an analytic solver on a redundant manipulator, we replace "random restart" with "iterate over possible values of the free joint(s)" in the above statement.

Unfortunately, I don't have any concrete suggestions about how to improve this.

[jslee02]

I think we have invaluable comments here about the IK module. @mxgrey Could you update the comment on the code if needed? Otherwise, I believe this PR is ready to merge.

[mxgrey]

I've crammed in what I think are the most important points from the discussion in this thread. I don't want to get excessively detailed in a header file, so some of the information in the discussion should maybe be reserved for tech reports or wiki pages.

[jslee02]

Thanks! It looks perfect to me. Let's merge once CI becomes happy.