Change mujoco_py bindings for mujoco Deepmind bindings #2762

rodrigodelazcano · 2022-04-19T17:18:51Z

Description

This PR is the continuation of #2595. The PR updates the python bindings for the mujoco environments. The new v4 versions of the mujoco environments now use the new mujoco python bindings from DeepMind https://pypi.org/project/mujoco/.

Fixes # (issue)
This PR also fixes the contact force issue in the Ant env (only v4) at #1541

Type of change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

v2-v4 benchmark by @vwxyzjn

@vwxyzjn provided benchmarks between old mujoco environments (v2) and the newer (v4). Results are provided here #2595 (comment)

Checklist:

I have run the pre-commit checks with pre-commit run --all-files (see CONTRIBUTING.md instructions to set it up)
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes

saran-t · 2022-04-19T18:14:50Z

setup.py

@@ -14,19 +14,25 @@
 "accept-rom-license": ["autorom[accept-rom-license]~=0.4.2"],
 "box2d": ["box2d-py==2.3.5", "pygame==2.1.0"],
 "classic_control": ["pygame==2.1.0"],
- "mujoco": ["mujoco_py>=1.50, <2.0"],
+ "mujoco_py": ["mujoco_py<2.2,>=2.1"],


I have mild concerns around gym declaring a dependency on both mujoco and mujoco-py. In principle, this isn't a problem as long as only one gets imported, but if someone tries to import both it's likely that they'll run into inscrutable errors. Would you consider declaring mujoco-py as an optional dependency (extras_require)?

Ignore me: this is already in extras_require.

vwxyzjn · 2022-04-19T21:07:54Z

gym/envs/mujoco/mujoco_env.py

+ if self._mujoco_bindings.__name__ != "mujoco_py":
+ self._mujoco_bindings.mj_rnePostConstraint(self.model, self.data)


This is not commented out as in ab38034, which causes regression in Ant-v4. See https://wandb.ai/costa-huang/cleanRL/reports/-4-19-MuJoCo-v4-vs-v2-CleanRL-s-PPO--VmlldzoxODYzODM0, https://github.com/vwxyzjn/validate-new-gym-mujoco-envs

Our previous benchmark with ab38034 fixes the performance in Ant-v4 as shown below, but I don’t know how it affects performance in other envs.

Introducing bugs on purpose to avoid regression of performance makes no sense to me. It seems way more appropriate to just remove the contact forces from the observation space and reward, instead of carrying out computations pretending it is doing what it is supposed to do. It is just utterly confusing for actual roboticist willing to use mujoco with gym.

I agree with @duburcqa
Doesn't this change make the benchmark compatible with MuJoCo < 2.0? Or do all those three (mujoco-py with MuJoCo < 2.0, mujoco-py with MuJoCo >= 2.0, and mujoco) produce different results?

Theoretically, for this change in particular it is the same as mujoco < 2.0, but there are probably other internal changes in mujoco binary so I doubt the result would be exactly the same as before.

kngwyu · 2022-04-22T09:02:56Z

gym/envs/mujoco/mujoco_rendering.py

+ else:
+ for name, import_func in _ALL_RENDERERS.items():
+ try:
+ self.opengl_context = _ALL_RENDERERS["osmesa"](width, height)


Shouldn't this use name instead of osmesa?
Or

Suggested change

self.opengl_context = _ALL_RENDERERS["osmesa"](width, height)

self.opengl_context = import_func(width, height)

Yes, you are right. Thank you for pointing this out.

Trinkle23897 · 2022-04-25T14:00:36Z

gym/envs/mujoco/walker2d_v4.py

@@ -0,0 +1,278 @@
+from turtle import distance


This is an unused import for walker2d.

I'll remove this. Thanks!

rfernand2 · 2022-05-24T01:37:55Z

Thanks for all the work on this! Will this be in gym 0.24.0 and is there an ETA for that release?

* update new mujoco bindings * optional ctc_force ant-v4 * force changes * contact force weight * add ctc force range * mujoco v3 skip test * doc Ant-v4 * inverted pendulum limit control

rodrigodelazcano · 2022-05-24T03:04:03Z

Thanks for all the work on this! Will this be in gym 0.24.0 and is there an ETA for that release?

:) @jkterry1 can probably address this question better.

rodrigodelazcano · 2022-05-24T03:28:30Z

This PR is ready to be merged. Last additional change:

