Add myolegs26 (26-muscle legs-only) model and expose composed leg specs#114
Add myolegs26 (26-muscle legs-only) model and expose composed leg specs#114cnrobbins wants to merge 2 commits into
Conversation
Vittorio-Caggiano
left a comment
There was a problem hiding this comment.
Review findings
Five issues found after a multi-angle review. Ordered by severity.
1. 🔴 gastroc_l_med_gas_l-P2_z joint range has wrong sign (myolegs26_chain.xml)
The range is "-0.0258 -0.0235" (all negative), but the equality coupler constant is +0.02498 and LEGS26_STAND_QPOS sets +0.025 for this slot. The right-side mirror correctly pairs a negative constant with a negative range. At runtime MuJoCo clamps the joint to -0.0235 while the equality solver drives it toward +0.025, producing permanent large constraint-violation forces on the left gastroc via-point throughout any simulation.
Fix: change the range to "0.0235 0.0258".
2. 🔴 Unscoped default class names in myolegs26_assets.xml
The file uses class="muscle", class="motor", class="myo_leg_touch". The wiki (docs/wiki/engineering-standards.md lines 22–26) requires scoped names: "all nested default class names must be scoped to the body part — never use generic names like wrap, marker, muscle, coll". The existing myolegs_assets.xml correctly uses myolegs_muscle, myolegs_wrap, myolegs_touch. When myolegs26 is composed with any other model that also declares class="muscle", MuJoCo will reject the compile with a duplicate-class error.
Fix: rename to myolegs26_muscle, myolegs26_motor, myolegs26_touch throughout assets and muscle XMLs.
3. 🔴 edl and fdl have mismatched biasprm[2]/gainprm[2] (myolegs26_muscle.xml)
edl_r/l: biasprm[2]=553.241 vs gainprm[2]=818.778. fdl_r/l: biasprm[2]=332.13 vs gainprm[2]=1081.909 (3.25× discrepancy). Every other muscle in the file has matching Fmax in both arrays. Verified against myolegs_muscle.xml (the 80-muscle reference): both muscles correctly use matching values there — the divergence was introduced in this PR. With gaintype/biastype="muscle", index 2 is peak isometric force; a mismatch makes passive and active force scales diverge, producing wrong ankle and toe dynamics.
Fix: set biasprm[2] equal to gainprm[2] for edl_r, edl_l, fdl_r, fdl_l.
4. 🟡 LEGS26_STAND_QPOS trailing 6 zeros outside all 6 iliopsoas joint ranges (compose.py)
Indices 41–46 map to iliopsoas_r/l-P3_x/y/z. Their ranges ([-0.0288, -0.0238], [-0.0805, -0.057], [0.0759, 0.0816] and mirrors) all exclude 0. mj_resetDataKeyframe copies qpos verbatim without clamping, so any consumer reading musculotendon geometry immediately after reset gets iliopsoas via-points at anatomically invalid positions. MuJoCo also emits limit-violation warnings on the first simulation step.
Fix: replace the 6 trailing zeros with the polycoef[0] equilibrium values at neutral hip: -0.02855, -0.07933, 0.08132, -0.02855, -0.07933, -0.08132.
5. 🟢 build_spec() missing return type annotation (__init__.py)
def build_spec(name: str): has no return type. The wiki requires type hints on all new function signatures; the sibling load() is annotated -> tuple.
Fix: add -> mujoco.MjSpec.
There was a problem hiding this comment.
Hi Calder, thanks for the PR! Here's a few things that I would suggest to change:
- We just merged mm_refactor_mjspec into dev, can you edit to PR to point to that?
- Do you think its possible to remove myolegs26_chain and myolegs26_assets if its using the same underlying kinematics chains & meshes? It would be best to prevent duplicates as much as possible.
New leg fragment in the gait2392/Seth-Ong-Tan lineage, patella removed (quadriceps insertions as moving via-points): - myolegs26_chain.xml: pelvis/femur/tibia/talus/calcn/toes subtree plus equality-coupled via-point helper bodies - myolegs26_muscle.xml: 26 muscle actuators - myolegs26_tendon.xml: 26 spatial tendons - myolegs26_assets.xml: meshes, part-scoped default classes, joint couplers, touch + jointlimit sensors, self-collision contacts
- compose.py: add LEGS26_BASE build strategy, myolegs26 registry entry, load_legs26_spec / build_legs26_base_spec, and an at-rest `stand` keyframe (LEGS26_STAND_QPOS) since the joint couplers cannot be satisfied at qpos0 - __init__.py: expose myolegs / myolegs26 through FRAGMENT_SPEC_BUILDERS and a new build_spec(name) -> mujoco.MjSpec accessor - tests/test_build_registry.py: cover the myolegs26 build strategy - correct stale "Myo Hand (MuJoCoV2.0)" header on the myolegs (80-muscle) assets/tendon files
7705361 to
601cc08
Compare
Vittorio-Caggiano
left a comment
There was a problem hiding this comment.
can we have also documentation
| # fully at-rest pose. Shipped as a keyframe because the couplers cannot be satisfied | ||
| # at qpos0. | ||
| # fmt: off | ||
| LEGS26_STAND_QPOS = ( |
There was a problem hiding this comment.
we might want to have those in a separate place instead to bake them in the compose function e.g. we could have a myolegs26_keyframes.xml
| Copyright 2020 Vikash Kumar, Vittorio Caggiano, Huawei Wang | ||
| Model :: Myo Legs 26 (MuJoCoV3.3.3) | ||
| Author :: Vikash Kumar (vikashplus@gmail.com), Vittorio Caggiano, Huawei Wang, (Charlie) Li, Cheryl Wang, Calder Robbins | ||
| source :: https://github.com/vikashplus |
There was a problem hiding this comment.
can you remove this?
| Copyright 2020 Vikash Kumar, Vittorio Caggiano, Huawei Wang | ||
| Model :: Myo Legs 26 (MuJoCoV3.3.3) | ||
| Author :: Vikash Kumar (vikashplus@gmail.com), Vittorio Caggiano, Huawei Wang, (Charlie) Li, Cheryl Wang, Calder Robbins | ||
| source :: https://github.com/vikashplus |
There was a problem hiding this comment.
can you remove this line?
There was a problem hiding this comment.
I realized this might be needed for many more files. it will need a more comprehensive check but it is outside the scope of the PR
There was a problem hiding this comment.
No worries, that was my confusion :) Will take care of it for the 26 muscle files.
Adds a reduced 26-muscle, legs-only model (myolegs26) and exposes the composed leg specs for downstream consumers.
myolegs26 is ported from the reduced leg model (Seth/Ong/Tan lineage, with EDL/FDL added from myolegs). It follows the existing models/leg convention with separate assets, chain, muscle, and tendon files, and is legs-only by design: the HAT/torso, arms, and head are removed, leaving the pelvis and both legs with a single free root joint. The skeleton matches the existing 80-muscle myolegs (shared meshes); only the joints, muscles, and tendons differ, so it ships as its own file set. The knee uses the reduced translation-coupler model, and the moving muscle-path points are kept as constrained helper bodies. A 2D sagittal (22-muscle) variant is intended to follow as a later mjspec reduction/modifiable configuration.
It is wired in like the other composed models: load("myolegs26") and build_model("myolegs26") return the compiled base (free root), build_spec("myolegs26") returns the editable pre-compile MjSpec, and FRAGMENT_SPEC_BUILDERS["myolegs26"] returns the bare fragment for attaching/merging. This also adds a top-level build_spec(name) accessor (the spec-returning mirror of load) and registers the existing 80-muscle myolegs in FRAGMENT_SPEC_BUILDERS, so both leg models are reachable for in-memory composition.
Base model has a single free root joint and the standard myosuite scene, and it ships a "stand" keyframe: an upright pose with the feet on the pedestal, oriented to the myo_sim world-frame heading (a -90deg yaw about world z). A keyframe is used rather than qpos0 because the model's equality couplers (the knee translations and the moving muscle-path points) cannot be satisfied at qpos0. Setting joint refs would shift the coupler targets and break them so the init pose is provided as a keyframe, as in the legacy model. Base files carry no scene and no keyframe so they can be attached or merged cleanly.
Validated on mujoco 3.3.3 and the latest release: myolegs26 compiles (nbody=23, njnt=41, nu=26), the "stand" keyframe is upright on the pedestal, and the full test suite passes (89 passed). No changes to existing models.