google-deepmind / mujoco_menagerie

A collection of high-quality models for the MuJoCo physics engine, curated by Google DeepMind.
Other
1.45k stars 202 forks source link

make unitree go2 mjx model compatible with hfields #77

Closed 7oponaut closed 3 months ago

7oponaut commented 4 months ago

I had two issues when attempting to use unitree_go2/go2_mjx.xml in scenes with hfields such as google_barkour_vb/scene_hfield_mjx.xml:

1) the margin attribute in the go2 class default section results in the following error:

NotImplementedError: (mjtGeom.mjGEOM_HFIELD, mjtGeom.mjGEOM_SPHERE) margin/gap not implemented.

2) the impratio="100" option setting makes the scene unstable with mjx acceleration (specifically, I encountered nan qpos values)

This PR removes these two attributes, resulting in no margin and an impratio="1" default.

I don't have a full understanding of how these changes affect the simulation. Please determine if they should be merged.

@saran-t requested this PR after encountering my post about the instability on X.

p.s. My version of the file has additional changes which might be helpful

image

google-cla[bot] commented 4 months ago

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

kevinzakka commented 4 months ago

Thanks for these @7oponaut. We should probably check if the same changes should be made to the other Unitree MJX variants. Will wait on @erikfrey to review!

kevinzakka commented 4 months ago

@erikfrey I'm okay with increasing friction, but since elliptic is now supported by MJX, should we not switch to impratio + elliptic?

erikfrey commented 4 months ago

As a rule of thumb, for locomotion you can rely on pyramidal friction - it's cheaper to simulate but still transfers to real just fine.

kevinzakka commented 4 months ago

Gotcha, then all good for increasing friction!

kevinzakka commented 3 months ago

@erikfrey Can we get this merged in?

kevinzakka commented 3 months ago

@7oponaut Can you sign the CLA?

erikfrey commented 3 months ago

@kevinzakka yes, once CLA is signed.

7oponaut commented 3 months ago

Sorry for taking your time, I was under the impression that I am contributing to an open source repository.

I am not comfortable with signing the CLA.

I'm taking issue with 3) in the CLA: I'm not a lawyer or a Google employee and I don't want to deal with litigatory issues in case Google later commercializes any of this repository and gets sued for whatever reason.