mjcf: add schema regression tests for the declarations lost in regeneration

[shoemoney]

Fixes #552.

Superseded in part by d2cf3a3, which regenerated schema.xml and restored all three declarations this PR was opened to fix. Rebased onto current main and reduced to the regression coverage, which nothing upstream added.

Checked against main at 3e9cd0b:

• sensor has its contact child again, with the same 18 attributes the original hunk restored (verified as sorted attribute sets, identical).
• jointinparent carries type="reference" reference_namespace="joint" on all nine actuator elements again.
• custom/numeric data is type="array" array_type="float" again.

Keeping the original schema hunk on top of that would have added a second <element name="contact" repeated="true" namespace="sensor"> to the same parent, shadowing the generated one. The suite still passed with the duplicate present, which is the argument for the tests rather than against them.

What remains is four tests in schema_test.py. Each asserts the behavior a user loses, not the text of the file:

• a contact sensor parses and compiles, and its geom1 reference resolves
• jointinparent is scoped to the child model on attach()
• custom/numeric accepts array data
• no attribute of type="reference" omits its reference_namespace

The last one is the file-level invariant. schema.py:152-153 falls back to the attribute's own name when reference_namespace is absent, so a dropped namespace is invisible wherever the two coincide and silently wrong where they do not. Asserting on the file catches the loss in both cases.

These three declarations were lost once and came back through a regeneration rather than a guard, so the next regeneration can drop them again. That is what the tests are for.

Verification, against the mutation each test is meant to catch:

• Clean main: dm_control/mjcf 158 passed, schema_test.py 4 passed.
• With the tests added, no schema change: 158 passed, schema_test.py 8 passed.
• Reverting the contact element, jointinparent and numeric data on top of current main: 3 failed, 5 passed.
• Stripping one reference_namespace: fails with ['site'] has length of 1.

[sylvesterkaczmarek]

One attach-time namespace case is still missing: the restored contact sensor declares site as a plain reference. MuJoCo’s generated dm_control schema uses type="reference" reference_namespace="site". Without that namespace, attaching a child model with a contact sensor configured by site leaves the child site name unprefixed and can fail compilation, the same class of regression this PR fixes for jointinparent. Please restore reference_namespace="site" and add an attach regression for the site-based contact sensor path.

[shoemoney]

Made the change in de77a71, and it should be made, but the reason given for it does not hold and the regression you asked for cannot fail. Both worth saying plainly.

The change is right. site was declared as a bare type="reference" while body1, body2, subtree1, subtree2, geom1 and geom2 in the same element all carry a namespace. Stronger than that: grep -c 'type="reference"/>' over the whole schema returned 1, and it was that line. It was the only bare reference in the file, which is exactly the shape of loss this PR exists to undo.

The stated consequence does not reproduce. dm_control/mjcf/schema.py:152-153:

other_kwargs['reference_namespace'] = (
    attribute_xml.get('reference_namespace') or name)

An absent reference_namespace falls back to the attribute's own name, and the attribute is named site, so the resolved namespace is "site" either way. Measured on the attach case you described, with and without the declaration:

• the emitted XML is byte-identical, <contact name="child/cs" num="1" data="found" site="child/s"/> in both, so the child site is not left unprefixed
• root.find('sensor', 'cs').site resolves to an _AttachableElement in both
• renaming the target site updates the reference in both

So it does not fail compilation, and an attach regression for it passes with the bug present. I wrote the test you asked for first and it passed against the unfixed schema, which is the only reason I went looking for why.

What I added instead asserts the invariant that can actually break: no attribute of type="reference" may omit its namespace. Without the fix it fails with ['site'] has length of 1. That catches the same class of regeneration loss as jointinparent, checked on the file rather than on one code path, so it still fires in the cases where the name-based fallback happens to mask the symptom. schema_test.py: 8 passed.

The fallback is also why this was easy to miss when the element was restored, and why the file-level assertion seemed worth more than a per-attribute one.

[sylvesterkaczmarek]

Rechecked de77a71f. You are right that the attach-time symptom I described is masked when the attribute name and fallback namespace are both site. Restoring the explicit namespace is still correct, and the file-level invariant that every reference declares its namespace is a stronger regression for this regeneration class. No remaining blocker from me.

[shoemoney]

Rebased onto current main and cut this down to the tests.

d2cf3a3 already restored all three declarations. The sensor contact element is back with the same 18 attributes (compared as sorted attribute sets, identical), jointinparent carries reference_namespace="joint" on all nine actuators, and custom/numeric data is a float array. So the schema hunk is redundant and is dropped.

Worth noting why it is dropped rather than left in. Upstream added its contact element at a different point in the file, so the rebase merged cleanly and produced two <element name="contact" repeated="true" namespace="sensor"> under the same parent. The full dm_control/mjcf suite passed with that duplicate in place, 158 passed, so nothing would have caught it at review time.

What is left is four tests. Three cover the declarations that were lost. The fourth asserts that no attribute of type="reference" omits its reference_namespace, which is the file-level version of the same regression class.

Each one is checked against the mutation it exists to catch, on top of current main:

• reverting the contact element, jointinparent and numeric data: 3 failed, 5 passed
• stripping one reference_namespace: fails with ['site'] has length of 1
• unmodified main, tests added: dm_control/mjcf 158 passed, schema_test.py 8 passed

Head is b89132a. Happy to close this instead if the generator is the intended home for the guard, but the three declarations came back through a regeneration rather than a check, so the next one can drop them again.

[sylvesterkaczmarek]

Rechecked current b89132a. The PR is now reduced to regression coverage only because current main already restored the three schema declarations. The four tests cover the contact sensor, jointinparent attach scoping, numeric array data, and the file-level invariant that every reference declares a namespace. That is a cleaner guard against future regeneration loss than carrying redundant schema edits. No blocker from me.