Transform root_physx_view linear velocities to link frame

[jtigue-bdai]

Description

Currently the root_physx_views of the rigid bodies and articulations output linear and angular velocities of the com of bodies rather than the link frame. This PR transforms the velocities and accelerations to the link frame of the body.

Fixes #942

Type of change

• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

Checklist

• [x] I have run the pre-commit checks with ./isaaclab.sh --format
• [x] I have made corresponding changes to the documentation
• [x] My changes generate no new warnings
• [ ] I have added tests that prove my fix is effective or that my feature works
• [ ] I have updated the changelog and the corresponding version in the extension's config/extension.toml file
• [x] I have added my name to the CONTRIBUTORS.md or my name already exists there

[Mayankm96]

Couple of points:

• This is a breaking change from the framework point of view. I was wondering if we should make it explicit always to use root_state_link_w and root_state_com_w attributes and add a warning on root_state_w to make users switch to either of these.
• The setters for the root state need to be adapted too. Should this now also expect velocities to be in the link frame? If so then we would need to add operations to set them in the com frame into the simulator.
• torch.bmm is a slow operation from what I know. Did you see a perf difference because of it?
• Missing tests for this change.

[jtigue-bdai]

Couple of points:

• This is a breaking change from the framework point of view. I was wondering if we should make it explicit always to use root_state_link_w and root_state_com_w attributes and add a warning on root_state_w to make users switch to either of these.
• The setters for the root state need to be adapted too. Should this now also expect velocities to be in the link frame? If so then we would need to add operations to set them in the com frame into the simulator.
• torch.bmm is a slow operation from what I know. Did you see a perf difference because of it?
• Missing tests for this change.

1. Yeah we can do the separated properties. that is easy enough.
2. I forgot about the setters, but yeah I think that would be beneficial
3. Will torch.matmul be a better alternative? It seems like it should handle a batch matrix multiplication provided the right shape. Do we have a script/test that would give a good indication? If not I will look at it when I make the tests.
4. Yeah I wanted to make sure an implementation was agreed on before make them. Now that we are going with the explicit properties I will work those tests out.

[jtigue-bdai]

@Mayankm96 are you envisioning that we have a deprecation warning for the root_state_w property and then from now on have the root_state_com_w and root_state_link_w properties? (and the same for body_state_w)

If so what we do we want to do with the sub properties root_pos_w, root_quat_w, root_vel_w, etc? Do we want deprecation warnings and _link _com versions on those too? This is going to get pretty kinda big and messy. I wonder if it will be more confusing from a user perspective.

[kellyguo11]

I guess we'll also need to extend this for RigidObjectCollection class now?

[jtigue-bdai]

Oh yeah I guess so

[jtigue-bdai]

I guess we'll also need to extend this for RigidObjectCollection class now?

@kellyguo11 These have been addressed.

[kellyguo11]

hmm after updating the environments to the new APIs, training seems to be slowing down quite a bit. For Isaac-Repose-Cube-Shadow-Direct-v0, I'm seeing ~100k FPS before the change and ~30k FPS after. These transform computations seem to be pretty costly.

[kellyguo11]

hmm after updating the environments to the new APIs, training seems to be slowing down quite a bit. For Isaac-Repose-Cube-Shadow-Direct-v0, I'm seeing ~100k FPS before the change and ~30k FPS after. These transform computations seem to be pretty costly.

I pushed a change to use the physics APIs directly for link pose and com vel. @jtigue-bdai let me know if this looks ok to you. I'll let this sit for another day before we merge it in.

[jtigue-bdai]

hmm after updating the environments to the new APIs, training seems to be slowing down quite a bit. For Isaac-Repose-Cube-Shadow-Direct-v0, I'm seeing ~100k FPS before the change and ~30k FPS after. These transform computations seem to be pretty costly.

I pushed a change to use the physics APIs directly for link pose and com vel. @jtigue-bdai let me know if this looks ok to you. I'll let this sit for another day before we merge it in.

Hey @kellyguo11, I was worried about that, those transforms are costly. Its currently a problem for the IMU too. It seems to be passing the tests. Does your change make is faster? How much of a hit do we take?

[kellyguo11]

hmm after updating the environments to the new APIs, training seems to be slowing down quite a bit. For Isaac-Repose-Cube-Shadow-Direct-v0, I'm seeing ~100k FPS before the change and ~30k FPS after. These transform computations seem to be pretty costly.

I pushed a change to use the physics APIs directly for link pose and com vel. @jtigue-bdai let me know if this looks ok to you. I'll let this sit for another day before we merge it in.

Hey @kellyguo11, I was worried about that, those transforms are costly. Its currently a problem for the IMU too. It seems to be passing the tests. Does your change make is faster? How much of a hit do we take?

yup tests are passing now. the latest change helps bring back perf to pretty much where it was before, but it would only be the case if only link pose and com velocities are used. If com pose and link velocities are required (or the full states for either of them), then we'll still have to go through the transform computations, which maybe it's ok for now since we can't really work around that.

[jtigue-bdai]

hmm after updating the environments to the new APIs, training seems to be slowing down quite a bit. For Isaac-Repose-Cube-Shadow-Direct-v0, I'm seeing ~100k FPS before the change and ~30k FPS after. These transform computations seem to be pretty costly.

I pushed a change to use the physics APIs directly for link pose and com vel. @jtigue-bdai let me know if this looks ok to you. I'll let this sit for another day before we merge it in.

Hey @kellyguo11, I was worried about that, those transforms are costly. Its currently a problem for the IMU too. It seems to be passing the tests. Does your change make is faster? How much of a hit do we take?

yup tests are passing now. the latest change helps bring back perf to pretty much where it was before, but it would only be the case if only link pose and com velocities are used. If com pose and link velocities are required (or the full states for either of them), then we'll still have to go through the transform computations, which maybe it's ok for now since we can't really work around that.

OK cool. That sounds good for now.

II think in general we need to figure out a faster way to do transforms. Maybe we can ask Physx team for getters for com_pose and link_velocities. That way the transforms are faster. Or we figure out a faster way to do transforms in IsaacLab.