Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MuJoCo parser] Support parsing models from mujoco menagerie #20444

Closed
RussTedrake opened this issue Oct 29, 2023 · 8 comments · Fixed by #22481
Closed

[MuJoCo parser] Support parsing models from mujoco menagerie #20444

RussTedrake opened this issue Oct 29, 2023 · 8 comments · Fixed by #22481
Assignees

Comments

@RussTedrake
Copy link
Contributor

RussTedrake commented Oct 29, 2023

The MuJoCo XML parser loads and properly displays all of the models in the DeepMind Control Suite (or will after my upcoming set of PRs) except for dog.xml, which is an outlier with tendons and skin, etc. I've opened this issue to keep track of the (sad) state of our current MuJoCo parser in loading the menagerie assets.

All of the models are now parsed in detail_mujoco_parser_examples_test.cc, with a clear demarkation about any remaining errors that prevent the model from loading properly.

A number of models still fail to load properly load the visual textures.

To reproduce, run e.g.

from pydrake.all import ModelVisualizer, StartMeshcat, PackageMap

visualizer = ModelVisualizer(meshcat=meshcat)
visualizer.parser().package_map().AddRemote(
    package_name="mujoco_menagerie",
    params=PackageMap.RemoteParams(
        # This repository doesn't have up-to-date tags/releases; the scary
        # hash in the url is the most recent commit sha at the time of my
        # writing.
        urls=[
            f"https://github.com/google-deepmind/mujoco_menagerie/archive/bf110a75a56e5bd146c7ed76965737c71d48425d.tar.gz"
        ],
        sha256=("2562301c38ac82b593b31e523004b94c263b20952f99d2bcbb9939fa5c6bebd2"),
        strip_prefix="mujoco_menagerie-bf110a75a56e5bd146c7ed76965737c71d48425d/",
    ),
)
visualizer.AddModels(
    url="package://mujoco_menagerie/unitree_a1/a1.xml"
)
visualizer.Run(loop_once=True)
@RussTedrake RussTedrake self-assigned this Oct 29, 2023
RussTedrake added a commit to RussTedrake/drake that referenced this issue Oct 29, 2023
Towards RobotLocomotion#20232. These were required to achieve the status reported in RobotLocomotion#20444.
RussTedrake added a commit to RussTedrake/drake that referenced this issue Dec 3, 2023
Towards RobotLocomotion#20232. These were required to achieve the status reported in RobotLocomotion#20444.
RussTedrake added a commit to RussTedrake/drake that referenced this issue Dec 3, 2023
Towards RobotLocomotion#20232.  Related to RobotLocomotion#20444.

In RobotLocomotion#20232 we realized that the default eulerseq in MJCF is "xyz",
which is NOT the standard eulerseq in urdf/sdf. This PR fixes a bug in
which we assumed the wrong eulerseq by default, leading to incorrect
kinematics in some parsed models. It also adds support for non-default
eulerseq.

It also adds a test so that we can readily visualize the parsed
kinematics from the dm_control suite which are being used to test the
parser.
RussTedrake added a commit to RussTedrake/drake that referenced this issue Dec 3, 2023
Mesh defaults can be used to set the scale of the assets globally.
"assetdir" is another way to set the "meshdir".

Towards RobotLocomotion#20444.
RussTedrake added a commit to RussTedrake/drake that referenced this issue Dec 3, 2023
Towards RobotLocomotion#20444.

As discussed in
https://drakedevelopers.slack.com/archives/C43KX47A9/p1697895553790089
, this PR:
- avoids registering collision geometry for geom with contype==conaffinity==0
- avoids registering visual geometry for group > 2.
RussTedrake added a commit to RussTedrake/drake that referenced this issue Dec 3, 2023
Towards RobotLocomotion#20232.  Related to RobotLocomotion#20444.

In RobotLocomotion#20232 we realized that the default eulerseq in MJCF is "xyz",
which is NOT the standard eulerseq in urdf/sdf. This PR fixes a bug in
which we assumed the wrong eulerseq by default, leading to incorrect
kinematics in some parsed models. It also adds support for non-default
eulerseq.

It also adds a test so that we can readily visualize the parsed
kinematics from the dm_control suite which are being used to test the
parser.
RussTedrake added a commit to RussTedrake/drake that referenced this issue Jan 1, 2024
Towards RobotLocomotion#20444.

As discussed in
https://drakedevelopers.slack.com/archives/C43KX47A9/p1697895553790089
, this PR:
- avoids registering collision geometry for geom with contype==conaffinity==0
- avoids registering visual geometry for group > 2.

Also implements, per the mujoco documentation, that zero-radius
spheres are the default geometry if no other type/size is mentioned.
RussTedrake added a commit to RussTedrake/drake that referenced this issue Jan 1, 2024
Towards RobotLocomotion#20232.  Related to RobotLocomotion#20444.

