More schema-related cleanup.

- Declare every nonzero default; make the defaults cross-check total.
- Skip default-valued attributes in the hand-written writer paths.
- Fix type facts on hand-read elements, found by the dm_control diff.

PiperOrigin-RevId: 958999733
Change-Id: I3064ccc6ae1f049c20f273abc234cd02990a8b7e
This commit is contained in:
Yuval Tassa
2026-08-04 07:04:38 -07:00
committed by Copybara-Service
parent 10793e539e
commit 574b6bd6bf
8 changed files with 885 additions and 469 deletions
+24 -2
View File
@@ -13,9 +13,13 @@
// limitations under the License.
// Checks that the attribute defaults declared in mjcf.schema agree with the
// C default-constructors: every generated row is compared against a
// freshly-constructed spec element.
// C default-constructors: every bound numeric attribute is compared against a
// freshly-constructed spec element. Coverage is total, so a nonzero
// constructor default that the schema does not declare is a failure: the
// schema cannot silently under-declare. Fields whose constructor value is
// the mjNAN "unset" sentinel are marked in the table and asserted NaN.
#include <cmath>
#include <map>
#include <string>
@@ -40,6 +44,14 @@ TEST_F(SchemaDefaultsTest, DeclaredDefaultsMatchConstructors) {
std::map<std::string, const void*> objects = {
{"mjOption", &spec->option},
{"mjVisual", &spec->visual},
{"mjStatistic", &spec->stat},
{"mjSpec", spec},
{"mjLROpt", &spec->compiler.LRopt},
{"mjsCompiler", &spec->compiler},
{"mjsFlex", mjs_addFlex(spec)},
{"mjsHField", mjs_addHField(spec)},
{"mjsKey", mjs_addKey(spec)},
{"mjsNumeric", mjs_addNumeric(spec)},
{"mjsBody", body},
{"mjsFrame", mjs_addFrame(body, nullptr)},
{"mjsJoint", mjs_addJoint(body, nullptr)},
@@ -69,6 +81,16 @@ TEST_F(SchemaDefaultsTest, DeclaredDefaultsMatchConstructors) {
for (int j = 0; j < entry.len; j++) {
double expected = j < entry.ndecl ? entry.value[j] : 0;
double actual = 0;
if (entry.unset && j == 0) {
// the constructor holds the mjNAN "unset" sentinel in the first
// slot (the rest are zeros), deliberately undeclared in the schema
double v = entry.kind == 4
? (double)reinterpret_cast<const mjtNum*>(field)[j]
: reinterpret_cast<const double*>(field)[j];
EXPECT_TRUE(std::isnan(v))
<< table.structname << "." << entry.attr << "[" << j << "]";
continue;
}
switch (entry.kind) {
case 0: // double
actual = reinterpret_cast<const double*>(field)[j];
+2 -2
View File
@@ -135,8 +135,8 @@ TEST_F(MujocoTest, SaveXmlShortString) {
std::array<char, 10> out;
EXPECT_THAT(mj_saveXMLString(spec, out.data(), out.size(), error.data(),
error.size()),
273);
EXPECT_STREQ(error.data(), "Output string too short, should be at least 274");
223);
EXPECT_STREQ(error.data(), "Output string too short, should be at least 224");
mj_deleteSpec(spec);
mj_deleteModel(model);
+81
View File
@@ -1638,5 +1638,86 @@ TEST_F(XMLWriterTest, EmptyFlagsAttribute) {
EXPECT_THAT(saved_xml, Not(HasSubstr("output=")));
}
TEST_F(XMLWriterTest, OmitsDefaultJointPosAxis) {
static constexpr char xml[] = R"(
<mujoco>
<worldbody>
<body name="b">
<joint name="j" type="hinge"/>
<geom size=".1"/>
</body>
</worldbody>
</mujoco>
)";
MjModelPtr model = LoadModelFromString(xml);
ASSERT_THAT(model.get(), NotNull());
std::string saved_xml = SaveAndReadXml(model.get());
EXPECT_THAT(saved_xml, Not(HasSubstr("pos=\"0 0 0\"")));
EXPECT_THAT(saved_xml, Not(HasSubstr("axis=")));
}
TEST_F(XMLWriterTest, KeepsAuthoredJointPosAxis) {
static constexpr char xml[] = R"(
<mujoco>
<worldbody>
<body name="b">
<joint name="j" type="hinge" pos="0 0 0.5" axis="1 0 0"/>
<geom size=".1"/>
</body>
</worldbody>
</mujoco>
)";
MjModelPtr model = LoadModelFromString(xml);
ASSERT_THAT(model.get(), NotNull());
std::string saved_xml = SaveAndReadXml(model.get());
EXPECT_THAT(saved_xml, HasSubstr("pos=\"0 0 0.5\""));
EXPECT_THAT(saved_xml, HasSubstr("axis=\"1 0 0\""));
}
TEST_F(XMLWriterTest, OmitsDefaultEqualityData) {
static constexpr char xml[] = R"(
<mujoco>
<worldbody>
<body name="b1"><joint name="j1" type="hinge"/><geom size=".1"/></body>
<body name="b2"><joint name="j2" type="hinge"/><geom size=".1"/></body>
</worldbody>
<equality>
<weld body1="b1" body2="b2"/>
<joint joint1="j1" joint2="j2"/>
</equality>
</mujoco>
)";
MjModelPtr model = LoadModelFromString(xml);
ASSERT_THAT(model.get(), NotNull());
std::string saved_xml = SaveAndReadXml(model.get());
EXPECT_THAT(saved_xml, Not(HasSubstr("anchor=")));
EXPECT_THAT(saved_xml, Not(HasSubstr("torquescale=")));
EXPECT_THAT(saved_xml, Not(HasSubstr("polycoef=")));
// relpose is legitimately saved: the compiler resolves the zero-quat
// "compute current pose" sentinel into the actual relative pose
EXPECT_THAT(saved_xml, HasSubstr("relpose=\"0 0 0 1 0 0 0\""));
}
TEST_F(XMLWriterTest, WritesDefaultClassJointPosAxis) {
// pos and axis set in a default class used to be lost when saving
static constexpr char xml[] = R"(
<mujoco>
<default>
<joint pos="1 2 3" axis="0 1 0"/>
</default>
<worldbody>
<body name="b">
<joint name="j" type="hinge"/>
<geom size=".1"/>
</body>
</worldbody>
</mujoco>
)";
MjModelPtr model = LoadModelFromString(xml);
ASSERT_THAT(model.get(), NotNull());
std::string saved_xml = SaveAndReadXml(model.get());
EXPECT_THAT(saved_xml, HasSubstr("<joint pos=\"1 2 3\" axis=\"0 1 0\"/>"));
}
} // namespace
} // namespace mujoco