Ant-v4 has an option to use contact forces in its observation space. To use this feature set the use_contact_forces argument to True. Note that this option has been added because an ablation test with PPO was performed (https://github.com/vwxyzjn/validate-new-gym-mujoco-envs) and shows that there is a regression on learning when using contact forces. Results shown on image bellow.

pseudo-rnd-thoughts · 2022-05-24T09:40:02Z

@rfernand2 Yes, there is one other PR that needs approval and merging then after this PR is merged then v0.24.0 will be released.

saran-t · 2022-05-24T19:59:37Z

I know that people spent a great deal of effort validating the v4 envs and this may well be a bit late now, but please consider pinning to mujoco==2.2.0 instead of 2.1.5 for your v0.24.0 release, since that's the first open sourced version of MuJoCo. It'll allow people to actually look at source code should any future discrepancies arise.

pseudo-rnd-thoughts · 2022-05-24T21:05:02Z

@saran-t Has there been any significant changes since 2.1.5 for 2.2.0 that would change the training results you think otherwise we should be able to do that

Trinkle23897 · 2022-05-24T21:12:32Z

@pseudo-rnd-thoughts No. I ran the alignment test and it seems that 2.2.0 can align with 2.1.5 per step and can have 5% free speedup.

rodrigodelazcano mentioned this pull request Apr 19, 2022

Use mujoco bindings instead of mujoco_py #2595

Closed

saran-t reviewed Apr 19, 2022

View reviewed changes

vwxyzjn reviewed Apr 19, 2022

View reviewed changes

kngwyu reviewed Apr 22, 2022

View reviewed changes

Trinkle23897 mentioned this pull request Apr 23, 2022

C++ ways to get MjDataBodyViews google-deepmind/mujoco#251

Closed

Trinkle23897 reviewed Apr 25, 2022

View reviewed changes

rodrigodelazcano force-pushed the mujoco-bindings branch from bc2d9aa to ff8e3fb Compare May 23, 2022 14:29

rodrigodelazcano force-pushed the mujoco-bindings branch from ff8e3fb to cb7e1a3 Compare May 23, 2022 22:24

Add new MuJoCo bindings

9c97414

* update new mujoco bindings * optional ctc_force ant-v4 * force changes * contact force weight * add ctc force range * mujoco v3 skip test * doc Ant-v4 * inverted pendulum limit control

rodrigodelazcano force-pushed the mujoco-bindings branch from cb7e1a3 to 9c97414 Compare May 24, 2022 02:52

jkterry1 merged commit 3e006f3 into openai:master May 24, 2022

Trinkle23897 mentioned this pull request May 24, 2022

Upgrade mujoco to 2.2.0 sail-sg/envpool#142

Merged

wookayin mentioned this pull request May 24, 2022

Support MuJoCo 2.1.1 (including arm64 mac support) openai/mujoco-py#662

Open

pseudo-rnd-thoughts mentioned this pull request May 24, 2022

Updated mujoco to 2.2.0 from 2.1.5 #2835

Merged

vwxyzjn mentioned this pull request Oct 3, 2022

Update to support Gymnasium vwxyzjn/cleanrl#277

Closed

21 tasks

vwxyzjn mentioned this pull request Oct 19, 2022

RLops Guide vwxyzjn/cleanrl#296

Closed

Kallinteris-Andreas mentioned this pull request Dec 12, 2022

[Bug Report] MuJoCo.Ant contact forces being off by default is based on a wrong experiment Farama-Foundation/Gymnasium#214

Closed

1 task

pseudo-rnd-thoughts mentioned this pull request Oct 17, 2023

[Bug Report] Lunar Lander reset determinism Farama-Foundation/Gymnasium#728

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change mujoco_py bindings for mujoco Deepmind bindings #2762

Change mujoco_py bindings for mujoco Deepmind bindings #2762

rodrigodelazcano commented Apr 19, 2022 •

edited

Loading

saran-t Apr 19, 2022

saran-t Apr 19, 2022

vwxyzjn Apr 19, 2022 •

edited

Loading

duburcqa May 1, 2022 •

edited

Loading

kngwyu May 2, 2022

duburcqa May 2, 2022

kngwyu Apr 22, 2022

rodrigodelazcano May 7, 2022

Trinkle23897 Apr 25, 2022

rodrigodelazcano May 7, 2022

rfernand2 commented May 24, 2022

rodrigodelazcano commented May 24, 2022

rodrigodelazcano commented May 24, 2022

pseudo-rnd-thoughts commented May 24, 2022

saran-t commented May 24, 2022

pseudo-rnd-thoughts commented May 24, 2022

Trinkle23897 commented May 24, 2022 •

edited

Loading

		if self._mujoco_bindings.__name__ != "mujoco_py":
		self._mujoco_bindings.mj_rnePostConstraint(self.model, self.data)

	self.opengl_context = _ALL_RENDERERS["osmesa"](width, height)
	self.opengl_context = import_func(width, height)

Change mujoco_py bindings for mujoco Deepmind bindings #2762

Change mujoco_py bindings for mujoco Deepmind bindings #2762

Conversation

rodrigodelazcano commented Apr 19, 2022 • edited Loading

Description

Type of change

v2-v4 benchmark by @vwxyzjn

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vwxyzjn Apr 19, 2022 • edited Loading

Choose a reason for hiding this comment

duburcqa May 1, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rfernand2 commented May 24, 2022

rodrigodelazcano commented May 24, 2022

rodrigodelazcano commented May 24, 2022

pseudo-rnd-thoughts commented May 24, 2022

saran-t commented May 24, 2022

pseudo-rnd-thoughts commented May 24, 2022

Trinkle23897 commented May 24, 2022 • edited Loading

rodrigodelazcano commented Apr 19, 2022 •

edited

Loading

vwxyzjn Apr 19, 2022 •

edited

Loading

duburcqa May 1, 2022 •

edited

Loading

Trinkle23897 commented May 24, 2022 •

edited

Loading