In RobotLocomotion#20232 we realized that the default eulerseq in MJCF is "xyz",
which is NOT the standard eulerseq in urdf/sdf. This PR fixes a bug in
which we assumed the wrong eulerseq by default, leading to incorrect
kinematics in some parsed models. It also adds support for non-default
eulerseq.

It also adds a test so that we can readily visualize the parsed
kinematics from the dm_control suite which are being used to test the
parser.
RussTedrake added a commit to RussTedrake/drake that referenced this issue Jan 1, 2024
Mesh defaults can be used to set the scale of the assets globally.
"assetdir" is another way to set the "meshdir".

Towards RobotLocomotion#20444.
RussTedrake added a commit to RussTedrake/drake that referenced this issue Jan 3, 2024
Towards RobotLocomotion#20232.  Related to RobotLocomotion#20444.

In RobotLocomotion#20232 we realized that the default eulerseq in MJCF is "xyz",
which is NOT the standard eulerseq in urdf/sdf. This PR fixes a bug in
which we assumed the wrong eulerseq by default, leading to incorrect
kinematics in some parsed models. It also adds support for non-default
eulerseq.

It also adds a test so that we can readily visualize the parsed
kinematics from the dm_control suite which are being used to test the
parser.
RussTedrake added a commit to RussTedrake/drake that referenced this issue Jan 4, 2024
Mesh defaults can be used to set the scale of the assets globally.
"assetdir" is another way to set the "meshdir".

Towards RobotLocomotion#20444.
RussTedrake added a commit to RussTedrake/drake that referenced this issue Jan 4, 2024
Mesh defaults can be used to set the scale of the assets globally.
"assetdir" is another way to set the "meshdir".

Towards RobotLocomotion#20444.
RussTedrake added a commit that referenced this issue Jan 4, 2024
Towards #20232.  Related to #20444.

In #20232 we realized that the default eulerseq in MJCF is "xyz",
which is NOT the standard eulerseq in urdf/sdf. This PR fixes a bug in
which we assumed the wrong eulerseq by default, leading to incorrect
kinematics in some parsed models. It also adds support for non-default
eulerseq.

It also adds a test so that we can readily visualize the parsed
kinematics from the dm_control suite which are being used to test the
parser.
rpoyner-tri added a commit that referenced this issue Jan 5, 2024
Mesh defaults can be used to set the scale of the assets globally.
"assetdir" is another way to set the "meshdir".

Towards #20444.

Co-Authored-By: Rick Poyner <[email protected]>
RussTedrake added a commit that referenced this issue Jan 8, 2024
…20622)

Towards #20444.

As discussed in
https://drakedevelopers.slack.com/archives/C43KX47A9/p1697895553790089
, this PR:
- avoids registering collision geometry for geom with contype==conaffinity==0
- avoids registering visual geometry for group > 2.

Also implements, per the mujoco documentation, that zero-radius
spheres are the default geometry if no other type/size is mentioned.
@RussTedrake
Copy link
Contributor Author

RussTedrake commented Jun 29, 2024

See this (TRI internal) slack discussion for discussion about the extent to which we want to support "the legacy, non-standard, MuJoCo texture projection".

RussTedrake added a commit to RussTedrake/drake that referenced this issue Jun 30, 2024
RussTedrake added a commit to RussTedrake/drake that referenced this issue Jun 30, 2024
RussTedrake added a commit to RussTedrake/drake that referenced this issue Jul 1, 2024
@RussTedrake
Copy link
Contributor Author

#21666 resolves some errors. I've just done a complete sweep through the menagerie and made a detailed list of errors/warnings above.

RussTedrake added a commit to RussTedrake/drake that referenced this issue Jan 5, 2025
RussTedrake added a commit to RussTedrake/drake that referenced this issue Jan 6, 2025
The InertiaCalculator is not supposed to throw, but could still throw
in debug mode based on how it was constructing an intermediate
RotationalInertia. Now we skip the intermediate check and properly
catch the error to display a helpful message in the parser.

Towards RobotLocomotion#20444.
@RussTedrake
Copy link
Contributor Author

RussTedrake commented Jan 6, 2025

OK, I've addressed all of the small errors I know about. I've also create make_drake_compatible_model that will preprocess an xml (or urdf) to overcome all of the known errors.

