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

get_state and set_state do not involve the pos and quat of the mocap body #473

Open
zichunxx opened this issue Apr 17, 2024 · 5 comments
Open

Comments

@zichunxx
Copy link

Hi Robosuite team,

I constructed an arena including a mocap body whose pose should be reset at the beginning of each episode.

However, I found set_state and get_state in binding_utils.py only save and load data.qpos and data.qvel:

def get_state(self):
"""Return MjSimState instance for current state."""
return MjSimState(
time=self.data.time,
qpos=np.copy(self.data.qpos),
qvel=np.copy(self.data.qvel),
)
def set_state(self, value):
"""
Set internal state from MjSimState instance. Should
call @forward afterwards to synchronize derived quantities.
"""
self.data.time = value.time
self.data.qpos[:] = np.copy(value.qpos)
self.data.qvel[:] = np.copy(value.qvel)

This way of saving and loading MoJoCo data ignores the mocap body information. Thus, I made some changes to set_state, get_state, and from_flattened to contain mocap body information. These changes are sufficient and I have verified them with my code.

If I am right and the repo is open to contributions, I'll pull a request for you to check.

Thanks.

@zichunxx zichunxx changed the title get_state and set_state_from_flattened do not involve the pos and quat of the mocap body get_state and set_state do not involve the pos and quat of the mocap body Apr 17, 2024
@zhuyifengzju
Copy link
Contributor

Hi, thanks for bringing up this issue to our attention. Our repo is open to the community contribution, but we need to discuss and decide how to incorporate this new change. To make the codebase compatible with default behaviors in existing versions, can you make sure you have an additional flag that opt in / out the mocap body saving? Thanks!

@amandlek What do you think of this new feature?

@zichunxx
Copy link
Author

To make the codebase compatible with default behaviors in existing versions, can you make sure you have an additional flag that opt in / out the mocap body saving? Thanks!

Actually, I added a default mocap=False label for get_state and set_state_from_flattened functions and created another MjSimStateMocap class referring to MjSimState to save mocap information. Maybe it's not elegant.

@amandlek
Copy link
Member

Thanks for this contribution! Why don't we start by having you open a PR so we can review the changes?

@zichunxx
Copy link
Author

I have pulled a request for you to check. Thanks.

@Steven-xzr
Copy link

Hi @zichunxx! I'm also trying to add mocap bodies to the robosuite codebase these days. Maybe we could have a more detailed discussion on it if it's possible? You can contact me via email [email protected]. Thanks.

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

No branches or pull requests

4 participants