mujoco icon indicating copy to clipboard operation
mujoco copied to clipboard

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

Open traversaro opened this issue 1 year ago • 9 comments

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:

  • iDynTree: https://github.com/robotology/idyntree/blob/ff407448de6fbc635353dbdb31dcc3d913f15820/src/model/src/SolidShapes.cpp#L216-L308
  • pinocchio: https://github.com/stack-of-tasks/pinocchio/blob/0caf0ca4d07e63834cdc420c703993662c59e01b/src/parsers/urdf/geometry.cpp#L460-L462 and https://github.com/stack-of-tasks/pinocchio/blob/0caf0ca4d07e63834cdc420c703993662c59e01b/src/utils/file-explorer.cpp#L55

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 avatar Feb 20 '24 16:02 traversaro

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

traversaro avatar Feb 20 '24 17:02 traversaro

This should be done as a resource provider.

saran-t avatar Feb 20 '24 18:02 saran-t

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.

traversaro avatar Feb 24 '24 19:02 traversaro

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.

yuvaltassa avatar Feb 25 '24 03:02 yuvaltassa

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:

  • You can't include URDF models in bigger MJCF models by include elements, as done instead in Gazebo
  • As you can't include URDF models in bigger MJFC models, you can't specify the option that specify how such URDF is parsed, while instead those options needs to be specified as elements of the URDF itself.
  • Also from the code, you can't specify how the URDF parser parses the URDF from the code, you always need to modify the URDF file or string itself to specify if you want to strippath, or discardvisual, or similar.

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:

  • Workflow1: You just load URDF once, and then you manually "massage" it to obtain a .mjfc, and then you commit and maintain the mjfc somewhere. This is what is mentioned by @kevinzakka in https://github.com/google-deepmind/mujoco/issues/273#issuecomment-1124060013 and described in the READMEs of mujoco_menagerie , in URDF → MJCF derivation steps or MJCF and URDF sections
  • Workflow2: You want to load the URDF everytime (typically because the URDF is itself generated by something else, and is not something static that you want to manually manage). In this case, you have some code to automatically perform the "massaging" of the URDF → MJCF. Handling the package:// URIs is just a step of many small "massaging" step that would need to be done anyhow, so I am not sure what we gain by having package:// support inside MuJoCo itself, perhaps it would be easier to have a mujoco-urdf-automatic-massaging library that simplifies the "automatic massaging" of the URDF → MJCF, that also includes the handling of package:// (in the sense in setting the meshdir and transforming mesh package:// URIs in path relative to filedir).

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

traversaro avatar Feb 25 '24 12:02 traversaro

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.

akhilsathuluri avatar Feb 26 '24 07:02 akhilsathuluri

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

traversaro avatar Feb 26 '24 08:02 traversaro

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.

akhilsathuluri avatar Feb 26 '24 11:02 akhilsathuluri

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://.

traversaro avatar Feb 26 '24 12:02 traversaro