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

make unitree go2 mjx model compatible with hfields #77

Closed
wants to merge 1 commit into from

Conversation

7oponaut
Copy link

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.

  1. 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

  • The go2-floor contact is way too slippery imo, it looks unrealistic to me. I 5x'ed friction on the feet of the go2 model, since they have higher priority than the floor.
  • The tracking camera is too high for me. I set its z coordinate to 0.5 to center the go2 better.

image

Copy link

google-cla bot commented Jul 19, 2024

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.

@saran-t saran-t requested a review from erikfrey July 19, 2024 11:22
@kevinzakka
Copy link
Collaborator

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!

Copy link

@erikfrey erikfrey left a comment

Choose a reason for hiding this comment

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

Both of these changes look good. Although you can use impratio for pyramidal friction, it's more stable for elliptic cone friction.

The reason you are seeing more slippage and had to raise the friction is probably due to removing impratio, but that's OK. I'll leave it to you and @kevinzakka to discuss whether to update friction, as he knows better than me where those values came from.

@kevinzakka
Copy link
Collaborator

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

@erikfrey
Copy link

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
Copy link
Collaborator

Gotcha, then all good for increasing friction!

@kevinzakka
Copy link
Collaborator

@erikfrey Can we get this merged in?

@kevinzakka
Copy link
Collaborator

@7oponaut Can you sign the CLA?

@erikfrey
Copy link

@kevinzakka yes, once CLA is signed.

@7oponaut
Copy link
Author

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.

@7oponaut 7oponaut closed this Aug 14, 2024
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.

3 participants