Skip to content
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

Add algorithm to compute the standalone Coriolis matrix C(q, ν) #75

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

flferretti
Copy link
Collaborator

@flferretti flferretti commented Feb 2, 2024

This PR will add an algorithm to compute the standalone Coriolis matrix basing on the one developed by S. Echeandia et al..

Solves #41


📚 Documentation preview 📚: https://jaxsim--75.org.readthedocs.build/75/

@flferretti flferretti self-assigned this Feb 2, 2024
@diegoferigo
Copy link
Member

Some questions. When I checked your code a while ago, if I recall it was missing the base-related blocks of the matrix. Is this the case?

And I mean, given a model with state $(\bar{\mathbf{q}}, \, \bar{\boldsymbol{\nu}})$, would the following check pass?

$$\text{RNEA}(\mathbf{q}=\bar{\mathbf{q}}, \, \boldsymbol{\nu} = \bar{\boldsymbol{\nu}}, \dot{\boldsymbol{\nu}} = \mathbf{0}, \mathbf{f}^{\text{ext}} = \mathbf{0}) == C(\bar{\mathbf{q}}, \\, \bar{\boldsymbol{\nu}}) \, \bar{\boldsymbol{\nu}}$$

@flferretti
Copy link
Collaborator Author

Thanks Diego, I'll take a look at it! It would probably be a good idea to also add that check in the tests

@diegoferigo
Copy link
Member

diegoferigo commented Feb 2, 2024

For the records, for those that are not familiar with the high_level resources, the left-hand side of the previous equation is no more no less than Model.free_floating_bias_forces.

@traversaro
Copy link
Contributor

This also computes as a by-product the time derivative of the mass matrix, right? This may be of interest of @rob-mau .

)

# Compute model acceleration with ABA
H, H_dot, C = model.coriolis_matrix()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
H, H_dot, C = model.coriolis_matrix()
H, H_dot, C = model.coriolis_matrix()

Could it make sense to add a test here to check that the mass matrix here is the same that the mass matrix computed with CRBA?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea @traversaro.

@flferretti please note that the low-level jaxsim.physics.algos.crba.crba always computes $M_B(\mathbf{s})$ in body-fixed representation since it's the only representation that is independent from the base pose. Then, the rest of the high_level logic converts the body-fixed mass matrix to the active representation.

Make sure to compare the same quantities.

tests/test_coriolis.py Outdated Show resolved Hide resolved
@flferretti
Copy link
Collaborator Author

Thanks @traversaro! I will address your comments one-by-one in future commits

xfb=self.data.model_state.xfb(),
)

return H, H_dot, C
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, similarly to the other high-level methods, the returned matrices should be converted to the active velocity representation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some notes on this in https://github.com/ami-iit/idynfor/blob/master/doc/theory_background.md#dynamics , even if the do not covert the
$\dot{M}$ and $C$ case at the moment.

def body(C):
W_H_B = self.base_transform()
B_X_W = sixd.se3.SE3.from_matrix(W_H_B).inverse().adjoint()
C = B_X_W.T @ C @ B_X_W
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still haven't run this code, but I was expecting a matrix $C \in \mathbb{R}^{(6+n)\times(6+n)}$. Here it seems be $\mathbb{R}^{6 \times 6}$, am I missing something?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that $C$ has to be transformed as 3.60b:

Screenshot_20240202_144953

Copy link
Contributor

@traversaro traversaro Feb 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest, I do not remember if this transformation ensures that the $\dot{M} = C + C^T$ is preserved, but it should be easy to check (we can do this at the whiteboard next week).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants