IsaacLab icon indicating copy to clipboard operation
IsaacLab copied to clipboard

[Proposal] Change asset.data.root_state_w to output velocities at the root link frame rather than COM for all asset types

Open jtigue-bdai opened this issue 1 year ago • 3 comments

Proposal

. I propose we add processing to the data.root_state_w to transform the linear velocity to the root link frame.

It would also be beneficial to

Motivation

Currently the root_state_w of an asset provides linear velocities of the COM of the root body. Many users assume data is coming in at root frame rather than COM

Alternatives

An alternative is to create a separate property from data.root_state_w that does this. However I think if we have separate properties for at the com and at the body frame. We should make it consistent for all values in root sate (i.e. pose would also need to be consistent with COM).

Checklist

  • [x] I have checked that there is no similar issue in the repo (required)

Acceptance Criteria

Add the criteria for which this task is considered done. If not known at issue creation time, you can add this once the issue is assigned.

  • [ ] data.root_state_w provides linear velocity of the link frame instead of the com or
  • [ ] a separate properties data.root_state_link_w and data.root_state_com_w

jtigue-bdai avatar Sep 05 '24 14:09 jtigue-bdai

I like it! I was assuming data.root_state_w is base link rather than COM, and used it to calculate distance between robot and treat. Then I found out A1 is trying to fetch the treat by stretching out it hand forward and realized root_state_w is COM, hahah.

begger

zoctipus avatar Sep 05 '24 21:09 zoctipus

@zoctipus Root state returns a mix as mentioned here: https://isaac-sim.github.io/IsaacLab/source/api/lab/omni.isaac.lab.assets.html#omni.isaac.lab.assets.ArticulationData.root_state_w

The position and quaternion are of the articulation root’s actor frame. Meanwhile, the linear and angular velocities are of the articulation root’s center of mass frame.

So I don't think for your problem this is correct?

@jtigue-bdai I like your second suggestion more. We should see how much overhead it is for us to do the COM -> link transformation manually. But if it is a lazy buffer, it should be okay 👀

Mayankm96 avatar Sep 06 '24 09:09 Mayankm96

@Mayankm96 Oh I am sorry, I check code again, I meant to say root_pos_w and confused it with root_state_w(the position part of root_state_w). I was using distance calculated as norm(robot.data.root_pos_w, treat.data.root_pos_w) to calculate the reward for fetching cube. Then I got this stretching behavior and made me think maybe root_pos_w is center of mass position? I remember after I switch to body_state_w, then this stretching hand behavior disappear.

zoctipus avatar Sep 06 '24 10:09 zoctipus