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

Fixed compatibility issues for MuJoCo >3.0 upgrade #10

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rbharadwaj9
Copy link

Followed the changelog recommendation to account for the removal of the mjOption.collision and the associated option/collision attribute.

Full Details from the CHANGELOG: https://mujoco.readthedocs.io/en/latest/changelog.html#id40

Removed mjOption.collision and the associated option/collision attribute.
Migration:

  • For models which have <option collision="all"/>, delete the attribute.
  • For models which have <option collision="dynamic"/>, delete all pair elements.
  • For models which have <option collision="predefined"/>, disable all dynamic collisions (determined via contype/conaffinity) by first deleting all contype and conaffinity attributes in the model and then setting them globally to 0 using .

checklist

PR can be merged after all these are met

  • describe the changes (with screenshots if it helps):
  1. For the collision removal: Followed the recommendations above and loaded both the faive hand and the book within MuJoCo 3.2.3 viewer using python -m mujoco.viewer
  2. Also noticed that the damping property of the joint in simple_book.xml should be a real number. I assume there was an erroneous space added so I've removed that too.
  • If this PR modifies any part of the training, post the W&B results of the following experiments (post screenshot of the consecutive_successes) No changes here.
    python train.py task=FaiveHandP0 capture_video=True force_render=False wandb_activate=True wandb_group=srl_ethz wandb_project=faive_hand wandb_name=faivehandp0_check

@Yasu31
Copy link
Contributor

Yasu31 commented Oct 24, 2024

Thanks for the PR!
I'll check if this doesn't cause any unintended issues in IsaacGym (its MJCF parser is based on mujoco<3 but it's not a perfect implementation, so sometimes it has weird errors- I'm a bit worried that removing collision attributes could cause some issues) later and merge the PR.

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.

2 participants