Add the possibility to pass a reference DCM trajectory to the DCMPlanner #208
Conversation
| this->opti.subject_to( | ||
| vrp(2, k) - 9.81 / (casadi::MX::pow(omega(Sl(), k), 2) - omegaDot(Sl(), k)) | ||
| == contactPhaseListIt->activeContacts.begin()->second->pose.translation()[2]); |
There was a problem hiding this comment.
Can we define a placeholder for -9.81?
There was a problem hiding this comment.
If we define a constant somewhere, let's use the internationally agreed standard CODATA2018 of 9.80665 https://physics.nist.gov/cgi-bin/cuu/Value?gn
There was a problem hiding this comment.
I would expose this value in some way. Often when debugging I personally use and simulate with -10.0 and it would be great at this point if this parameter can be exposed to the user. Note that the initial omega included in the initial state and passed to setInitialState is typically computed by the users using their value of g. If, then, internally another value is hardcoded, it could get funky results.
As a matter of facts, I remember in the past changing the value to something different than 9.81 and the optimization was failing. I didn't delve into further details at that time. I'm not saying it was related to it, but it rang a bell in my memory.
There was a problem hiding this comment.
Yes, those are two different aspects:
- It would be great to have
9.80665defined somewhere likeinclude/BipedalLocomotion/Math/Constants.hasBipedalLocomotion::Math::StandardAccelerationOfGravitation(name taken from https://github.com/JuliaPhysics/PhysicalConstants.jl/blob/03abe02aca18c0d38c64a3c33b9fd025a86dc891/src/codata2018.jl#L42) or something similar. gin this case is a parameter of theTimeVaryingDCMPlanner, and if not specified (if this is supported, not sure) it should be fallback to the previous mentioned constant.
There was a problem hiding this comment.
As a matter of facts, I remember in the past changing the value to something different than 9.81 and the optimization was failing. I didn't delve into further details at that time. I'm not saying it was related to it, but it rang a bell in my memory.
In my experience, I have faced such issues as well. While integrating system with 9.80665 in one place and using 9.81 in another place, by choosing a big value for sampling time introduces errors easily into the estimator/controller.
There was a problem hiding this comment.
Ops I've just noticed #208 (comment) I created a new component. Now I ever the changes to follow what @traversaro suggested in #208 (comment)
There was a problem hiding this comment.
I think also what you did is perfect, no need to change to do exactly what I said.
There was a problem hiding this comment.
I opened an issue #210 to propagate the use of gravity constant in the rest of the codebase. No need to do it in this PR tough.
There was a problem hiding this comment.
Here I am! I introduced BipedalLocomotion::Math::StandardAccelerationOfGravitation a4af4be I exposed it in python 94e84fc
I finally introduced the gravity parameter in TimeVaryingDCMPlanner a24da91. It's optional and if the user does not set it BipedalLocomotion::Math::StandardAccelerationOfGravitation is used
GiulioRomualdi
force-pushed
the
dcm_planner/additional_input
branch
2 times, most recently
from
7570ccf
to
995b5c3
Compare
- Add the possibility to pass a desiredDCMTrajectory as external reference - Initialize the DCM planner with a std::weak_ptr instead of std::shared_ptr
…erationOfGravitation
GiulioRomualdi
force-pushed
the
dcm_planner/additional_input
branch
from
995b5c3
to
38312cc
Compare
With this PR it will be possible to pass a desired reference DCM trajectory to the DCMPlanner. The trajectory will be used as a regularization term.
In details it introduces the optional parameter
use_external_dcm_reference. If set to true the user should callsetDCMReference()to set the desired DCM reference.The python bindings have been updated and they already implement this new feature.
I checked the output of the planner with the already existing code and this is the planned trajectory
cc @paolo-viceconte @diegoferigo