RobotLocomotion / drake

Model-based design and verification for robotics.
https://drake.mit.edu
Other
3.17k stars 1.24k forks source link

[Mujoco parser] Don't call CalcSpatialInertia unless needed #21665

Open RussTedrake opened 2 days ago

RussTedrake commented 2 days ago

Resolves #21648.

This defers the call to CalcSpatialInertia until/unless it is actually needed. If inertia values are specified for the body in the xml file, then we don't call CalcSpatialInertia. This allows us to successfully parse models where CalcSpatialInertia fails on the obj, but the calculation was not needed.

Adds the Rethink Robotics Sawyer (Apache 2.0) model to the detail_mujoco_parser_test; parsing fails on this model without the fix in this PR.

+@SeanCurtis-tri for feature review, please. (Rico is platform tomorrow). I will open a separate issue for the (seemingly very common) failures in CalcSpatialInertia from obj.


This change is Reviewable

jwnimmer-tri commented 12 hours ago

I haven't studied the code, but please be aware of one extremely important invariant that seems like it might be at risk here --

When our parser fails to load some input file fully and correctly into the plant / scene graph, at the very least the user must receive a diagnostic.Warning message about the parse failure(s).

If we are using inertia-from-OBB to avoid geometry bugs, but the input file told us to do something different, then we need to warn the user that the parsed model is not going to match what they expected.

xuchenhan-tri commented 3 hours ago

multibody/parsing/detail_mujoco_parser.cc line 806 at r3 (raw file): and I meant

const SpatialInertia M_GG_G_one = calculator.Calc(*geom.shape);