Remove id attribute from mjSpec objects in Python bindings.

Using ids is error prone in scenarios of repeated attachment and detachment. Python users are encouraged to use names for unique identification of model elements.

PiperOrigin-RevId: 699227286
Change-Id: Ifd83e6d85d36ff72ea43caf8b82eab9e4d552440

doc/changelog.rst

@@ -6,6 +6,11 @@ Changelog
 Upcoming version (not yet released)
 -----------------------------------
 
+General
+^^^^^^^
+- Removed id attribute from :ref:`mjSpec` objects in Python bindings. Using ids is error prone in scenarios of repeated
+  attachment and detachment. Python users are encouraged to use names for unique identification of model elements.
+
 MJX
 ^^^
 - Added muscle actuators.

python/mjspec.ipynb

@@ -300,8 +300,8 @@
     "  </worldbody>\n",
     "\n",
     "  <actuator>\n",
-    "    <position joint=\"hip\" kp=\"10\"/>\n",
+    "    <position joint=\"hip\" kp=\"10\" name=\"hip\"/>\n",
-    "    <position joint=\"knee\" kp=\"10\"/>\n",
+    "    <position joint=\"knee\" kp=\"10\" name=\"knee\"/>\n",
     "  </actuator>\n",
     "</mujoco>\n",
     "\"\"\"\n",
