From d88675a03446a85a9352062bd325b427809a3a7d Mon Sep 17 00:00:00 2001 From: Yuval Tassa Date: Fri, 13 Oct 2023 05:04:36 -0700 Subject: [PATCH] Change the default of `` from "false" to "true". PiperOrigin-RevId: 573184793 Change-Id: I24075a8d953d0a3c3ddf65c67e1e18075cd029b6 --- doc/XMLreference.rst | 3 +-- doc/changelog.rst | 40 ++++++++++++++++-------------- src/user/user_model.cc | 3 +-- src/xml/xml_native_writer.cc | 6 ++--- test/xml/xml_native_writer_test.cc | 28 +++++++++++++++------ 5 files changed, 48 insertions(+), 32 deletions(-) diff --git a/doc/XMLreference.rst b/doc/XMLreference.rst index f851d7ec..a97481a7 100644 --- a/doc/XMLreference.rst +++ b/doc/XMLreference.rst @@ -146,13 +146,12 @@ any effect. The settings here are global and apply to the entire model. .. _compiler-autolimits: -:at:`autolimits`: :at-val:`[false, true], "false"` +:at:`autolimits`: :at-val:`[false, true], "true"` This attribute affects the behavior of attributes such as "limited" (on or ), "forcelimited", "ctrllimited", and "actlimited" (on ). If "true", these attributes are unnecessary and their value will be inferred from the presence of their corresponding "range" attribute. If "false", no such inference will happen: For a joint to be limited, both limited="true" and range="min max" must be specified. In this mode, it is an error to specify a range without a limit. - |br| The default for this option will be set to "true" in an upcoming release. .. _compiler-boundmass: diff --git a/doc/changelog.rst b/doc/changelog.rst index 6b41cc5d..cc9d11ae 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -101,25 +101,29 @@ General **Migration:** Replace uses of ``mjModel.eq_active`` with ``mjData.eq_active``. -13. Added a new :ref:`dyntype`, ``filterexact``, which updates first-order filter states with + 13. Changed the default of :ref:`autolimits` from "false" to "true". This is a minor breaking + change. The potential breakage applies to models which have elements with "range" defined and "limited" not set. + Such models cannot be loaded since version 2.2.2 (July 2022). + +14. Added a new :ref:`dyntype`, ``filterexact``, which updates first-order filter states with the exact formula rather than with Euler integration. -14. Added an actuator attribute, :ref:`actearly`, which uses semi-implicit integration for +15. Added an actuator attribute, :ref:`actearly`, which uses semi-implicit integration for actuator forces: using the next step's actuator state to compute the current actuator forces at the current timestep. -15. Renamed ``actuatorforcerange`` and ``actuatorforcelimited``, introduced in the previous version to +16. Renamed ``actuatorforcerange`` and ``actuatorforcelimited``, introduced in the previous version to :ref:`actuatorfrcrange` and :ref:`actuatorfrclimited`, respectively. -16. Added the flag :ref:`eulerdamp`, which disables implicit integration of joint damping in the +17. Added the flag :ref:`eulerdamp`, which disables implicit integration of joint damping in the Euler integrator. See the :ref:`Numerical Integration` section for more details. -17. Added the flag :ref:`invdiscrete`, which enables discrete-time inverse dynamics for all +18. Added the flag :ref:`invdiscrete`, which enables discrete-time inverse dynamics for all :ref:`integrators` other than ``RK4``. See the flag documentation for more details. -18. Added :ref:`ls_iterations` and :ref:`ls_tolerance` options for adjusting +19. Added :ref:`ls_iterations` and :ref:`ls_tolerance` options for adjusting linesearch stopping criteria in CG and Newton solvers. These can be useful for performance tuning. -19. Added ``mesh_pos`` and ``mesh_quat`` fields to :ref:`mjModel` to store the normalizing transformation applied to +20. Added ``mesh_pos`` and ``mesh_quat`` fields to :ref:`mjModel` to store the normalizing transformation applied to mesh assets. Fixes `#409 `__ . -20. Added camera :ref:`resolution` attribute and :ref:`camprojection` +21. Added camera :ref:`resolution` attribute and :ref:`camprojection` sensor. If camera resolution is set to positive values, the camera projection sensor will report the location of a target site, projected onto the camera image, in pixel coordinates. -21. Added :ref:`camera` calibration attributes: +22. Added :ref:`camera` calibration attributes: - The new attributes are :ref:`resolution`, :ref:`focal`, :ref:`focalpixel`, :ref:`principal`, @@ -128,10 +132,10 @@ General attributes are specified. See the following `example model `__. - Note that these attributes only take effect for offline rendering and do not affect interactive visualisation. -22. Added multi-threaded constraint solving via :ref:`mj_island` and :ref:`mjThreadPool` to :ref:`testspeed` +23. Added multi-threaded constraint solving via :ref:`mj_island` and :ref:`mjThreadPool` to :ref:`testspeed` exposed via npoolthread flag. The `22 humanoids `__ model shows a 3x speedup compared to the single threaded simulation. -23. Implemented reversed Z rendering for better depth precision. An enum :ref:`mjtDepthMap` was added with values +24. Implemented reversed Z rendering for better depth precision. An enum :ref:`mjtDepthMap` was added with values :ref:`mjDEPTH_ZERONEAR` and :ref:`mjDEPTH_ZEROFAR`, which can be used to set the new ``readDepthMap`` attribute in :ref:`mjrContext`` to control how the depth returned by :ref:`mjr_readPixels` is mapped from ``znear`` to ``zfar``. `Contribution `__ by `Levi Burner `__. @@ -139,11 +143,11 @@ General Python bindings ^^^^^^^^^^^^^^^ -24. Fixed `#870 `__ where calling ``update_scene`` with an invalid +25. Fixed `#870 `__ where calling ``update_scene`` with an invalid camera name used the default camera. -25. Added ``user_scn`` to the :ref:`passive viewer` handle, which allows users to add custom +26. Added ``user_scn`` to the :ref:`passive viewer` handle, which allows users to add custom visualization geoms (`#1023 `__). -26. Added optional boolean keyword arguments ``show_left_ui`` and ``show_right_ui`` to the functions ``viewer.launch`` +27. Added optional boolean keyword arguments ``show_left_ui`` and ``show_right_ui`` to the functions ``viewer.launch`` and ``viewer.launch_passive``, which allow users to launch a viewer with UI panels hidden. Simulate @@ -153,21 +157,21 @@ Simulate :align: right :width: 240px -27. Added **state history** mechanism to :ref:`simulate` and the managed +28. Added **state history** mechanism to :ref:`simulate` and the managed :ref:`Python viewer`. State history can be viewed by scrubbing the History slider and (more precisely) with the left and right arrow keys. See screen capture: -28. The ``LOADING...`` label is now shown correctly. +29. The ``LOADING...`` label is now shown correctly. `Contribution `__ by `Levi Burner `__. Bug fixes ^^^^^^^^^ -29. Fixed a bug that was causing :ref:`geom margin` to be ignored during the construction of +30. Fixed a bug that was causing :ref:`geom margin` to be ignored during the construction of midphase collision trees. -30. Fixed a bug that was generating incorrect values in ``efc_diagApprox`` for weld equality constraints. +31. Fixed a bug that was generating incorrect values in ``efc_diagApprox`` for weld equality constraints. Version 2.3.7 (July 20, 2023) diff --git a/src/user/user_model.cc b/src/user/user_model.cc index db2eecda..657fc30a 100644 --- a/src/user/user_model.cc +++ b/src/user/user_model.cc @@ -82,8 +82,7 @@ mjCModel::mjCModel() { modelfiledir.clear(); //------------------------ compiler settings - // TODO(b/245077553): Toggle to true by default. - autolimits = false; + autolimits = true; boundmass = 0; boundinertia = 0; settotalmass = -1; diff --git a/src/xml/xml_native_writer.cc b/src/xml/xml_native_writer.cc index ba5d3fe9..13de9be0 100644 --- a/src/xml/xml_native_writer.cc +++ b/src/xml/xml_native_writer.cc @@ -861,9 +861,9 @@ void mjXWriter::Compiler(XMLElement* root) { if (model->boundinertia) { WriteAttr(section, "boundinertia", 1, &model->boundinertia); } - // always enable autolimits. limited attributes will be written appropriately - // TODO(b/245077553): Remove this when the default is true. - WriteAttrTxt(section, "autolimits", "true"); + if (!model->autolimits) { + WriteAttrTxt(section, "autolimits", "false"); + } } diff --git a/test/xml/xml_native_writer_test.cc b/test/xml/xml_native_writer_test.cc index 51d61f8e..114dfb64 100644 --- a/test/xml/xml_native_writer_test.cc +++ b/test/xml/xml_native_writer_test.cc @@ -255,6 +255,19 @@ TEST_F(XMLWriterTest, DropsInertialIfFromGeom) { mj_deleteModel(model); } +TEST_F(XMLWriterTest, KeepsAutoLimitsFalse) { + static constexpr char xml[] = R"( + + + + )"; + mjModel* model = LoadModelFromString(xml); + ASSERT_THAT(model, NotNull()); + std::string saved_xml = SaveAndReadXml(model); + EXPECT_THAT(saved_xml, HasSubstr("autolimits=\"false\"")); + mj_deleteModel(model); +} + TEST_F(XMLWriterTest, DoesNotKeepInferredJointLimited) { static constexpr char xml[] = R"( @@ -272,6 +285,7 @@ TEST_F(XMLWriterTest, DoesNotKeepInferredJointLimited) { std::string saved_xml = SaveAndReadXml(model); EXPECT_THAT(saved_xml, HasSubstr("range=\"-1 1\"")); EXPECT_THAT(saved_xml, Not(HasSubstr("limited=\"true\""))); + EXPECT_THAT(saved_xml, Not(HasSubstr("autolimits=\"true\""))); mj_deleteModel(model); } @@ -290,7 +304,7 @@ TEST_F(XMLWriterTest, DoesNotKeepExplicitJointLimitedIfAutoLimits) { mjModel* model = LoadModelFromString(xml); ASSERT_THAT(model, NotNull()); std::string saved_xml = SaveAndReadXml(model); - EXPECT_THAT(saved_xml, HasSubstr("autolimits=\"true\"")); + EXPECT_THAT(saved_xml, Not(HasSubstr("autolimits=\"true\""))); EXPECT_THAT(saved_xml, HasSubstr("range=\"-1 1\"")); EXPECT_THAT(saved_xml, Not(HasSubstr("limited=\"true\""))); mj_deleteModel(model); @@ -338,7 +352,7 @@ TEST_F(XMLWriterTest, DoesNotKeepInferredTendonLimited) { mjModel* model = LoadModelFromString(xml); ASSERT_THAT(model, NotNull()); std::string saved_xml = SaveAndReadXml(model); - EXPECT_THAT(saved_xml, HasSubstr("autolimits=\"true\"")); + EXPECT_THAT(saved_xml, Not(HasSubstr("autolimits=\"true\""))); EXPECT_THAT(saved_xml, HasSubstr("range=\"-1 1\"")); EXPECT_THAT(saved_xml, Not(HasSubstr("limited=\"true\""))); mj_deleteModel(model); @@ -367,7 +381,7 @@ TEST_F(XMLWriterTest, DoesNotKeepExplicitTendonLimitedIfAutoLimits) { mjModel* model = LoadModelFromString(xml); ASSERT_THAT(model, NotNull()); std::string saved_xml = SaveAndReadXml(model); - EXPECT_THAT(saved_xml, HasSubstr("autolimits=\"true\"")); + EXPECT_THAT(saved_xml, Not(HasSubstr("autolimits=\"true\""))); EXPECT_THAT(saved_xml, HasSubstr("range=\"-1 1\"")); EXPECT_THAT(saved_xml, Not(HasSubstr("limited=\"true\""))); mj_deleteModel(model); @@ -418,7 +432,7 @@ TEST_F(XMLWriterTest, DoesNotKeepInferredActlimited) { mjModel* model = LoadModelFromString(xml); ASSERT_THAT(model, NotNull()); std::string saved_xml = SaveAndReadXml(model); - EXPECT_THAT(saved_xml, HasSubstr("autolimits=\"true\"")); + EXPECT_THAT(saved_xml, Not(HasSubstr("autolimits=\"true\""))); EXPECT_THAT(saved_xml, HasSubstr("actrange=\"-1 1\"")); EXPECT_THAT(saved_xml, Not(HasSubstr("actlimited=\"true\""))); mj_deleteModel(model); @@ -442,7 +456,7 @@ TEST_F(XMLWriterTest, DoesNotKeepExplicitActlimitedIfAutoLimits) { mjModel* model = LoadModelFromString(xml); ASSERT_THAT(model, NotNull()); std::string saved_xml = SaveAndReadXml(model); - EXPECT_THAT(saved_xml, HasSubstr("autolimits=\"true\"")); + EXPECT_THAT(saved_xml, Not(HasSubstr("autolimits=\"true\""))); EXPECT_THAT(saved_xml, HasSubstr("actrange=\"-1 1\"")); EXPECT_THAT(saved_xml, Not(HasSubstr("actlimited=\"true\""))); mj_deleteModel(model); @@ -555,7 +569,7 @@ TEST_F(XMLWriterTest, DoesNotKeepInferredForcelimited) { mjModel* model = LoadModelFromString(xml); ASSERT_THAT(model, NotNull()); std::string saved_xml = SaveAndReadXml(model); - EXPECT_THAT(saved_xml, HasSubstr("autolimits=\"true\"")); + EXPECT_THAT(saved_xml, Not(HasSubstr("autolimits=\"true\""))); EXPECT_THAT(saved_xml, HasSubstr("forcerange=\"-1 1\"")); EXPECT_THAT(saved_xml, Not(HasSubstr("forcelimited=\"true\""))); mj_deleteModel(model); @@ -578,7 +592,7 @@ TEST_F(XMLWriterTest, DoesNotKeepExplicitForcelimited) { mjModel* model = LoadModelFromString(xml); ASSERT_THAT(model, NotNull()); std::string saved_xml = SaveAndReadXml(model); - EXPECT_THAT(saved_xml, HasSubstr("autolimits=\"true\"")); + EXPECT_THAT(saved_xml, Not(HasSubstr("autolimits=\"true\""))); EXPECT_THAT(saved_xml, HasSubstr("forcerange=\"-1 1\"")); EXPECT_THAT(saved_xml, Not(HasSubstr("forcelimited=\"true\""))); mj_deleteModel(model);