Farama-Foundation / Gymnasium

An API standard for single-agent reinforcement learning environments, with popular reference environments and related utilities (formerly Gym)
https://gymnasium.farama.org
MIT License
7.14k stars 793 forks source link

[Bug Report] Gymnasium asks for mujoco-py bindings in V4 envs when mujoco-py is installed #896

Closed MischaPanch closed 7 months ago

MischaPanch commented 8 months ago

Describe the bug

It's a bit of a strange case, but easy to run in. For creating a V4 env, mujoco bindings are not necessary. So, after installing pip install 'gymnasium[mujoco]' one can directly call gymnasium.make("Ant-v4").

However, if after that one also does pip install mujoco-py, then the same code triggers

...
Exception: 
You appear to be missing MuJoCo.  We expected to find the file here: /home/mpanchen/.mujoco/mujoco210

The creation of a task in v4 should not depend on whether mujoco-py is installed or not. I am guessing the control-flow somewhere is dependent on trying to import mujoco-py, causing this behavior. Since mujoco-py is not necessary for v4 envs, this is a bug. The solution is to uninstall mujoco-py, but that's far from obvious after reading the error message, and I haven't found any info on that online.

Proposal: remove the import-dependent control flow. If you want, I can prepare a PR for that

Code example

No response

System info

No response

Additional context

No response

Checklist

pseudo-rnd-thoughts commented 8 months ago

Ok, this is a bit weird and a product of the class structure. On gymnasium.make("Ant-v4"), if you only have mujoco installed then there isn't an issue but if you have mujoco_py installed then this is also imported. Therefore, if you have mujoco_py but not the executable then even when you just want Ant-v4 this will raise an error.

@MischaPanch I would just uninstall mujoco_py and this should fix your error

@Kallinteris-Andreas You understand the mujoco env classes better than I, is it possible to split them such that a mujoco env won't load mujoco-py at all even if installed?

MischaPanch commented 8 months ago

It's not an acute problem for me, I figured out that I should uninstall it. Just wanted to report the bug :). Let me know if you'd like my help for resolving it, I'd be happy to get more involved in Gymnasium development since I'm using it heavily

Kallinteris-Andreas commented 8 months ago

Yes, the problem is that if mujoco-py and a mujoco or mujoco-py is installed, then mujoco-py will be attempted to imported which causes this error message

https://github.com/Farama-Foundation/Gymnasium/blob/de909da30a1fe40288275da331fa7ad8e219a2bc/gymnasium/envs/mujoco/mujoco_env.py#L12-L17

as far as I can tell, this "Error" is not catchable.

try:
    import mujoco_py
except Exception as e:
    MUJOCO_PY_IMPORT_ERROR = e
else:
    MUJOCO_PY_IMPORT_ERROR = None

The only solution that could work, would be to factorize MuJocoPyEnv to its own file

MischaPanch commented 8 months ago

Should I prepare a PR for that? I would use git split (a script written by a colleague of mine) to carry over the git history to the new file

pseudo-rnd-thoughts commented 8 months ago

@rodrigodelazcano: Why didn't we split the mujoco and mujoco-py environments? Simplicity? I believe we had a conversation

Kallinteris-Andreas commented 8 months ago

Yes, please make a PR (git split is not required, but I do not mind if you use it)

MuJocoPyEnv should be moved to a new file named mujoco_py_env.py. MuJocoPyEnv should still be a superclass of BaseMujocoEnv.

MischaPanch commented 8 months ago

Yes, please make a PR (git split is not required, but I do not mind if you use it)

MuJocoPyEnv should be moved to a new file named mujoco_py_env.py. MuJocoPyEnv should still be a superclass of BaseMujocoEnv.

Will do!