google-deepmind / mujoco

Multi-Joint dynamics with Contact. A general purpose physics simulator.
https://mujoco.org
Apache License 2.0
8.25k stars 823 forks source link

Bazel + pypi installation of `mujoco-mjx` fails due to incorrect `mujoco` namespacing #2119

Closed hartikainen closed 1 month ago

hartikainen commented 1 month ago

Intro

Hi!

I am a MuJoCo user working on manipulation.

My setup

MuJoCo: (built from https://github.com/google-deepmind/mujoco/commit/dba32e827c22914262752de77c3dcd8346444112)

$ python -c "import mujoco; print(mujoco.__version__)"
3.2.3

API: Python

OS:

$ python -c "import platform; print(f'{platform.system()=}, {platform.release()=}, {platform.machine()=}')"
platform.system()='Darwin', platform.release()='24.0.0', platform.machine()='arm64'

What's happening? What did you expect?

I've installed both mujoco-mjx and mujoco from pypi package using Bazel's rules_python. mujoco-mjx doesn't work, however, because python can't find mjx within the mujoco namespace:

$ bazel run //:mjx_test
[...]
Traceback (most recent call last):
  File "[build-path]/bin/mjx_test.runfiles/_main/mjx_test.py", line 3, in <module>
    from mujoco import mjx
ImportError: cannot import name 'mjx' from 'mujoco' ([build-path]/bin/mjx_test.runfiles/rules_python~~pip~pypi_311_mujoco/site-packages/mujoco/__init__.py)

I investigated this a bit and believe that the Python Packaging User Guide covers what the issue here is. It basically describes two ways of handling the namespaces, neither of which MuJoCo currently satisfies:

There are currently two different approaches to creating namespace packages, from which the latter is discouraged:

Use native namespace packages. This type of namespace package is defined in PEP 420 and is available in Python 3.3 and later. This is recommended if packages in your namespace only ever need to support Python 3 and installation via pip.

Use legacy namespace packages. This comprises pkgutil-style namespace packages and pkg_resources-style namespace packages.

It would obviously be preferable to use the former, native namespace packages, way to fix this. However, I think this is non-trivial given the current state of mujoco python's module structure. Specfically, the python/mujoco/__init__.py would have to be deleted to make this work. I haven't fully figured out the details of what removing python/mujoco/__init__.py would involve, but I believe it would require at least some refactoring that would result in some sort of breaking API changes to the mujoco's namespaces.

The latter solution, legacy name space package with pkgutil-style-namespace-packages, seems like it would at least be safe and easy to implement. I believe this would require just adding the following line at the beginning of python/mujoco/__init__.py:

__path__ = __import__('pkgutil').extend_path(__path__, __name__)

If I understand correctly, this basically tells the runtime that multiple directories contribute to the same package, mujoco in this case, and thus should enable the import module to find mujoco.mjx.

cc @milutter

Steps for reproduction

Clone there repository here and run bazel run //:mjx_test from the project root.

Minimal model for reproduction

N/A

Code required for reproduction

See above.

Confirmations