@@ -450,14 +450,16 @@
    "source": [
     "#@title Video of the movement{vertical-output: true}\n",
     "\n",
+    "data = mj.MjData(model)\n",
     "duration = 10   # (Seconds)\n",
     "framerate = 30  # (Hz)\n",
     "video = []\n",
     "pos_x = []\n",
     "pos_y = []\n",
     "geoms = arena.worldbody.find_all(mj.mjtObj.mjOBJ_GEOM)\n",
-    "torsos = [geom.id for geom in geoms if 'torso' in geom.name]\n",
+    "torsos_data = [data.geom(geom.name) for geom in geoms if 'torso' in geom.name]\n",
-    "actuators = [actuator.id for actuator in arena.actuators]\n",
+    "torsos_model = [model.geom(geom.name) for geom in geoms if 'torso' in geom.name]\n",
+    "actuators = [data.actuator(actuator.name) for actuator in arena.actuators]\n",
     "\n",
     "# Control signal frequency, phase, amplitude.\n",
     "freq = 5\n",
@@ -465,17 +467,17 @@
     "amp = 0.9\n",
     "\n",
     "# Simulate, saving video frames and torso locations.\n",
-    "data = mj.MjData(model)\n",
     "mj.mj_resetData(model, data)\n",
     "with mj.Renderer(model) as renderer:\n",
     "  while data.time < duration:\n",
     "    # Inject controls and step the physics.\n",
-    "    data.ctrl[actuators] = amp * np.sin(freq * data.time + phase)\n",
+    "    for i, actuator in enumerate(actuators):\n",
+    "      actuator.ctrl = amp * np.sin(freq * data.time + phase[i])\n",
     "    mj.mj_step(model, data)\n",
     "\n",
     "    # Save torso horizontal positions using name indexing.\n",
-    "    pos_x.append(data.geom_xpos[torsos, 0].copy())\n",
+    "    pos_x.append([torso.xpos[0] for torso in torsos_data])\n",
-    "    pos_y.append(data.geom_xpos[torsos, 1].copy())\n",
+    "    pos_y.append([torso.xpos[1] for torso in torsos_data])\n",
     "\n",
     "    # Save video frames.\n",
     "    if len(video) < data.time * framerate:\n",
@@ -496,7 +498,7 @@
    "source": [
     "#@title Movement trajectories{vertical-output: true}\n",
     "\n",
-    "creature_colors = model.geom_rgba[torsos][:, :3]\n",
+    "creature_colors = [torso.rgba[:3] for torso in torsos_model]\n",
     "fig, ax = plt.subplots(figsize=(4, 4))\n",
     "ax.set_prop_cycle(color=creature_colors)\n",
     "_ = ax.plot(pos_x, pos_y, linewidth=4)"

python/mujoco/specs.cc

@@ -407,8 +407,6 @@ PYBIND11_MODULE(_specs, m) {
   });
 
   // ============================= MJSBODY =====================================
-  mjsBody.def_property_readonly(
-      "id", [](raw::MjsBody& self) -> int { return mjs_getId(self.element); });
   mjsBody.def(
       "add_freejoint",
       [](raw::MjsBody& self, py::kwargs kwargs) -> raw::MjsJoint* {
@@ -650,8 +648,6 @@ PYBIND11_MODULE(_specs, m) {
       py::return_value_policy::reference_internal);
 
   // ============================= MJSFRAME ====================================
-  mjsFrame.def_property_readonly(
-      "id", [](raw::MjsFrame& self) -> int { return mjs_getId(self.element); });
   mjsFrame.def("delete", [](raw::MjsFrame& self) { mjs_delete(self.element); });
   mjsFrame.def("set_frame", [](raw::MjsFrame& self, raw::MjsFrame& frame) {
     mjs_setFrame(self.element, &frame);
@@ -671,8 +667,6 @@ PYBIND11_MODULE(_specs, m) {
       py::return_value_policy::reference_internal);
 
   // ============================= MJSGEOM =====================================
-  mjsGeom.def_property_readonly(
-      "id", [](raw::MjsGeom& self) -> int { return mjs_getId(self.element); });
   mjsGeom.def("delete", [](raw::MjsGeom& self) { mjs_delete(self.element); });
   mjsGeom.def("set_frame", [](raw::MjsGeom& self, raw::MjsFrame& frame) {
     mjs_setFrame(self.element, &frame);
@@ -688,8 +682,6 @@ PYBIND11_MODULE(_specs, m) {
       py::return_value_policy::reference_internal);
 
   // ============================= MJSJOINT ====================================
-  mjsJoint.def_property_readonly(
-      "id", [](raw::MjsJoint& self) -> int { return mjs_getId(self.element); });
   mjsJoint.def("delete", [](raw::MjsJoint& self) { mjs_delete(self.element); });
   mjsJoint.def("set_frame", [](raw::MjsJoint& self, raw::MjsFrame& frame) {
     mjs_setFrame(self.element, &frame);
@@ -705,8 +697,6 @@ PYBIND11_MODULE(_specs, m) {
       py::return_value_policy::reference_internal);
 
   // ============================= MJSSITE =====================================
-  mjsSite.def_property_readonly(
-      "id", [](raw::MjsSite& self) -> int { return mjs_getId(self.element); });
   mjsSite.def("delete", [](raw::MjsSite& self) { mjs_delete(self.element); });
   mjsSite.def("set_frame", [](raw::MjsSite& self, raw::MjsFrame& frame) {
     mjs_setFrame(self.element, &frame);
@@ -735,9 +725,6 @@ PYBIND11_MODULE(_specs, m) {
       py::return_value_policy::reference_internal);
 
   // ============================= MJSCAMERA ===================================
-  mjsCamera.def_property_readonly("id", [](raw::MjsCamera& self) -> int {
-    return mjs_getId(self.element);
-  });
   mjsCamera.def("delete",
                 [](raw::MjsCamera& self) { mjs_delete(self.element); });
   mjsCamera.def("set_frame", [](raw::MjsCamera& self, raw::MjsFrame& frame) {
@@ -754,8 +741,6 @@ PYBIND11_MODULE(_specs, m) {
       py::return_value_policy::reference_internal);
 
   // ============================= MJSLIGHT ====================================
-  mjsLight.def_property_readonly(
-      "id", [](raw::MjsLight& self) -> int { return mjs_getId(self.element); });
   mjsLight.def("delete", [](raw::MjsLight& self) { mjs_delete(self.element); });
   mjsLight.def("set_frame", [](raw::MjsLight& self, raw::MjsFrame& frame) {
     mjs_setFrame(self.element, &frame);
@@ -771,9 +756,6 @@ PYBIND11_MODULE(_specs, m) {
       py::return_value_policy::reference_internal);
 
   // ============================= MJSMATERIAL =================================
-  mjsMaterial.def_property_readonly("id", [](raw::MjsMaterial& self) -> int {
-    return mjs_getId(self.element);
-  });
   mjsMaterial.def("delete",
                   [](raw::MjsMaterial& self) { mjs_delete(self.element); });
   mjsMaterial.def("set_default",
@@ -788,8 +770,6 @@ PYBIND11_MODULE(_specs, m) {
       py::return_value_policy::reference_internal);
 
   // ============================= MJSMESH =====================================
-  mjsMesh.def_property_readonly(
-      "id", [](raw::MjsMesh& self) -> int { return mjs_getId(self.element); });
   mjsMesh.def("delete", [](raw::MjsMesh& self) { mjs_delete(self.element); });
   mjsMesh.def("set_default", [](raw::MjsMesh& self, raw::MjsDefault& def) {
     mjs_setDefault(self.element, &def);
@@ -802,8 +782,6 @@ PYBIND11_MODULE(_specs, m) {
       py::return_value_policy::reference_internal);
 
   // ============================= MJSPAIR =====================================
-  mjsPair.def_property_readonly(
-      "id", [](raw::MjsPair& self) -> int { return mjs_getId(self.element); });
   mjsPair.def("delete", [](raw::MjsPair& self) { mjs_delete(self.element); });
   mjsPair.def("set_default", [](raw::MjsPair& self, raw::MjsDefault& def) {
     mjs_setDefault(self.element, &def);
@@ -816,9 +794,6 @@ PYBIND11_MODULE(_specs, m) {
       py::return_value_policy::reference_internal);
 
   // ============================= MJSEQUAL ====================================
-  mjsEquality.def_property_readonly("id", [](raw::MjsEquality& self) -> int {
-    return mjs_getId(self.element);
-  });
   mjsEquality.def("delete",
                   [](raw::MjsEquality& self) { mjs_delete(self.element); });
   mjsEquality.def("set_default",
@@ -833,9 +808,6 @@ PYBIND11_MODULE(_specs, m) {
       py::return_value_policy::reference_internal);
 
   // ============================= MJSACTUATOR =================================
-  mjsActuator.def_property_readonly("id", [](raw::MjsActuator& self) -> int {
-    return mjs_getId(self.element);
-  });
   mjsActuator.def("delete",
                   [](raw::MjsActuator& self) { mjs_delete(self.element); });
   mjsActuator.def("set_default",
@@ -850,9 +822,6 @@ PYBIND11_MODULE(_specs, m) {
       py::return_value_policy::reference_internal);
 
   // ============================= MJSTENDON ===================================
-  mjsTendon.def_property_readonly("id", [](raw::MjsTendon& self) -> int {
-    return mjs_getId(self.element);
-  });
   mjsTendon.def("delete",
                 [](raw::MjsTendon& self) { mjs_delete(self.element); });
   mjsTendon.def("set_default", [](raw::MjsTendon& self, raw::MjsDefault& def) {
@@ -890,64 +859,38 @@ PYBIND11_MODULE(_specs, m) {
       py::return_value_policy::reference_internal);
 
   // ============================= MJSSENSOR ===================================
-  mjsSensor.def_property_readonly("id", [](raw::MjsSensor& self) -> int {
-    return mjs_getId(self.element);
-  });
   mjsSensor.def("delete",
                 [](raw::MjsSensor& self) { mjs_delete(self.element); });
 
   // ============================= MJSFLEX =====================================
-  mjsFlex.def_property_readonly(
-      "id", [](raw::MjsFlex& self) -> int { return mjs_getId(self.element); });
   mjsFlex.def("delete", [](raw::MjsFlex& self) { mjs_delete(self.element); });
 
   // ============================= MJSHFIELD ===================================
-  mjsHField.def_property_readonly("id", [](raw::MjsHField& self) -> int {
-    return mjs_getId(self.element);
-  });
   mjsHField.def("delete",
                 [](raw::MjsHField& self) { mjs_delete(self.element); });
 
   // ============================= MJSSKIN =====================================
-  mjsSkin.def_property_readonly(
-      "id", [](raw::MjsSkin& self) -> int { return mjs_getId(self.element); },
-      py::return_value_policy::reference_internal);
   mjsSkin.def("delete", [](raw::MjsSkin& self) { mjs_delete(self.element); });
 
   // ============================= MJSTEXTURE ==================================
-  mjsTexture.def_property_readonly("id", [](raw::MjsTexture& self) -> int {
-    return mjs_getId(self.element);
-  });
   mjsTexture.def("delete",
                  [](raw::MjsTexture& self) { mjs_delete(self.element); });
 
   // ============================= MJSKEY ======================================
-  mjsKey.def_property_readonly(
-      "id", [](raw::MjsKey& self) -> int { return mjs_getId(self.element); });
   mjsKey.def("delete", [](raw::MjsKey& self) { mjs_delete(self.element); });
 
   // ============================= MJSTEXT =====================================
-  mjsText.def_property_readonly(
-      "id", [](raw::MjsText& self) -> int { return mjs_getId(self.element); });
   mjsText.def("delete", [](raw::MjsText& self) { mjs_delete(self.element); });
 
   // ============================= MJSNUMERIC ==================================
-  mjsNumeric.def_property_readonly("id", [](raw::MjsNumeric& self) -> int {
-    return mjs_getId(self.element);
-  });
   mjsNumeric.def("delete",
                  [](raw::MjsNumeric& self) { mjs_delete(self.element); });
 
   // ============================= MJSEXCLUDE ==================================
-  mjsExclude.def_property_readonly("id", [](raw::MjsExclude& self) -> int {
-    return mjs_getId(self.element);
-  });
   mjsExclude.def("delete",
                  [](raw::MjsExclude& self) { mjs_delete(self.element); });
 
   // ============================= MJSTUPLE ====================================
-  mjsTuple.def_property_readonly(
-      "id", [](raw::MjsTuple& self) -> int { return mjs_getId(self.element); });
   mjsTuple.def("delete", [](raw::MjsTuple& self) { mjs_delete(self.element); });
 
   // ============================= MJSPLUGIN ===================================

python/mujoco/specs_test.py

@@ -92,18 +92,11 @@ class SpecsTest(absltest.TestCase):
     self.assertEqual(site.type, mujoco.mjtGeom.mjGEOM_BOX)
     np.testing.assert_array_equal(site.userdata, [1, 2, 3, 4, 5, 6])
 
-    # Check that the site and body have no id before compilation.
-    self.assertEqual(body.id, -1)
-    self.assertEqual(site.id, -1)
-
     # Compile the spec and check for expected values in the model.
     model = spec.compile()
-    self.assertEqual(spec.worldbody.id, 0)
-    self.assertEqual(body.id, 1)
-    self.assertEqual(site.id, 0)
     self.assertEqual(model.nbody, 2)  # 2 bodies, including the world body
-    np.testing.assert_array_equal(model.body_pos[1], [1, 2, 3])
+    np.testing.assert_array_equal(model.body('baz').pos, [1, 2, 3])
-    np.testing.assert_array_equal(model.body_quat[1], [0, 1, 0, 0])
+    np.testing.assert_array_equal(model.body('baz').quat, [0, 1, 0, 0])
     self.assertEqual(model.nsite, 1)
     self.assertEqual(model.nuser_site, 6)
     np.testing.assert_array_equal(model.site_user[0], [1, 2, 3, 4, 5, 6])