diff --git a/doc/changelog.rst b/doc/changelog.rst index 917e2e63..e448e8bf 100644 --- a/doc/changelog.rst +++ b/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. diff --git a/python/mjspec.ipynb b/python/mjspec.ipynb index 772faf30..886032f4 100644 --- a/python/mjspec.ipynb +++ b/python/mjspec.ipynb @@ -300,8 +300,8 @@ " \n", "\n", " \n", - " \n", - " \n", + " \n", + " \n", " \n", "\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", - "actuators = [actuator.id for actuator in arena.actuators]\n", + "torsos_data = [data.geom(geom.name) for geom in geoms if 'torso' in geom.name]\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_y.append(data.geom_xpos[torsos, 1].copy())\n", + " pos_x.append([torso.xpos[0] for torso in torsos_data])\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)" diff --git a/python/mujoco/specs.cc b/python/mujoco/specs.cc index d88a0544..725894c6 100644 --- a/python/mujoco/specs.cc +++ b/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 =================================== diff --git a/python/mujoco/specs_test.py b/python/mujoco/specs_test.py index 7bca48f0..74a34648 100644 --- a/python/mujoco/specs_test.py +++ b/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_quat[1], [0, 1, 0, 0]) + np.testing.assert_array_equal(model.body('baz').pos, [1, 2, 3]) + 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])