Skip to content

fix(sim/newton): an MJCF position servo keeps its damping and torque ceiling - #3862

Merged
cagataycali merged 2 commits into
strands-labs:mainfrom
cagataycali:fix/newton-mjcf-servo-gains
Sep 18, 2026
Merged

cagataycali merged 2 commits into
strands-labs:mainfrom
cagataycali:fix/newton-mjcf-servo-gains

Conversation

@cagataycali

Copy link
Copy Markdown
Member

Newton's MJCF importer reads a <position> actuator's kp into joint_target_ke and drops the rest of the servo: the dampratio MuJoCo compiles into a velocity gain (-actuator_biasprm[2]) and the forcerange that caps the torque. The joint arrives P-only with a 1e6 N m ceiling, so a held position command oscillates instead of settling. Both values are now read off the compiled model and written onto the builder before finalize.

send_action({"Rotation": 0.5}) on the shipped so100, held 3000 substeps

backend kd effort limit final max overshoot
MuJoCo (reference) compiled 3.5 N m 0.4997 0.4997 none
Newton, before 0.0 1e6 N m 0.2991 0.9559 91.2%, never settles
Newton, after 5.13 3.5 N m 0.4996 0.4996 none

so100 joint step response

Tests

tests/simulation/newton/test_mjcf_servo_gains.py (11 cases, MuJoCo-only - no Warp or GPU): the reader on dampratio / explicit kv / a plain motor, the writer landing on the DOF index rather than the joint ordinal behind a free base, and an unreadable model keeping the gains Newton did carry. tests/simulation/newton + docs scope 828 passed / 30 skipped; whole-tree graders 6183 passed / 77 skipped; ruff + mypy clean.

Serves 0.7 in #3818: a coverage matrix row is only worth publishing if the two backends mean the same thing by one command. +353 LOC, all of it the reader, its pins and the docs note.

…ceiling

Newton's MJCF importer reads a <position> actuator's kp into
joint_target_ke and drops the rest of the servo: the dampratio MuJoCo compiles
into a velocity gain (-actuator_biasprm[2]) and the forcerange that caps the
torque. The joint then arrives with joint_target_kd = 0 and a 1e6 N m ceiling,
so a constant position command oscillates instead of settling.

Measured on the shipped so100, send_action({"Rotation": 0.5}) held for 3000
substeps: the Newton backend swung between 0.054 and 0.956 rad (91.2% overshoot,
still oscillating at the end, final 0.299) where the MuJoCo backend settles on
0.4997. Reading both values off the compiled model and writing them onto the
builder before finalize takes Newton to 0.4996 with no overshoot - 0.15 mrad
from the MuJoCo reference - which is the cross-backend equivalence add_robot
advertises.
@cagataycali
cagataycali enabled auto-merge (squash) September 18, 2026 15:42

@yinsong1986 yinsong1986 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Summary

Newton's MJCF importer carries a <position> actuator's kp into joint_target_ke but drops the compiled velocity gain (-actuator_biasprm[2], from dampratio/kv) and the forcerange torque ceiling, leaving joints P-only with a 1e6 N m limit that oscillates instead of settling. This PR adds a small MuJoCo-backed reader (actuator_gains.mjcf_joint_servos) plus a writer (apply_joint_servos) that lands both values on the correct builder DOF indices before finalize, hooked into _rebuild for MJCF imports only. The hook runs under the _rebuild lock contract, only touches the two values Newton drops (kp is deliberately left to Newton's own importer), scopes writes to the just-imported robot via first_joint, and degrades gracefully (keep imported gains + log) when MuJoCo cannot compile the model — which is the pre-PR behaviour, so nothing regresses on that path.

What's good

  • DOF-index (not joint-ordinal) writes, consistent with the existing joint_qd_start mapping convention in _rebuild, with a dedicated free-base regression test pinning the trap.
  • The fallback path (unreadable MJCF keeps imported gains) is documented, narrow in effect, and regression-tested rather than asserted.
  • Empirical cross-backend validation in the description (so100 step response, MuJoCo vs Newton before/after) rather than gains-in-isolation claims.
  • Changelog fragment, docs note, ASCII-clean strings, no host paths in tests — AGENTS.md hygiene throughout.

Verification suggestions

  • Spot-check the so100 step response on a Newton-capable box: send_action({"Rotation": 0.5}), hold ~3000 substeps, confirm settling near 0.4997 with no overshoot, matching the table in the description.

@cagataycali
cagataycali merged commit d955417 into strands-labs:main Sep 18, 2026
3 checks passed
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.

2 participants