-
Notifications
You must be signed in to change notification settings - Fork 9
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
base: main
Are you sure you want to change the base?
Conversation
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 |
Thanks Diego, I'll take a look at it! It would probably be a good idea to also add that check in the tests |
For the records, for those that are not familiar with the |
This also computes as a by-product the time derivative of the mass matrix, right? This may be of interest of @rob-mau . |
tests/test_coriolis.py
Outdated
) | ||
|
||
# Compute model acceleration with ABA | ||
H, H_dot, C = model.coriolis_matrix() |
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.
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?
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.
Good idea @traversaro.
@flferretti please note that the low-level jaxsim.physics.algos.crba.crba
always computes high_level
logic converts the body-fixed mass matrix to the active representation.
Make sure to compare the same quantities.
Thanks @traversaro! I will address your comments one-by-one in future commits |
src/jaxsim/high_level/model.py
Outdated
xfb=self.data.model_state.xfb(), | ||
) | ||
|
||
return H, H_dot, C |
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.
Also, similarly to the other high-level methods, the returned matrices should be converted to the active velocity representation.
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 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
src/jaxsim/high_level/model.py
Outdated
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 |
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 still haven't run this code, but I was expecting a matrix
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.
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.
To be honest, I do not remember if this transformation ensures that the
d1f81b9
to
ff05233
Compare
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/