google-deepmind / mujoco

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

Support loading mesh specified by package:// URIs when loading URDF models #1432

Open traversaro opened 6 months ago

traversaro commented 6 months ago

Is your feature request related to a problem? Please describe.

A common problem we are experiencing with users using MuJoCo with iCub models (see https://github.com/robotology/icub-models/issues/155 for a related issue) are related to the fact that the MuJoCo URDF parser strips any path information from the filename attribute of meshes. See https://github.com/google-deepmind/mujoco/blob/7c534a066c8d61bf87f7818624baf6fbe500c17b/src/xml/xml_urdf.cc#L565-L572 for the related snippet code. It would be great if MuJoCo had at least an option to avoid this behaviour, and instead actually resolve URDF's package:// URIs.

Describe the solution you'd like

Possible solutions include look for meshes specified via package:// URIs by inspecting the values of ROS_PACKAGE_PATH (for ROS1-style packages) or AMENT_PREFIX_PATH (for ROS2-style packages). This can be done without any specific dependency on ROS via some dedicated code. For an example of how this is implemented in other libraries that consume URDFs like iDynTree or Pinocchio see the following snippets:

For reference, a similar logic is implemented in the following minimal Python package https://github.com/ami-iit/resolve-robotics-uri-py/blob/main/src/resolve_robotics_uri_py/resolve_robotics_uri_py.py#L19-L68 . This code is not directly usable in the mujoco C++ URDF parser, but it just meant to show how the logic works.

Describe alternatives you've considered

An alternative choice done by Drake, is instead not to hardcode the lookup for any environment variable inside the library itself, but rather expose an API for users to specify the directories in which to look for files specified via package:// URIs, see https://github.com/RobotLocomotion/drake-external-examples/issues/170 for an example of this. While this may require more work on the users to actually successfully parse URDF files, that would also a way to fix the problem.

Additional context

I will be happy to eventually work on this, but I would like to understand if maintainers are interested in this.

traversaro commented 6 months ago

fyi @CarlottaSartore @giotherobot @siddharthdeore @diegoferigo @flferretti @akhilsathuluri

saran-t commented 6 months ago

This should be done as a resource provider.

traversaro commented 6 months ago

This should be done as a resource provider.

Thanks, I built a (quick and dirty) prototype using the resource provider API, available at https://github.com/traversaro/mujoco-urdf-package-uri .

I tried to build it as a project external to mujoco, so that I could use it also with existing version of MuJoCo, but unfortunately I had to modify MuJoCo itself due to the code in https://github.com/google-deepmind/mujoco/blob/7c534a066c8d61bf87f7818624baf6fbe500c17b/src/xml/xml_urdf.cc#L565-L572 that was always stripping all the path and the extension from the meshes. So I did the modifications in https://github.com/traversaro/mujoco/commit/d3d1ff019e967dbe56f028a7884a4d32a22d053a .

In the example, then I am able to load unmodified URDF files for Panda and iCub:

Screenshot 2024-02-24 201148

Screenshot 2024-02-24 194420

At this point, probably we need to understand if you are interested in having such as resource provider inside MuJoCo itself, or if you imagine this as something that would live outside of mujoco.

yuvaltassa commented 6 months ago

Re having to modify MuJoCo, did strippath="false" (docs) not work for you?

I think it makes sense for this to be in MuJoCo itself, but I'll let @saran-t comment on that.

traversaro commented 6 months ago

Re having to modify MuJoCo, did strippath="false" (docs) not work for you?

Indeed it works fine, I was just confusing between meshname and meshfile in https://github.com/google-deepmind/mujoco/blob/7c534a066c8d61bf87f7818624baf6fbe500c17b/src/xml/xml_urdf.cc#L565-L572 , sorry for the noise! I modified the example to do that in https://github.com/traversaro/mujoco-urdf-package-uri/blob/main/examples/example.py#L13-L33 .

Actually, working on this get me a more clear understanding of the problem. I was a bit biased on my expectation on how a simulator loads URDFs by how Gazebo does that, however the MuJoCo case it is a bit different:

Given that and observing how users in my lab and around GitHub are using MuJoCo+URDF, it seems to me there are two main workflows:

As it seems to me that probably the core MuJoCo team is more familiar with Workflow1 then Workflow2, probably it could make sense for mujoco-urdf-automatic-massaging library to be developed outside of mujoco, at least initially.

TL;DR: After working a bit on this, I am not sure I think it is really worth to have package:// URI support in MuJoCo itself. For me we could also close the issue, but obviously if someone wants to comment he is welcome.

fyi @CarlottaSartore @giotherobot @siddharthdeore @diegoferigo @flferretti @akhilsathuluri @francesco-romano

akhilsathuluri commented 6 months ago

One place where I see the utility of a package_path resolver is when we deal with predefined meshes (as modules) that are assembled as a part of a larger simulation. For example, when using a library like Timor or DoD, where one ships module meshes along with the library that a user can assemble. In this case, auto-resolving paths is helpful, keeping the generated URDFs, any additional external meshes used, and the pre-defined meshes separate. That being said, Workflow2 works fine in this case too.

traversaro commented 6 months ago

One place where I see the utility of a package_path resolver is when we deal with predefined meshes (as modules) that are assembled as a part of a larger simulation. For example, when using a library like Timor or DoD, where one ships module meshes along with the library that a user can assemble. In this case, auto-resolving paths is helpful, keeping the generated URDFs, any additional external meshes used, and the pre-defined meshes separate. That being said, Workflow2 works fine in this case too.

Indeed this is why I was discussing about the fact that you can't "include" a URDF in a bigger file in MuJoCo in https://github.com/google-deepmind/mujoco/issues/1432#issuecomment-1962918624 . Without the "include" feature, I guess there is less need for automatically resolving package:/ URIs, right

akhilsathuluri commented 6 months ago

Indeed this is why I was discussing about the fact that you can't "include" a URDF in a bigger file in MuJoCo in #1432 (comment) . Without the "include" feature, I guess there is less need for automatically resolving package:/ URIs, right

That is correct. If you want to "include" a URDF into a bigger file, then it is perhaps not so useful. But what I meant, is to use raw meshes and create a URDF on the fly, for example via odio_urdf.

traversaro commented 6 months ago

Indeed this is why I was discussing about the fact that you can't "include" a URDF in a bigger file in MuJoCo in #1432 (comment) . Without the "include" feature, I guess there is less need for automatically resolving package:/ URIs, right

That is correct. If you want to "include" a URDF into a bigger file, then it is perhaps not so useful. But what I meant, is to use raw meshes and create a URDF on the fly, for example via odio_urdf.

We aligned in person with @akhilsathuluri on this, and we agree that this anyhow is not strictly relevant for this issue, just a general comment on the usage of URDF and package://.