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

Fix JaxSimModelReferences.apply_frame_forces #220

Merged
merged 1 commit into from
Aug 20, 2024

Conversation

flferretti
Copy link
Collaborator

@flferretti flferretti commented Aug 19, 2024


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

@flferretti flferretti self-assigned this Aug 19, 2024
Copy link
Member

@diegoferigo diegoferigo left a comment

Choose a reason for hiding this comment

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

LGTM! It seems to me that before there were two problems:

  1. The returned references was wrongly switched to Inertial instead of Body when the link forces were applied (and the name of the variable W_f_L_i should have been L_f_L_i).
  2. The returned references had a different velocity representation than self.

Did I get it right?

@flferretti
Copy link
Collaborator Author

  1. The returned references was wrongly switched to Inertial instead of Body when the link forces were applied (and the name of the variable W_f_L_i should have been L_f_L_i).

I guess it's the other way around, I was converting the frame forces to body and few lines below I was summing them using Inertial representation. Keeping everything in inertial, made more sense at this point.

  1. The returned references had a different velocity representation than self.

This is correct!

There was also a third mistake that had to do with the name of a variable:

- frame_idx=frame_idxs
+ frame_index=frame_idxs

@flferretti flferretti merged commit a0efffd into main Aug 20, 2024
25 checks passed
@flferretti flferretti deleted the fix/apply_frame_forces branch August 20, 2024 07:53
@diegoferigo
Copy link
Member

I guess it's the other way around, I was converting the frame forces to body and few lines below I was summing them using Inertial representation. Keeping everything in inertial, made more sense at this point.

Yep what I meant is that the previous logic to express them in the link frames L was ok, the problem with that approach is that you were applying them as they were Inertial instead of Body. Anyway, I agree that keeping everything in inertial as after this PR is more clean.

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.

2 participants