-
Notifications
You must be signed in to change notification settings - Fork 416
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Extend Support for Mimic Joint #2441
base: devel
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👋 Hi,
This is a reminder message to assign an extra build label to this Pull Request if needed.
By default, this PR will be build with minimal build options (URDF support and Python bindings)
The possible extra labels are:
- build_collision (build Pinocchio with coal support)
- build_casadi (build Pinocchio with CasADi support)
- build_autodiff (build Pinocchio with CppAD support)
- build_codegen (build Pinocchio with CppADCodeGen support)
- build_extra (build Pinocchio with extra algorithms)
- build_mpfr (build Pinocchio with Boost.Multiprecision support)
- build_sdf (build Pinocchio with SDF parser)
- build_accelerate (build Pinocchio with APPLE Accelerate framework support)
- build_all (build Pinocchio with ALL the options stated above)
Thanks.
The Pinocchio development team.
c43fad1
to
52358ed
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will provide a detailed review later.
3f25772
to
5ea8402
Compare
Hi, I started using mimic joints, so I'm following this PR with interest ! How/Where can I report any bug I find ? |
bd466b3
to
b90f6e0
Compare
Hi, Thanks a lot for your interest in this PR, ! In order to simplify the PR and the review process, if you want to add new unittests, please do it directly on this branch. If you don't feel confortable with this solution, we'll find another way. As for bugs, please add comments here, and we'll take a look at it. Thanks again ! |
What do you mean ? Opening a PR in your forked repository ? |
ea956c8
to
083d240
Compare
…in the joint list and has a index higher than the new mimic
…s are unsigned int)
… mimic_parents_fromRow and non_mimic_parents_fromRow
Co-authored-by: Joris Vaillant <joris.vaillant@gmail.com>
model.joints[i], data.joints[i], typename Pass::ArgsType(model, data, q.derived(), J_)); | ||
} | ||
|
||
typedef JointJacobianForwardStep< | ||
Scalar, Options, JointCollectionTpl, ConfigVectorType, Matrix6xLike, ADDTO> | ||
MimicPass; | ||
// During the backward pass, mimicking joint jacobian might have been overwritten by the | ||
// mimicked joint (since mimicked_id < mimicking_id) Do another pass to accumulate it back | ||
for (size_t i = 0; i < model.mimicking_joints.size(); i++) | ||
{ | ||
const JointIndex mimicking_id = model.mimicking_joints[i]; | ||
const JointIndex mimicked_id = model.mimicked_joints[i]; | ||
const typename Model::IndexVector & joint_support = model.supports[jointId]; | ||
if ( | ||
std::find(joint_support.begin(), joint_support.end(), mimicking_id) | ||
== joint_support.end()) | ||
continue; // This mimicking joint does not support the selected joint, skip | ||
if ( | ||
std::find(joint_support.begin(), joint_support.end(), mimicked_id) == joint_support.end()) | ||
continue; // This mimicked joint does not support the selected joint, skip | ||
MimicPass::run( | ||
model.joints[mimicking_id], data.joints[mimicking_id], | ||
typename Pass::ArgsType(model, data, q.derived(), J_)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can patch this the same way we patch translateJacobian
: First pass only on the non mimic, second pass only on the mimic. Like we are doing in CRBA.
Co-authored-by: Joris Vaillant <joris.vaillant@gmail.com>
Co-authored-by: Joris Vaillant <joris.vaillant@gmail.com>
Co-authored-by: Joris Vaillant <joris.vaillant@gmail.com>
# For forward dynamics, aba does not support joints mimic. | ||
# However it's still possible to compute acceleration of the | ||
# mimic model, using the G matrix | ||
a_computed = np.linalg.solve(G @ M_full @ G.transpose(), tau_mimic - G @ C_full) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think is better to show we can compute aba with M_mimic and C_mimic instead of using G.
unittest/data.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should test idx_v_extended_fromRow, mimic_parents_fromRow and non_mimic_parents_fromRow.
unittest/model.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should patch standard test to check idx_vExtendeds, nvExtendeds, mimicking_joints and mimicked_joints.
… and fix jacobian bug
Overview
This PR enhances support for the mimic joint.$q_{mimic} = \alpha * q_{primary} + \beta$ (where $\alpha, \beta$ are defined by the user)
Mimic joint refer to a joint which configuration depends on the configuration of another joint (that we named the primary joint). The relation is
Key Updates:
URDF Parsing:
The mimic tag can now be parsed from urdf, and add the corresponding mimic joint to the model.
A
mimic
flag has been added to the parser to enable this feature or not (default tofalse
) in order to preserve backward compatibility.Robot and Joint Model Modifications:
nq
andnv
(resp.idx_q
,idx_v
) values have now been extended with anvExtended
(respidx_vExtended
) value, holding the dimension of the extended robot/joint tangent space. (for all classical jointnv==nvExtened
but for mimicnv==0
andnvExtended==primary.nvExtended
). The suffix vExtended stand for extended tangent space and is mainly used for computing jacobians rows and columns.Features of the joint mimic:
Extended Model
The model and joint
nq
andnv
(resp.idx_q
,idx_v
) values have now been extended with anvExtended
(respidx_vExtended
) value, holding the dimension of the extended robot/joint tangent space.The suffix vExtended stand for extended tangent space.
The choice of using
nv=0
(resp.nq=0
) for a mimic joint, allows for a transparent merge of the mimic joint contribution to the dynamics of the model.The extended tangent space (nvExtended) is useful to compute an extended Jacobian that includes the effects of both primary and mimic joints.
Changes were also introduced in vector and matrices accessors:
Configuration Vector accessor is now split in 2:
jointConfig
is the same as before for all joints, except mimic, where it will return a 0-sized vectorjointMappedConfig
is the same asjointConfig
for most joint, except mimic where it will return the configuration vector of the joint it is mimickingTangent Vector accessor is split in 2 in a similar fashion:
jointVelocity
is the same as before for all joints, except mimic, where it will return a 0-sized vectorjointMappedVelocity
is the same asjointVelocity
for most joint, except mimic where it will return the velocity vector of the joint it is mimickingAll matrices accessors are also split in 2:
jointRows
,jointCols
,jointBlock
are the same as before for all joints except mimic. For mimic, it will return the row/column/block corresponding to its mimicked index.jointExtendedRows
,jointExtendedCols
,jointExtendedBlock
will do the samejointRows
,jointCols
,jointBlock
for all joints except mimic. For mimic, it will return its own row/column/block using itsnvExtended
andidx_vExtended
New Model and Data informations
To fully support this new joint, and keep code efficiency, we introduce new vectors in model and data:
model.mimicking_joints
- in this vector, we store all the indexes of mimicking joint (JointMimicModel)model.mimicked_joints
- Here are all the indexes of the mimicked joints (the one directing the joint couple)data.mimic_subtree_joint
- It stores the index of the first joint in the mimicking joint subtree, that is not a mimicking joint.Joint which allows a mimic
Not all the joints are able to be mimicked. Please see the following list for the joints that can be mimicked:
- Prismatics
- Revolutes
- Helicoidals
Please also note that the mimic joint must be after its primary (i.e. id_mimicking > id_mimicked) in the kinematic tree for it to be created in pinocchio. If that's not the case, the model will not be created.
Attention Revolute, prismatic or helicoidal joints that are inside a composite joint, should not be mimicked, it will lead to undefined behaviour and should be avoided. Same for mimic joint inside a composite joint.
Algorithms
The following algorithms support the new joint mimic :
- RNEA
- CRBA
- Forward Kinematics
- Jacobians and Frames
- Centroidal Algorithm (ccrba)
- Reachable Workspace
Parsing
The mimic tag in a urdf file can now be taken into account if the user decides it. Otherwise it will be a fully autonomous joint in the model.
Only mimic in urdf are taken into account. We are aware a similar mechanism exists for mjcf model, but it is not supported for now.
Testing & Validation:
Limitations:
Feedback Needed:
The naming of the new member variable and functions is pretty arbitrary, we are eager to change them to better suit pinocchio naming standards.
Non exhaustively, here are a list of the lesser quality one (that should most probably be renamed) :
idx_vExtended
/nvExtended
: representing the dimension and index in the "extended model"jointCols
,jointRows
, etc method has been split intojointCols
andjointExtendedCols
(respectively if they refer toidx_v
/nv
oridx_j
/nj
)SimilarlyjointConfigSelector
was splitted intojointConfigFromDofSelector
andjointConfigFromNqSelector
to meet the need for mimic joint to select config either from the parent joint or don't select anythingSimilarlyjointVelocitySelector
was splitted intojointVelocityFromDofSelector
andjointConfigFromNqSelector
to meet the need for mimic joint to select config either from the parent joint or don't select anythingNext
Non linear mimic support
We can add a functor to JointModelMimicTpl constructor argument. This functor will hold the functon to apply to the config and velocity vectors.
This can be added latter as:
The latter options is less effective.
In both case, this will avoid a call to a visitor, since the configVectorAffineTransform can be solved at the JointModelMimicTpl construction.
Special Thanks:
A special thank you to @EtienneAr for his guidance and advice throughout this development process.
Review TODO list
Following our IRL meeting, here is the todo list:
...
The following point are Nice To Have and should be addressed in separate issues / PRs: