From 413b3f1b3e945150ec1e97285d0818b2473d1af6 Mon Sep 17 00:00:00 2001 From: tushar Date: Fri, 17 Jul 2026 20:59:26 +0530 Subject: [PATCH 1/3] fix(approver): allow delta pose modes with clampable rotation deltas 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 #143. --- CHANGELOG.md | 9 +++++++++ docs/guide/adapters.md | 2 +- src/inspect_robots/approver.py | 22 +++++++++++++++------- tests/test_approvers.py | 24 ++++++++++++++++++++++-- tests/test_conformance.py | 20 ++++++++++++++++++++ 5 files changed, 67 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a4b5aae..ef1ec94 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -79,6 +79,15 @@ All notable changes to this project are documented here. The format is based on ### Fixed +- **`DeltaLimitApprover` no longer rejects displacement pose modes with + rotation deltas** (#143). The per-dimension rotation-repr refusal now fires + only for **absolute** pose modes (`eef_abs_pose`), where clamping an absolute + euler/quat orientation has wraparound and axis-coupling problems. A + displacement pose mode (`eef_delta_pose`) carries small rotation deltas that + clamp per dimension like any other bounded displacement, so an euler-delta + embodiment (e.g. BridgeData V2's 7-D xyz+euler deltas) is now guardrail-ready: + `doctor` reports it conformant, and CLI runs keep delta limiting instead of + silently degrading to clamp-only. - **Operator scoring no longer prompts twice for self-confirming embodiments** (#53). On interactive ad-hoc runs, definitive `success` or `failure` termination verdicts are adopted as the operator judgement, announced on the diff --git a/docs/guide/adapters.md b/docs/guide/adapters.md index db8ec2b..c90c5bd 100644 --- a/docs/guide/adapters.md +++ b/docs/guide/adapters.md @@ -109,7 +109,7 @@ constructor hardware-free; connect in `reset()`). | `bounds` | Finite `low`/`high` on every dim. Without them the bounds clamp is skipped and no default delta limit can be derived. | | `dim_labels` | Every dim is named (`("left_j0", ..., "right_gripper")`), uniquely. The agent moves joints by these names. | | `state_alignment` | Absolute-target modes (`joint_pos`, `eef_abs_pose`) declare exactly one `StateSpec` field with `shape == (action_dim,)`: the proprioceptive reference the agent interpolates from. | -| `guardrails` | `DeltaLimitApprover(action_space)` constructs. This catches pose modes with rotation representations that cannot be clamped per dimension (`quat_*`, `axis_angle`, `euler_xyz`; use `none` or `rot6d`). | +| `guardrails` | `DeltaLimitApprover(action_space)` constructs. This catches **absolute** pose modes (`eef_abs_pose`) whose rotation representation cannot be clamped per dimension (`quat_*`, `axis_angle`, `euler_xyz`; use `none` or `rot6d`). Displacement pose modes (`eef_delta_pose`) carry rotation deltas and accept any representation. | ### Warnings diff --git a/src/inspect_robots/approver.py b/src/inspect_robots/approver.py index 814487e..c8a0c95 100644 --- a/src/inspect_robots/approver.py +++ b/src/inspect_robots/approver.py @@ -80,8 +80,14 @@ def review(self, action: Action, store: dict[str, Any]) -> Action: # change, limited directly). Together these cover every ControlMode literal. _ABSOLUTE_MODES = frozenset({"joint_pos", "eef_abs_pose"}) _POSE_MODES = frozenset({"eef_abs_pose", "eef_delta_pose"}) -# Rotation reps that survive independent per-dimension clamping (same set the -# EnsemblingController accepts for per-dimension averaging). +# Absolute pose modes only: clamping an absolute euler/quat orientation per +# dimension has wraparound and axis-coupling problems, so those reps are +# refused. Displacement pose modes (eef_delta_pose) carry small rotation +# *deltas*, which clamp per dimension like any other bounded displacement. +_ABSOLUTE_POSE_MODES = _ABSOLUTE_MODES & _POSE_MODES +# Rotation reps that survive independent per-dimension clamping of an absolute +# orientation (same set the EnsemblingController accepts for per-dimension +# averaging). _LIMITABLE_ROT = frozenset({"none", "rot6d"}) _LAST_APPROVED_KEY = "delta_limit:last" @@ -103,8 +109,9 @@ class DeltaLimitApprover: explicit ``max_delta`` the limiter adds nothing beyond ``ClampApprover``. Construction never guesses: missing semantics, a needed missing/non-finite - bound without an explicit ``max_delta``, or a pose mode whose rotation - representation cannot be clamped per-dimension all raise ``ValueError``. + bound without an explicit ``max_delta``, or an **absolute** pose mode whose + rotation representation cannot be clamped per-dimension all raise + ``ValueError`` (a displacement pose mode's rotation deltas clamp fine). ``NaN`` anywhere in a reviewed action raises [`SafetyAbort`][inspect_robots.errors.SafetyAbort]. A modified action is flagged ``meta["delta_clamped"]``; an unmodified one is returned as the @@ -119,10 +126,11 @@ def __init__(self, action_space: Box, max_delta: float | Any | None = None): "DeltaLimitApprover: action space declares no semantics; the " "limiter cannot tell absolute targets from displacements" ) - if sem.control_mode in _POSE_MODES and sem.rotation_repr not in _LIMITABLE_ROT: + if sem.control_mode in _ABSOLUTE_POSE_MODES and sem.rotation_repr not in _LIMITABLE_ROT: raise ValueError( - f"DeltaLimitApprover: cannot clamp rotation_repr {sem.rotation_repr!r} " - f"per dimension; only {sorted(_LIMITABLE_ROT)} are safe" + f"DeltaLimitApprover: cannot clamp absolute rotation_repr " + f"{sem.rotation_repr!r} per dimension; only {sorted(_LIMITABLE_ROT)} " + f"are safe (displacement pose modes carry rotation deltas and are fine)" ) self._absolute = sem.control_mode in _ABSOLUTE_MODES dim = action_space.dim diff --git a/tests/test_approvers.py b/tests/test_approvers.py index a9c8698..17daceb 100644 --- a/tests/test_approvers.py +++ b/tests/test_approvers.py @@ -13,7 +13,7 @@ from inspect_robots.approver import ChainApprover, ClampApprover, DeltaLimitApprover from inspect_robots.errors import SafetyAbort -from inspect_robots.spaces import ActionSemantics, Box +from inspect_robots.spaces import ActionSemantics, Box, RotationRepr from inspect_robots.types import Action @@ -44,7 +44,9 @@ def test_refuses_missing_semantics() -> None: DeltaLimitApprover(Box(shape=(2,), low=np.zeros(2), high=np.ones(2))) -def test_refuses_pose_mode_with_unaverageable_rotation() -> None: +def test_refuses_absolute_pose_mode_with_unaverageable_rotation() -> None: + # Absolute euler/quat orientations have wraparound + axis coupling, so + # per-dim clamping is unsound and refused. space = Box( shape=(7,), low=np.full(7, -1.0), @@ -55,6 +57,24 @@ def test_refuses_pose_mode_with_unaverageable_rotation() -> None: DeltaLimitApprover(space) +@pytest.mark.parametrize("rotation_repr", ["euler_xyz", "quat_wxyz", "axis_angle"]) +def test_delta_pose_rotation_deltas_clamp_per_dim(rotation_repr: RotationRepr) -> None: + # eef_delta_pose carries small rotation *deltas*, not absolute orientations, + # so any rotation_repr clamps per dimension like a bounded displacement (#143). + # BridgeData V2 shape: 3 xyz + 3 euler deltas + 1 gripper. + space = Box( + shape=(7,), + low=np.full(7, -0.1), + high=np.full(7, 0.1), + semantics=ActionSemantics("eef_delta_pose", rotation_repr=rotation_repr), + ) + approver = DeltaLimitApprover(space, max_delta=0.05) + # A large rotation-delta dim (dpitch, index 4) clamps to the ±max_delta box. + out = approver.review(Action(data=np.array([0.0, 0.0, 0.0, 0.0, 0.5, 0.0, 0.0])), {}) + assert np.isclose(out.data[4], 0.05) + assert out.meta.get("delta_clamped") is True + + def test_refuses_derived_default_without_bounds() -> None: for mode in ("joint_pos", "joint_delta"): unbounded = Box(shape=(2,), semantics=ActionSemantics(mode)) diff --git a/tests/test_conformance.py b/tests/test_conformance.py index 5e47bd6..295036f 100644 --- a/tests/test_conformance.py +++ b/tests/test_conformance.py @@ -277,6 +277,7 @@ def test_absolute_mode_needs_exactly_one_aligned_state_field() -> None: def test_unlimitable_rotation_repr_is_an_error() -> None: + # Absolute pose modes still reject unclampable rotation reps. info = _info( space=Box( shape=(7,), @@ -294,6 +295,25 @@ def test_unlimitable_rotation_repr_is_an_error() -> None: assert codes["guardrails"] == "error" +def test_delta_pose_euler_rotation_deltas_are_guardrail_conformant() -> None: + # eef_delta_pose carries rotation deltas, which clamp per dimension, so a + # euler-delta embodiment (e.g. BridgeData V2's 7-D xyz+euler deltas) is + # guardrail-conformant, not rejected (#143). + info = _info( + space=Box( + shape=(7,), + low=np.full(7, -0.1), + high=np.full(7, 0.1), + semantics=ActionSemantics( + "eef_delta_pose", + rotation_repr="euler_xyz", + dim_labels=("dx", "dy", "dz", "droll", "dpitch", "dyaw", "dgrip"), + ), + ), + ) + assert "guardrails" not in _codes(info) + + def test_missing_control_hz_is_a_warning() -> None: info = _good_absolute() silent = EmbodimentInfo( From 1bcf88ee9613dde91006acfe65260bb8024eaf5c Mon Sep 17 00:00:00 2001 From: tushar Date: Tue, 21 Jul 2026 12:21:31 +0530 Subject: [PATCH 2/3] fix(approver): refuse quaternion deltas in displacement pose mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 (#150). --- CHANGELOG.md | 20 +++++++++++--------- docs/guide/adapters.md | 2 +- src/inspect_robots/approver.py | 31 ++++++++++++++++++++++++++----- tests/test_approvers.py | 25 ++++++++++++++++++++++--- 4 files changed, 60 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ef1ec94..83c6e40 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -79,15 +79,17 @@ All notable changes to this project are documented here. The format is based on ### Fixed -- **`DeltaLimitApprover` no longer rejects displacement pose modes with - rotation deltas** (#143). The per-dimension rotation-repr refusal now fires - only for **absolute** pose modes (`eef_abs_pose`), where clamping an absolute - euler/quat orientation has wraparound and axis-coupling problems. A - displacement pose mode (`eef_delta_pose`) carries small rotation deltas that - clamp per dimension like any other bounded displacement, so an euler-delta - embodiment (e.g. BridgeData V2's 7-D xyz+euler deltas) is now guardrail-ready: - `doctor` reports it conformant, and CLI runs keep delta limiting instead of - silently degrading to clamp-only. +- **`DeltaLimitApprover` no longer rejects displacement pose modes whose + rotation deltas are safe to clamp per dimension** (#143). The per-dimension + rotation-repr refusal now fires for absolute pose modes (`eef_abs_pose`, + where clamping an absolute orientation has wraparound and axis-coupling + problems) and, separately, for quaternion deltas in displacement pose modes + (`eef_delta_pose` + `quat_wxyz`/`quat_xyzw`, whose identity is not the zero + vector, so per-dimension clamping distorts the rotation instead of limiting + it). Euler and axis-angle deltas have no such problem and clamp fine, so an + euler-delta embodiment (e.g. BridgeData V2's 7-D xyz+euler deltas) is now + guardrail-ready: `doctor` reports it conformant, and CLI runs keep delta + limiting instead of silently degrading to clamp-only. - **Operator scoring no longer prompts twice for self-confirming embodiments** (#53). On interactive ad-hoc runs, definitive `success` or `failure` termination verdicts are adopted as the operator judgement, announced on the diff --git a/docs/guide/adapters.md b/docs/guide/adapters.md index c90c5bd..7b51440 100644 --- a/docs/guide/adapters.md +++ b/docs/guide/adapters.md @@ -109,7 +109,7 @@ constructor hardware-free; connect in `reset()`). | `bounds` | Finite `low`/`high` on every dim. Without them the bounds clamp is skipped and no default delta limit can be derived. | | `dim_labels` | Every dim is named (`("left_j0", ..., "right_gripper")`), uniquely. The agent moves joints by these names. | | `state_alignment` | Absolute-target modes (`joint_pos`, `eef_abs_pose`) declare exactly one `StateSpec` field with `shape == (action_dim,)`: the proprioceptive reference the agent interpolates from. | -| `guardrails` | `DeltaLimitApprover(action_space)` constructs. This catches **absolute** pose modes (`eef_abs_pose`) whose rotation representation cannot be clamped per dimension (`quat_*`, `axis_angle`, `euler_xyz`; use `none` or `rot6d`). Displacement pose modes (`eef_delta_pose`) carry rotation deltas and accept any representation. | +| `guardrails` | `DeltaLimitApprover(action_space)` constructs. This catches absolute pose modes (`eef_abs_pose`) whose rotation representation cannot be clamped per dimension (`quat_*`, `axis_angle`, `euler_xyz`; use `none` or `rot6d`), and displacement pose modes (`eef_delta_pose`) carrying a quaternion delta (`quat_wxyz`, `quat_xyzw`; use `none`, `rot6d`, `axis_angle`, or `euler_xyz`). | ### Warnings diff --git a/src/inspect_robots/approver.py b/src/inspect_robots/approver.py index c8a0c95..f098abf 100644 --- a/src/inspect_robots/approver.py +++ b/src/inspect_robots/approver.py @@ -83,12 +83,22 @@ def review(self, action: Action, store: dict[str, Any]) -> Action: # Absolute pose modes only: clamping an absolute euler/quat orientation per # dimension has wraparound and axis-coupling problems, so those reps are # refused. Displacement pose modes (eef_delta_pose) carry small rotation -# *deltas*, which clamp per dimension like any other bounded displacement. +# *deltas*, which mostly clamp per dimension like any other bounded +# displacement — except quaternions (see _DISPLACEMENT_POSE_UNSAFE_ROT below). _ABSOLUTE_POSE_MODES = _ABSOLUTE_MODES & _POSE_MODES +_DISPLACEMENT_POSE_MODES = _POSE_MODES - _ABSOLUTE_POSE_MODES # Rotation reps that survive independent per-dimension clamping of an absolute # orientation (same set the EnsemblingController accepts for per-dimension # averaging). _LIMITABLE_ROT = frozenset({"none", "rot6d"}) +# Displacement pose modes carry rotation *deltas*, whose no-op is not the zero +# vector for these reps (a quaternion identity is (1, 0, 0, 0)). Clamping such +# a delta per dimension toward a symmetric ±max_delta box drags it away from +# identity, and downstream re-normalization can amplify the rotation instead +# of limiting it — the same failure class as clamping an absolute quaternion. +# euler_xyz/axis_angle deltas have no such problem (their no-op is the zero +# vector) and clamp fine. +_DISPLACEMENT_POSE_UNSAFE_ROT = frozenset({"quat_wxyz", "quat_xyzw"}) _LAST_APPROVED_KEY = "delta_limit:last" @@ -109,9 +119,10 @@ class DeltaLimitApprover: explicit ``max_delta`` the limiter adds nothing beyond ``ClampApprover``. Construction never guesses: missing semantics, a needed missing/non-finite - bound without an explicit ``max_delta``, or an **absolute** pose mode whose - rotation representation cannot be clamped per-dimension all raise - ``ValueError`` (a displacement pose mode's rotation deltas clamp fine). + bound without an explicit ``max_delta``, an absolute pose mode whose + rotation representation cannot be clamped per-dimension, or a displacement + pose mode carrying a quaternion delta (whose identity is not the zero + vector, so per-dimension clamping distorts it) all raise ``ValueError``. ``NaN`` anywhere in a reviewed action raises [`SafetyAbort`][inspect_robots.errors.SafetyAbort]. A modified action is flagged ``meta["delta_clamped"]``; an unmodified one is returned as the @@ -130,7 +141,17 @@ def __init__(self, action_space: Box, max_delta: float | Any | None = None): raise ValueError( f"DeltaLimitApprover: cannot clamp absolute rotation_repr " f"{sem.rotation_repr!r} per dimension; only {sorted(_LIMITABLE_ROT)} " - f"are safe (displacement pose modes carry rotation deltas and are fine)" + f"are safe" + ) + if ( + sem.control_mode in _DISPLACEMENT_POSE_MODES + and sem.rotation_repr in _DISPLACEMENT_POSE_UNSAFE_ROT + ): + raise ValueError( + f"DeltaLimitApprover: cannot clamp displacement rotation_repr " + f"{sem.rotation_repr!r} per dimension; its identity is not at the " + f"origin, so per-dimension clamping distorts it instead of " + f"limiting it (euler_xyz and axis_angle are safe here)" ) self._absolute = sem.control_mode in _ABSOLUTE_MODES dim = action_space.dim diff --git a/tests/test_approvers.py b/tests/test_approvers.py index 17daceb..49767fd 100644 --- a/tests/test_approvers.py +++ b/tests/test_approvers.py @@ -57,10 +57,12 @@ def test_refuses_absolute_pose_mode_with_unaverageable_rotation() -> None: DeltaLimitApprover(space) -@pytest.mark.parametrize("rotation_repr", ["euler_xyz", "quat_wxyz", "axis_angle"]) +@pytest.mark.parametrize("rotation_repr", ["euler_xyz", "axis_angle"]) def test_delta_pose_rotation_deltas_clamp_per_dim(rotation_repr: RotationRepr) -> None: - # eef_delta_pose carries small rotation *deltas*, not absolute orientations, - # so any rotation_repr clamps per dimension like a bounded displacement (#143). + # eef_delta_pose carries small rotation *deltas*, not absolute orientations. + # euler_xyz/axis_angle deltas have a zero-vector no-op, so they clamp per + # dimension like a bounded displacement (#143). Quaternion deltas are the + # exception — see test_refuses_displacement_pose_mode_with_quat_rotation. # BridgeData V2 shape: 3 xyz + 3 euler deltas + 1 gripper. space = Box( shape=(7,), @@ -75,6 +77,23 @@ def test_delta_pose_rotation_deltas_clamp_per_dim(rotation_repr: RotationRepr) - assert out.meta.get("delta_clamped") is True +@pytest.mark.parametrize("rotation_repr", ["quat_wxyz", "quat_xyzw"]) +def test_refuses_displacement_pose_mode_with_quat_rotation(rotation_repr: RotationRepr) -> None: + # A quaternion delta's identity is (1, 0, 0, 0), not the zero vector: + # clamping it per dimension toward a symmetric ±max_delta box drags it away + # from identity, and downstream re-normalization can amplify the rotation + # instead of limiting it — same failure class as an absolute quat. + # 3 xyz + 4 quat + 1 gripper = 8 dims. + space = Box( + shape=(8,), + low=np.full(8, -1.0), + high=np.full(8, 1.0), + semantics=ActionSemantics("eef_delta_pose", rotation_repr=rotation_repr), + ) + with pytest.raises(ValueError, match="rotation_repr"): + DeltaLimitApprover(space) + + def test_refuses_derived_default_without_bounds() -> None: for mode in ("joint_pos", "joint_delta"): unbounded = Box(shape=(2,), semantics=ActionSemantics(mode)) From 6a2eb3b1abbc8c34ec71f58a0597a2a53489ca78 Mon Sep 17 00:00:00 2001 From: tushar Date: Tue, 21 Jul 2026 12:32:37 +0530 Subject: [PATCH 3/3] fix(approver): refuse rot6d rotation deltas in displacement pose mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A rot6d delta's identity is (1, 0, 0, 0, 1, 0), not the zero vector, same problem class as the quaternion case #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 #143/#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 #144 (same branch lineage); will rebase once that lands. --- CHANGELOG.md | 10 ++++++++++ docs/guide/adapters.md | 2 +- src/inspect_robots/approver.py | 20 ++++++++++++-------- tests/test_approvers.py | 27 +++++++++++++++++---------- tests/test_conformance.py | 21 +++++++++++++++++++++ 5 files changed, 61 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 83c6e40..0992123 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -79,6 +79,16 @@ All notable changes to this project are documented here. The format is based on ### Fixed +- **`DeltaLimitApprover` refuses `rot6d` rotation deltas in displacement pose + modes** (#150, breaking for any embodiment currently declaring them). A + `rot6d` delta's identity is `(1, 0, 0, 0, 1, 0)`, not the zero vector, so + per-dimension clamping toward a symmetric `±max_delta` box drags it away + from identity; the Gram-Schmidt re-normalization every consumer applies can + then amplify the rotation instead of limiting it — the same failure class + as clamping an absolute quaternion, and pre-existing behavior rather than a + regression (`eef_delta_pose` + `rot6d` already reached the displacement + clamp path before #143/#144). `euler_xyz` and `axis_angle` deltas have no + such problem and remain guardrail-conformant. - **`DeltaLimitApprover` no longer rejects displacement pose modes whose rotation deltas are safe to clamp per dimension** (#143). The per-dimension rotation-repr refusal now fires for absolute pose modes (`eef_abs_pose`, diff --git a/docs/guide/adapters.md b/docs/guide/adapters.md index 7b51440..bfceb44 100644 --- a/docs/guide/adapters.md +++ b/docs/guide/adapters.md @@ -109,7 +109,7 @@ constructor hardware-free; connect in `reset()`). | `bounds` | Finite `low`/`high` on every dim. Without them the bounds clamp is skipped and no default delta limit can be derived. | | `dim_labels` | Every dim is named (`("left_j0", ..., "right_gripper")`), uniquely. The agent moves joints by these names. | | `state_alignment` | Absolute-target modes (`joint_pos`, `eef_abs_pose`) declare exactly one `StateSpec` field with `shape == (action_dim,)`: the proprioceptive reference the agent interpolates from. | -| `guardrails` | `DeltaLimitApprover(action_space)` constructs. This catches absolute pose modes (`eef_abs_pose`) whose rotation representation cannot be clamped per dimension (`quat_*`, `axis_angle`, `euler_xyz`; use `none` or `rot6d`), and displacement pose modes (`eef_delta_pose`) carrying a quaternion delta (`quat_wxyz`, `quat_xyzw`; use `none`, `rot6d`, `axis_angle`, or `euler_xyz`). | +| `guardrails` | `DeltaLimitApprover(action_space)` constructs. This catches absolute pose modes (`eef_abs_pose`) whose rotation representation cannot be clamped per dimension (`quat_*`, `axis_angle`, `euler_xyz`; use `none` or `rot6d`), and displacement pose modes (`eef_delta_pose`) whose rotation representation's identity is not the zero vector (`quat_*`, `rot6d`; use `none`, `axis_angle`, or `euler_xyz`). | ### Warnings diff --git a/src/inspect_robots/approver.py b/src/inspect_robots/approver.py index f098abf..10411e9 100644 --- a/src/inspect_robots/approver.py +++ b/src/inspect_robots/approver.py @@ -84,7 +84,8 @@ def review(self, action: Action, store: dict[str, Any]) -> Action: # dimension has wraparound and axis-coupling problems, so those reps are # refused. Displacement pose modes (eef_delta_pose) carry small rotation # *deltas*, which mostly clamp per dimension like any other bounded -# displacement — except quaternions (see _DISPLACEMENT_POSE_UNSAFE_ROT below). +# displacement — except reps whose no-op is not the origin (see +# _DISPLACEMENT_POSE_UNSAFE_ROT below). _ABSOLUTE_POSE_MODES = _ABSOLUTE_MODES & _POSE_MODES _DISPLACEMENT_POSE_MODES = _POSE_MODES - _ABSOLUTE_POSE_MODES # Rotation reps that survive independent per-dimension clamping of an absolute @@ -92,13 +93,16 @@ def review(self, action: Action, store: dict[str, Any]) -> Action: # averaging). _LIMITABLE_ROT = frozenset({"none", "rot6d"}) # Displacement pose modes carry rotation *deltas*, whose no-op is not the zero -# vector for these reps (a quaternion identity is (1, 0, 0, 0)). Clamping such -# a delta per dimension toward a symmetric ±max_delta box drags it away from -# identity, and downstream re-normalization can amplify the rotation instead -# of limiting it — the same failure class as clamping an absolute quaternion. -# euler_xyz/axis_angle deltas have no such problem (their no-op is the zero -# vector) and clamp fine. -_DISPLACEMENT_POSE_UNSAFE_ROT = frozenset({"quat_wxyz", "quat_xyzw"}) +# vector for these reps (a quaternion identity is (1, 0, 0, 0); a rot6d +# identity is (1, 0, 0, 0, 1, 0)). Clamping such a delta per dimension toward +# a symmetric ±max_delta box drags it away from identity, and downstream +# re-normalization (Gram-Schmidt orthonormalization for rot6d) can then +# amplify the rotation instead of limiting it — the same failure class as +# clamping an absolute quaternion. euler_xyz/axis_angle deltas have no such +# problem (their no-op is the zero vector) and clamp fine. Note rot6d is +# limitable for *absolute* orientations (_LIMITABLE_ROT above) but not for +# displacement deltas — the two sets are deliberately not related. +_DISPLACEMENT_POSE_UNSAFE_ROT = frozenset({"quat_wxyz", "quat_xyzw", "rot6d"}) _LAST_APPROVED_KEY = "delta_limit:last" diff --git a/tests/test_approvers.py b/tests/test_approvers.py index 49767fd..bb2fb76 100644 --- a/tests/test_approvers.py +++ b/tests/test_approvers.py @@ -77,17 +77,24 @@ def test_delta_pose_rotation_deltas_clamp_per_dim(rotation_repr: RotationRepr) - assert out.meta.get("delta_clamped") is True -@pytest.mark.parametrize("rotation_repr", ["quat_wxyz", "quat_xyzw"]) -def test_refuses_displacement_pose_mode_with_quat_rotation(rotation_repr: RotationRepr) -> None: - # A quaternion delta's identity is (1, 0, 0, 0), not the zero vector: - # clamping it per dimension toward a symmetric ±max_delta box drags it away - # from identity, and downstream re-normalization can amplify the rotation - # instead of limiting it — same failure class as an absolute quat. - # 3 xyz + 4 quat + 1 gripper = 8 dims. +@pytest.mark.parametrize( + ("rotation_repr", "rot_dim"), + [("quat_wxyz", 4), ("quat_xyzw", 4), ("rot6d", 6)], +) +def test_refuses_displacement_pose_mode_with_off_origin_rotation( + rotation_repr: RotationRepr, rot_dim: int +) -> None: + # These reps' identity/no-op is not the zero vector (quaternion identity is + # (1, 0, 0, 0); rot6d identity is (1, 0, 0, 0, 1, 0)): clamping a delta per + # dimension toward a symmetric ±max_delta box drags it away from identity, + # and downstream re-normalization can amplify the rotation instead of + # limiting it — same failure class as clamping an absolute quat (#150). + # 3 xyz + rot_dim + 1 gripper. + dim = 3 + rot_dim + 1 space = Box( - shape=(8,), - low=np.full(8, -1.0), - high=np.full(8, 1.0), + shape=(dim,), + low=np.full(dim, -1.0), + high=np.full(dim, 1.0), semantics=ActionSemantics("eef_delta_pose", rotation_repr=rotation_repr), ) with pytest.raises(ValueError, match="rotation_repr"): diff --git a/tests/test_conformance.py b/tests/test_conformance.py index 295036f..b4fb7eb 100644 --- a/tests/test_conformance.py +++ b/tests/test_conformance.py @@ -314,6 +314,27 @@ def test_delta_pose_euler_rotation_deltas_are_guardrail_conformant() -> None: assert "guardrails" not in _codes(info) +def test_delta_pose_rot6d_rotation_deltas_are_a_guardrail_error() -> None: + # A rot6d delta's identity is (1, 0, 0, 0, 1, 0), not the zero vector: + # per-dimension clamping distorts it and post-clamp Gram-Schmidt + # re-normalization can amplify the rotation instead of limiting it, the + # same failure class as an absolute quaternion. 3 xyz + 6 rot6d + 1 + # gripper = 10 dims. + info = _info( + space=Box( + shape=(10,), + low=np.full(10, -0.1), + high=np.full(10, 0.1), + semantics=ActionSemantics( + "eef_delta_pose", + rotation_repr="rot6d", + dim_labels=tuple("abcdefghij"), + ), + ), + ) + assert _codes(info)["guardrails"] == "error" + + def test_missing_control_hz_is_a_warning() -> None: info = _good_absolute() silent = EmbodimentInfo(