Fix/delta limit rot6d displacement#153
Conversation
DeltaLimitApprover rejected any pose mode whose rotation_repr was not in
{none, rot6d}, but that guard fired for displacement pose modes too. For
eef_delta_pose the per-dim quantities are small rotation deltas, which clamp
per dimension just like any bounded displacement; only absolute orientations
(eef_abs_pose) have the wraparound and axis-coupling problems the guard
targets.
Restrict the rotation-repr refusal to absolute pose modes. The displacement
review path already clamps every dim through the box, so no runtime change is
needed there. Downstream recovers for free: conformance's guardrail probe now
reports euler-delta embodiments conformant, and CLI runs keep delta limiting
instead of degrading to clamp-only.
Closes robocurve#143.
Address review: the displacement-pose branch had no rotation-repr gate at
all, so quat_wxyz/quat_xyzw slipped through alongside the intended
euler_xyz/axis_angle. A quaternion delta's identity is (1, 0, 0, 0), not
the zero vector, so clamping it per dimension toward a symmetric
±max_delta box drags it away from identity; downstream re-normalization
can then amplify the rotation instead of limiting it (same failure class
as clamping an absolute quaternion).
- Add _DISPLACEMENT_POSE_UNSAFE_ROT = {quat_wxyz, quat_xyzw} and a second
constructor refusal, self-maintaining via _DISPLACEMENT_POSE_MODES =
_POSE_MODES - _ABSOLUTE_POSE_MODES.
- Fix the quat test's shape bug (a quat delta pose is 3 pos + 4 quat + 1
gripper = 8 dims, not 7) and split it into its own parametrized refusal
test over quat_wxyz/quat_xyzw, covering quat_xyzw for the first time.
euler_xyz/axis_angle keep the original happy-path clamp assertion.
- Drop mid-sentence bold in the docstring, adapters.md, and the CHANGELOG
entry, and update their content to describe the quat refusal accurately.
rot6d has the same identity-not-at-origin problem in displacement mode but
is intentionally out of scope here (robocurve#150).
A rot6d delta's identity is (1, 0, 0, 0, 1, 0), not the zero vector, same problem class as the quaternion case robocurve#144 fixed: per-dimension clamping toward a symmetric ±max_delta box drags it away from identity, and the Gram-Schmidt orthonormalization every consumer applies can then amplify the rotation instead of limiting it. Pre-existing behavior, not a regression: eef_delta_pose + rot6d already reached the displacement clamp path before robocurve#143/robocurve#144. - Add "rot6d" to _DISPLACEMENT_POSE_UNSAFE_ROT (now {quat_wxyz, quat_xyzw, rot6d}); the constructor refusal and its message already generalize, no change needed there. rot6d stays limitable for *absolute* orientations (_LIMITABLE_ROT is unrelated to _DISPLACEMENT_POSE_UNSAFE_ROT). - Generalize the displacement-refusal test to parametrize over (rotation_repr, rot_dim) so rot6d's 6-dim rotation and quat's 4-dim rotation each get the correct action-space shape. - Add a conformance test: a rot6d-delta embodiment is now a guardrails error, mirroring the existing euler-delta (conformant) and absolute-quat (error) tests. - Update adapters.md's guardrails row and add a CHANGELOG entry noting the breaking behavior change for any embodiment currently declaring rot6d deltas. Builds on the not-yet-merged robocurve#144 (same branch lineage); will rebase once that lands.
|
Reviewed alongside #144 and the rot6d reasoning holds up: the no-op at (1, 0, 0, 0, 1, 0) sits off-origin, so box-clamping toward zero distorts it and Gram-Schmidt downstream can turn that into an arbitrary rotation. Adding rot6d to Since this branch carries #144's commits, the path to landing is: #144 rebases and merges first, then rebase this one onto main so the diff reduces to the rot6d commit. Right now both branches share the same one-hunk CHANGELOG.md conflict with main, and CI won't run on either until the conflict clears (GitHub skips pull_request workflows when it can't build the merge commit). |
|
Understood on the sequencing — #144 is rebased and pushed (conflict-free now, force-pushed as 0611b14), so it's ready on your end. I won't rebase this branch until #144 actually lands on main, since rebasing onto my own unmerged branch right now wouldn't be the same as rebasing onto main, and could drift if anything changes during merge. Will rebase this one as soon as #144 is in. |
2 participants
Fix unsafe
rot6ddisplacement pose handling inDeltaLimitApproverSummary
DeltaLimitApproverincorrectly allowedrot6din displacement pose mode (eef_delta_pose).Unlike translation deltas, a
rot6d"no-op" is not the zero vector—it is(1, 0, 0, 0, 1, 0). Clamping each dimension independently toward a symmetric±max_deltarange distorts this representation. Since every downstream consumer re-normalizesrot6dusing Gram-Schmidt, the distorted value can become an arbitrarily large rotation.As a result, the delta limiter can amplify rotations instead of limiting them.
This is the same underlying failure mode as clamping absolute quaternions, and the same class of issue already addressed for
quat_wxyz/quat_xyzwdisplacement poses in #144.This is not a regression—the behavior existed before #143 and #144.
Changes
Validation
"rot6d"to_DISPLACEMENT_POSE_UNSAFE_ROT.{quat_wxyz, quat_xyzw, rot6d}rot6dremains supported for absolute orientation modes.eef_delta_pose) is rejected.Tests
(rotation_repr, rot_dim)pairs.rot6d(6D) action spaces.eef_delta_pose + rot6dnow raises the expected guardrails error.Documentation
adapters.mdto describe the guardrail based on the actual failure condition instead of listing individual rotation representations.CHANGELOG.md.Why this change?
rot6dcannot be safely limited using independent per-dimension clamping in displacement mode because:Rejecting this configuration is therefore the correct and safe behavior.
Checklist
ruff check .ruff format --check .mypy(strict)CHANGELOG.mdRelated
Closes #150
This branch is based on #144, since it builds directly on the
_DISPLACEMENT_POSE_MODESand_DISPLACEMENT_POSE_UNSAFE_ROTinfrastructure introduced there. It will be rebased ontomainonce #144 is merged.