A few more things on my list before I close out this issue:

  • Advertise make_drake_compatible_model from the parser. Is it ok to advertise the manipulation repo version? Or is migrating this to drake a prerequisite?
  • Escalate warnings for missing meshes to errors. It results in incorrect dynamics. And now that make_drake_compatible_model is in better shape, there is a reasonable work-around.
  • Capture warnings about missing textures in the detail_mujoco_parser_examples_test; make sure they are representative of the experience in meshcat, at least.
  • Resolve missing textures... possibly in make_drake_compatible_model, if's it's not currently viable in the parser. I think the immediate work-around is to generate an .mtl for the .obj that gets generated. (or gltf, if that is now to be preferred)

@jwnimmer-tri , @rpoyner-tri -- would appreciate any feedback you have on these.

@jwnimmer-tri
Copy link
Collaborator

Advertise make_drake_compatible_model from the parser.

One nice tactic for situations like this would be have the parser's message hyperlink to a (new) anchor on https://drake.mit.edu/troubleshooting.html and then have that documentation explain in more detail (and link to work-arounds like that tool). We don't want the parser message to be too brief (it should still be self-contained and directly explain the problem), but IMO explaining work-arounds is best left to the website.

Or alternatively, if there is a specific issue filed we can link to the issue instead of the troubleshooting, and the issue OP can contain an explanation of the workarounds, and people can subscribe to the issue to track progress.

RussTedrake added a commit to RussTedrake/drake that referenced this issue Jan 7, 2025
The InertiaCalculator is not supposed to throw, but could still throw
in debug mode based on how it was constructing an intermediate
RotationalInertia. Now we skip the intermediate check and properly
catch the error to display a helpful message in the parser.

Towards RobotLocomotion#20444.
rpoyner-tri pushed a commit to rpoyner-tri/drake that referenced this issue Jan 7, 2025
The InertiaCalculator is not supposed to throw, but could still throw
in debug mode based on how it was constructing an intermediate
RotationalInertia. Now we skip the intermediate check and properly
catch the error to display a helpful message in the parser.

Towards RobotLocomotion#20444.
rpoyner-tri pushed a commit that referenced this issue Jan 7, 2025
The InertiaCalculator is not supposed to throw, but could still throw
in debug mode based on how it was constructing an intermediate
RotationalInertia. Now we skip the intermediate check and properly
catch the error to display a helpful message in the parser.

Towards #20444.
RussTedrake added a commit to RussTedrake/drake that referenced this issue Jan 11, 2025
RussTedrake added a commit to RussTedrake/drake that referenced this issue Jan 12, 2025
The multibody parsers, in particular, had a habit of being very noisy,
since if an unsupported element or attribute is used once it is likely
used many times. This actually made it hard to visually inspect the
output to see the unique set of unsupported tags. This change
significantly improves the output for examples like in RobotLocomotion#20444.
@jwnimmer-tri
Copy link
Collaborator

... would appreciate any feedback you have on these.

Escalate warnings for missing meshes to errors. It results in incorrect dynamics.

SGTM

Capture warnings about missing textures in the detail_mujoco_parser_examples_test; ...

No opinion.

Resolve missing textures ...

I don't understand enough of the problem to offer an opinion.

@RussTedrake
Copy link
Contributor Author

I've added mujoco texture support to make_drake_compatible_model in the manipulation repo. This example notebook loads all of the robot models from the menagerie and visualizes them in meshcat. Apart from a few missing textures on a few model elements, and a few funky normals/windings picked up during the mesh file conversions, everything model now works or has a specific Drake issue tracking it.

RussTedrake added a commit that referenced this issue Jan 17, 2025
Previously if a mesh was unsupported, then we only produceed a warning
and ignored the associated geometry. Now it is an error. (We can use
make_drake_compatible_mesh in the manipulation repo to batch convert
stl to obj, etc).

Also removes the "if you said stl but I can find a juxtaposed obj,
I'll use it" logic.

The non-uniform mesh warning was turned to an error (non-uniform
meshes already threw an error, but later in the pipeline).

All of the errors now reference
http://drake.mit.edu/troubleshooting.html which points to
make_drake_compatible_model.

Resolves #20444.  Hoorah!
RussTedrake added a commit that referenced this issue Jan 17, 2025
Previously if a mesh was unsupported, then we only produceed a warning
and ignored the associated geometry. Now it is an error. (We can use
make_drake_compatible_mesh in the manipulation repo to batch convert
stl to obj, etc).

Also removes the "if you said stl but I can find a juxtaposed obj,
I'll use it" logic.

The non-uniform mesh warning was turned to an error (non-uniform
meshes already threw an error, but later in the pipeline).

All of the errors now reference
http://drake.mit.edu/troubleshooting.html which points to
make_drake_compatible_model.

Resolves #20444.  Hoorah!
@sherm1 sherm1 closed this as completed in 4382061 Jan 18, 2025
@github-project-automation github-project-automation bot moved this from Todo to Done in #dynamics (Drake board) Jan 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants