search

ethz-adrl / towr

A light-weight, Eigen-based C++ library for trajectory optimization for legged robots.
http://wiki.ros.org/towr
BSD 3-Clause "New" or "Revised" License
929 stars 233 forks source link

Access to optimization parameters #55

Closed disRecord closed 5 years ago

disRecord commented 5 years ago

This pull request addresses issue #54.

  1. Fields of Parameters class is made public. Unnecessary access methods are removed.
  2. GaitGenerator::SetGaits() is made public.
  3. A few cosmetic changes: std::pair to represent bounds instead of array<2>, optimized GetPhase Durations().
awinkler commented 5 years ago

Thanks for your help on this. I incorporated your changes and did some modifications in #56 to make the parameters and gaits more accessible.

Can you explain why you did the additional changes to GaitGenerator. So I can understand why you want to change that, e.g. remove GetNormalizedPhaseDurations() etc.

disRecord commented 5 years ago

Can you explain why you did the additional changes to GaitGenerator. So I can understand why you want to change that, e.g. remove GetNormalizedPhaseDurations() etc.

  1. I didn't like how private GetGaitPhases() return timings for all legs and then `GetNormalizedPhaseDurations() and GetNormalizedPhaseDurations() use only information related to one leg. So additional unnecessary computations and memory allocation is happen every GetPhaseDurations() call.

  2. GetNormalizedPhaseDurations() is unnecessary, you can simply call GetPhaseDurations(1.0, ee).

  3. On other hand GetUnscaledDurations() and GetUnscaledTotalDuration() can be useful. Unscaled duration of one elemental step sequence which corresponds to Gaits element (Walk1, Run1 and etc) is usually equal to one. So total unscaled duration is roughly equal to number of steps and it measures complexity of movement.

In my code I wanted to scale dt_constraint_range_of_motion_, duration_base_polynomial_ and other dt_* parameters to number of steps. So the duration of movement would not influence significantly on optimization problem complexity.

In current version of towr_app if you set movement duration to 5 or 6 seconds optimization problem becomes more complex. Moreover end effector polynomials can became over-constrained because there is fixed number such polynomials to each step.

awinkler commented 5 years ago

Okey, that makes sense, thanks for clarifying. To keep this clean, it would be great if you could open a new PR with just the changes to GaitGenerator based off of the updated master (i'll close this one then). The SetGaits method is already public, so the new PR focusses only on my above question. And please make sure towr_ros_app.cc is using the updated method in the same way.

Thanks! :)

disRecord commented 5 years ago

Ок. I'll do it a little later after I finish towr based gait generator for our project. (Well, mainly finish). I am not planning to change either dt_* parameters or GetPhaseDurations(). So towr_app should work as intended.

Also I noticed other inconvenience in towr library API (for example, it is imposible to set end effectors' positions and velocity at the end of trajectory). I'll suggest PR on it latter.