Skip to content

Fix #5: Derive SO-ARM units and safety bounds from use_degrees#22

Merged
jeqcho merged 1 commit into
robocurve:mainfrom
theonlypal:codex/support-normalized-units
Jul 17, 2026
Merged

Fix #5: Derive SO-ARM units and safety bounds from use_degrees#22
jeqcho merged 1 commit into
robocurve:mainfrom
theonlypal:codex/support-normalized-units

Conversation

@theonlypal

Copy link
Copy Markdown
Contributor

Description

This PR fixes #5 by supporting LeRobot's normalized motor mode end to end instead of rejecting use_degrees=False.

Previously, the package always declared joint_pos as deg+normalized and used degree-mode joint bounds of ±180. Under LeRobot's normalized mode, arm joints are limited to ±100 and the gripper remains 0–100, so the declared state units were incorrect and the default clamp no longer provided an effective backstop.

Changes

  • Replace the fixed module-level STATE_SPEC with a state specification derived from use_degrees.
  • Derive automatic safety bounds from the selected mode: ±180 for degree-mode joints or ±100 for normalized joints, with the gripper at 0–100 in both modes.
  • Preserve explicitly configured calibrated limits in either mode.
  • Add use_degrees to LeRobotPolicyConfig and thread the setting through both policy and embodiment observation contracts.
  • Keep homing guidance unit-correct by describing max_relative_target in native motor units.
  • Document the matching policy/embodiment configuration requirement and normalized-mode safety behavior.
  • Add regression coverage for configuration, packing, policy metadata, embodiment clamping, compatibility, and complete evaluation flow.

Verification

  • Ruff lint and formatting pass.
  • Strict mypy passes.
  • 92 tests pass on Python 3.11 and Python 3.12.
  • Line and branch coverage remain at 100%.
  • Pre-commit and pre-push hooks pass.
  • uv lock --check passes.
  • Runtime-only import hygiene passes without torch or LeRobot installed.
  • LeRobot 0.5.1 accepts the propagated use_degrees=False driver configuration.
  • The full suite passes under a fresh compatible dependency resolution.
  • Wheel and source-distribution builds pass.

Fixes #5

@jeqcho

jeqcho commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

@theonlypal happy to take a look again once the tests are fixed!

@theonlypal

Copy link
Copy Markdown
Contributor Author

@jeqcho Hi Jay, the implementation is green. The macOS 3.12 job was rate-limited during setup-uv, before dependencies or pytest ran; ci-ok only mirrors that setup failure. Could you rerun the failed jobs?

@jeqcho jeqcho 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.

Merging — thanks @theonlypal, this was a pleasure to review.

Two things stood out. The generator-expression trick in test_soarm_normalized_mode_preserves_explicit_degree_sized_limits, keeping the ±180 tuples from aliasing the sentinel, is exactly the care identity-based defaults demand — constant folding would have silently broken a naive version of that test. And the derived ±100 defaults with per-side swapping restore a real safety backstop: the old ±180 clamp simply never fires in normalized space, so this closes an actual gap rather than just widening an input.

One tiny thing I won't hold the merge for: the module docstring in policy.py (around line 21) still says chunks are "(degrees for the joints, 0..100 for the gripper)", which is stale now that normalized mode exists. If you feel like a one-line follow-up it's yours; otherwise we'll sweep it up.

And an apology: the "failing test" I pointed you at earlier turned out to be our CI runner getting rate-limited, not your code. Nothing was ever wrong with the tests. Hope to see more from you.

@jeqcho
jeqcho merged commit d7cbf2c into robocurve:main Jul 17, 2026
16 of 18 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.

use_degrees=False: derive STATE_SPEC units and clamp bounds from the flag

2 participants