Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,17 @@ All notable changes to this project are documented here. The format is based on
catch the `ConfigError` raised by `Task`'s epoch validation and surface it
through the existing `_resolve_or_exit` pattern, matching how invalid
constructor kwargs are handled for config-file components (#47).
- **`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
Expand Down
2 changes: 1 addition & 1 deletion docs/guide/adapters.md
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,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`), and displacement pose modes (`eef_delta_pose`) carrying a quaternion delta (`quat_wxyz`, `quat_xyzw`; use `none`, `rot6d`, `axis_angle`, or `euler_xyz`). |

### Warnings

Expand Down
52 changes: 43 additions & 9 deletions src/inspect_robots/approver.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@
from __future__ import annotations

from dataclasses import replace
from typing import Any, Protocol, runtime_checkable
from typing import Any, Protocol, get_args, runtime_checkable

import numpy as np
import numpy.typing as npt

from inspect_robots.errors import SafetyAbort
from inspect_robots.spaces import Box
from inspect_robots.spaces import Box, RotationRepr
from inspect_robots.types import Action


Expand Down Expand Up @@ -80,9 +80,30 @@ 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 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"})
# Derived from RotationRepr itself (not hand-listed) so a rep added there
# defaults to the safe side of this message without anyone remembering to
# update it — the refusal set above is still the one source of truth for
# what actually gets rejected.
_DISPLACEMENT_SAFE_ROT = frozenset(get_args(RotationRepr)) - _DISPLACEMENT_POSE_UNSAFE_ROT
_LAST_APPROVED_KEY = "delta_limit:last"


Expand All @@ -103,8 +124,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 a pose mode whose rotation
representation cannot be clamped per-dimension all raise ``ValueError``.
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
Expand All @@ -119,10 +142,21 @@ 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"
)
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; only {sorted(_DISPLACEMENT_SAFE_ROT)} are safe here"
)
self._absolute = sem.control_mode in _ABSOLUTE_MODES
dim = action_space.dim
Expand Down
43 changes: 41 additions & 2 deletions tests/test_approvers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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),
Expand All @@ -55,6 +57,43 @@ def test_refuses_pose_mode_with_unaverageable_rotation() -> None:
DeltaLimitApprover(space)


@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.
# 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,),
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


@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))
Expand Down
20 changes: 20 additions & 0 deletions tests/test_conformance.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,),
Expand All @@ -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(
Expand Down
Loading