[Mujoco parser] Silently ignore some attributes/elements.

[RussTedrake]

Now that MuJoCo has space in parsing_doxygen.h where we can list "silently ignored" attributes and elements, this PR goes through and silences those elements that we will never support (and reasonable users would not expected us to support).

This will help cut down a bit on the substantial noise coming out of the parser.

It also adds a few new warnings for new attributes and elements that have managed to spring up in the mujoco documentation.

+@rpoyner-tri for feature review, please.

---

This change is 

[rpoyner-tri]

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: 3 unresolved discussions, needs at least two assigned reviewers

---

multibody/parsing/parsing_doxygen.h line 39 at r1 (raw file):

@subsection mjcf_ignored_silent Tags ignored silently

The following attributes are specific to the MuJoCo solver; we have no plans to support them:

minor: recommend alpha-sorting these long lists.

---

multibody/parsing/parsing_doxygen.h line 80 at r1 (raw file):

- `/equality/connect/@solimp`

The following elements and attributes will not be supported by the MultibodyPlant parser (the require changes outside of MultibodyPlant):

typo

Suggestion:

they

---

multibody/parsing/detail_mujoco_parser.cc line 399 at r1 (raw file):

    }
    // Silently ignored: plugin.
    WarnUnsupportedElement(*node, "general");

minor: consider alpha-sorting the long lists of WarnUnsupportedElement() and WarnUnsupportedAttribute()

[jwnimmer-tri]

Reviewable status: 4 unresolved discussions, needs at least two assigned reviewers

---

multibody/parsing/parsing_doxygen.h line 80 at r1 (raw file):

- `/equality/connect/@solimp`

The following elements and attributes will not be supported by the MultibodyPlant parser (the require changes outside of MultibodyPlant):

I disagree with silently discarding many of these. The parser should be adding these. It is a shortcoming in our Parser that it does not, so we must warn when we're ignoring them.

In #19155 we started discussions how to parse the similar things in SDFormat. Yes, the parser will need to gain access to a DiagramBuilder in order to implement the feature. There's no reason we can't do that.

Perhaps things like "plugin" are correct to silently ignore, under the same reasoning as the "mujoco solver options" above -- they are not general-purpose elements, but rather specific only to MuJoCo's runtime instead of describing the scene.

However, cameras and lights and sensors are squarely in the "describing the model", not "configuring MuJoCo" and must remain as warning(s).

[RussTedrake]

multibody/parsing/parsing_doxygen.h line 80 at r1 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

I disagree with silently discarding many of these. The parser should be adding these. It is a shortcoming in our Parser that it does not, so we must warn when we're ignoring them.

In #19155 we started discussions how to parse the similar things in SDFormat. Yes, the parser will need to gain access to a DiagramBuilder in order to implement the feature. There's no reason we can't do that.

Perhaps things like "plugin" are correct to silently ignore, under the same reasoning as the "mujoco solver options" above -- they are not general-purpose elements, but rather specific only to MuJoCo's runtime instead of describing the scene.

However, cameras and lights and sensors are squarely in the "describing the model", not "configuring MuJoCo" and must remain as warning(s).

In fact, I'm already working on trying to support all of those options, but was starting to do it with a separate entry point (much like our ApplyCameraConfig, etc). The two designs I was teasing was an ApplyMujocoConfig that tries to do all, vs a preprocessor that kicks out cameraconfig, multibodyplantconfig, etc.

I didn't know about #19155, but it didn't get very far?

It's actually precisely because I was going to support those options elsewhere that I don't want them warning here.

If you do think we should be expanding the entry point of the multibody parsing to take diagrams, i'd be open to that. But that feels like a conceptual change (it's not even a multibody parser any more?) that is bigger than I was initially thinking.

[RussTedrake]

Reviewable status: 4 unresolved discussions, needs at least two assigned reviewers

---

multibody/parsing/detail_mujoco_parser.cc line 399 at r1 (raw file):

Previously, rpoyner-tri (Rick Poyner (rico)) wrote…

minor: consider alpha-sorting the long lists of WarnUnsupportedElement() and WarnUnsupportedAttribute()

They are current sorted by the order they appear in the mujoco docs. That's my preference.

---

multibody/parsing/parsing_doxygen.h line 39 at r1 (raw file):

Previously, rpoyner-tri (Rick Poyner (rico)) wrote…

minor: recommend alpha-sorting these long lists.

same as above.

[jwnimmer-tri]

Reviewable status: 4 unresolved discussions, needs at least two assigned reviewers

---

multibody/parsing/parsing_doxygen.h line 80 at r1 (raw file):

If you do think we should be expanding the entry point of the multibody parsing to take diagrams, i'd be open to that.

I actually don't know which will be the best design, but I also don't think we need to decide today. If we're going to start loading cameras(?) from MuJoCo files, we should definitely have a design discussion first and play around with some different approaches.

It's actually precisely because I was going to support those options elsewhere that I don't want them warning here.

Okay, that context helps me understand a bit more. Still, I think it seems premature to silence the warnings here, when the other sensor-loading code hasn't been landed yet? It's totally plausible that even if the camera loader is a separate class, it still owns or controls a mulbody::Parser and affirmatively disables or preempts the unknown element errors, so that the parser when used alone without the camera loader would still warn. (For example, the camera loader could chop the cameras out of the XML and then hand that remaining XML to the multibody parser to load.)

The fundamental invariant of our parser is that all input data is either loaded into our model (plant, scene graph, or whatever), or else is covered by a diagnostic.

Breaking that invariant for MuJoCo elements which specify semantically void data like the solver fine-tuning flags is probably the right trade-off, but it doesn't make sense to me why we'd break that invariant for semantically meaningful elements.

[rpoyner-tri]

Reviewable status: 3 unresolved discussions, needs at least two assigned reviewers

---

multibody/parsing/parsing_doxygen.h line 39 at r1 (raw file):

Previously, RussTedrake (Russ Tedrake) wrote…

same as above.

Fair enough. Maybe this section should include a link to relevant mujoco docs, and indicate that the ordering follows that source?