Skip to content

Fix set_to_muscle() to accept tuple arguments for timeconst and range#3283

Open
anishesg wants to merge 2 commits into
google-deepmind:mainfrom
proudhare:fix/ph-issue-3282
Open

Fix set_to_muscle() to accept tuple arguments for timeconst and range#3283
anishesg wants to merge 2 commits into
google-deepmind:mainfrom
proudhare:fix/ph-issue-3282

Conversation

@anishesg
Copy link
Copy Markdown

The set_to_muscle() method in the Python bindings incorrectly declared timeconst and range parameters as C-style array pointers (double timeconst[2]), which prevented Python callers from passing tuples or lists as intended for musculoskeletal modeling. The underlying C function expects 2-element arrays representing min/max pairs, but the pybind11 binding only accepted scalar values.

Changed the lambda signature in python/mujoco/specs.cc to use std::array<double, 2> for both timeconst and range parameters, matching the pattern used in set_to_dcmotor(). Updated the default argument for timeconst from a scalar -1 to std::array<double, 2>{-1, -1} for consistency. The arrays are correctly unpacked using .data() when calling the underlying mjs_setToMuscle() C function.

Added a test in specs_test.py that verifies tuple arguments are properly mapped to the dynprm and gainprm arrays, and that muscle actuator types are set correctly.

Fixes #3282

The `set_to_muscle()` method in the Python bindings incorrectly declared `timeconst` and `range` parameters as C-style array pointers (`double timeconst[2]`), which prevented Python callers from passing tuples or lists as intended for musculoskeletal modeling. The underlying C function expects 2-element arrays representing min/max pairs, but the pybind11 binding only accepted scalar values.

Signed-off-by: anish <anishesg@users.noreply.github.com>
@anishesg anishesg marked this pull request as ready for review May 24, 2026 00:13
self.assertEqual(actuator.gainprm[5], 1.6)
self.assertEqual(actuator.gainprm[6], 1.5)
self.assertEqual(actuator.gainprm[7], 1.3)
self.assertEqual(actuator.gainprm[8], 1.2)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably also test biasprm while we are at it

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch, added

Signed-off-by: anish <anishesg@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

set_to_muscle() Uses wrong arg types

2 participants