From 954efc48c39b928941c58dbe9526d80fe79ad655 Mon Sep 17 00:00:00 2001 From: Vedang Date: Wed, 22 Jul 2026 00:19:21 -0700 Subject: [PATCH] feat: add seconds-based task horizons --- CHANGELOG.md | 13 +++++ docs/guide/cli.md | 9 ++++ docs/guide/concepts.md | 8 +-- docs/guide/writing-a-benchmark.md | 25 +++++++++ plans/0026-seconds-based-horizons.md | 79 ++++++++++++++++++++++++++++ src/inspect_robots/CLAUDE.md | 4 +- src/inspect_robots/_html.py | 6 ++- src/inspect_robots/cli.py | 61 ++++++++++++++++++++- src/inspect_robots/compat.py | 36 +++++++++++++ src/inspect_robots/embodiment.py | 17 +++--- src/inspect_robots/eval.py | 22 ++++---- src/inspect_robots/log.py | 8 ++- src/inspect_robots/task.py | 62 ++++++++++++++++++++-- tests/test_compat.py | 44 ++++++++++++++++ tests/test_eval_log.py | 26 +++++++++ tests/test_eval_orchestration.py | 55 +++++++++++++++++-- tests/test_html_view.py | 21 ++++++++ tests/test_registry_cli.py | 46 ++++++++++++++++ tests/test_types_spaces.py | 76 ++++++++++++++++++++++++++ 19 files changed, 585 insertions(+), 33 deletions(-) create mode 100644 plans/0026-seconds-based-horizons.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 4deff07..3ac220e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,12 @@ All notable changes to this project are documented here. The format is based on ### Changed +- **Task horizon binding now follows compatibility checking.** An + embodiment's optional `bind_task()` hook receives the resolved step envelope + only after the policy/embodiment/task triple is known to be compatible. This + prevents adapters from acting on a seconds-derived budget built from an + invalid control rate (#160). + - **Docs site migrated from MkDocs Material to Docusaurus.** The site at inspectrobots.org now builds from `website/` (Docusaurus 3) while the Markdown source stays in `docs/`; every existing URL, `llms.txt`, and @@ -39,6 +45,13 @@ All notable changes to this project are documented here. The format is based on ### Added +- **Seconds-based benchmark horizons:** `Task(max_seconds=...)` gives every + compatible embodiment the same physical-time budget. `eval()` resolves it + with `ceil(max_seconds * embodiment.info.control_hz)`, rejects missing or + invalid control rates before `bind_task()` or rollout, and records both the + declared seconds and resolved steps in eval logs, CLI summaries, inspection, + and HTML reports (#160). + - **Policy lifecycle hook: `on_trial_end`** — policies can now hook into the end of a trial to persist state or artifacts. The orchestrator calls `policy.on_trial_end(record, log_dir, run_id)` and any metadata the policy diff --git a/docs/guide/cli.md b/docs/guide/cli.md index 0184a66..272f13e 100644 --- a/docs/guide/cli.md +++ b/docs/guide/cli.md @@ -210,6 +210,10 @@ inspect-robots eval-set 'kitchenbench/*' --policy xpolicylab -P url=ws://host:19 inspect-robots eval-set cubepick-reach my-other-task --policy scripted --embodiment cubepick ``` +For a task declared with `max_seconds`, its summary row includes both the +physical-time budget and the integer step limit resolved from the selected +embodiment, for example `[120s -> 1200 steps at 10 Hz]`. + Multiple patterns may match the same task; it still runs once. A pattern that matches nothing is an error listing every registered task. `--policy` and `--embodiment` (and `-P`/`-E`, `--sim`, `--epochs`, `--fail-on-error`, @@ -253,6 +257,7 @@ policy: scripted embodiment: cubepick run status: completed outcome: 5 succeeded +horizon: 120s -> 1200 steps at 10 Hz scenes: 5 trials: 5 metrics: success_at_end: 1 @@ -261,6 +266,10 @@ scenes: ... ``` +The `horizon` line appears for seconds-based tasks; step-only logs retain the +existing output. The HTML viewer likewise separates declared seconds from the +resolved step limit. + `completed` is the display form of the log's `success` status value; the on-disk field and Python API keep `success`. diff --git a/docs/guide/concepts.md b/docs/guide/concepts.md index 941c719..3e6ee89 100644 --- a/docs/guide/concepts.md +++ b/docs/guide/concepts.md @@ -22,8 +22,9 @@ without inheriting anything. Convenience base classes (`PolicyBase`, ## Tasks and scenes A [`Task`](/api/#inspect_robots.task.Task) is an embodiment-agnostic benchmark: a dataset -of [`Scene`](/api/#inspect_robots.scene.Scene)s plus scorer(s), a step horizon, and an epoch -count. A `Scene` is the robotics analog of Inspect AI's `Sample`, one initial +of [`Scene`](/api/#inspect_robots.scene.Scene)s plus scorer(s), a step- or seconds-based +horizon, and an epoch count. A seconds horizon is resolved to integer steps from +the paired embodiment's declared control rate. A `Scene` is the robotics analog of Inspect AI's `Sample`, one initial condition: an instruction, an optional success [`Target`](/api/#inspect_robots.scene.Target), and a seed. @@ -33,7 +34,8 @@ Before any rollout, [`check_compatibility`](/api/#inspect_robots.compat.check_co `(policy, embodiment)` pair: action dimensions and [`ActionSemantics`](/api/#inspect_robots.spaces.ActionSemantics) (control mode, rotation representation, gripper, frame), the observation cameras/state keys the policy requires (resolving a name remap), the control rate, -and whether each scene is realizable on the embodiment. Hard mismatches fail fast +whether a seconds-based task has a usable control rate, and whether each scene +is realizable on the embodiment. Hard mismatches fail fast with a [`CompatibilityError`](/api/#inspect_robots.errors.CompatibilityError). ## The rollout diff --git a/docs/guide/writing-a-benchmark.md b/docs/guide/writing-a-benchmark.md index dba59f7..623dea8 100644 --- a/docs/guide/writing-a-benchmark.md +++ b/docs/guide/writing-a-benchmark.md @@ -63,6 +63,31 @@ task = Task( ) ``` +## Horizons + +A task declares exactly one rollout horizon. Use `max_steps` when the protocol +is inherently discrete, as in the examples above. Use `max_seconds` when every +embodiment should receive the same physical-time budget: + +```python +task = Task( + name="two-minute-reach", + scenes=[...], + scorer=success_at_end(), + max_seconds=120.0, +) +``` + +At evaluation time, Inspect Robots resolves the budget as +`ceil(max_seconds * embodiment.info.control_hz)`. A 120-second task therefore +runs for 1,200 steps at 10 Hz and 1,800 steps at 15 Hz. The eval log records +both the declared seconds and the resolved integer step limit. + +A seconds-based task is incompatible with an embodiment whose `control_hz` is +missing, non-finite, zero, or negative. Event-driven embodiments should use +`max_steps`. Resolution changes the step budget only: `rollout()` does not add +wall-clock pacing, so real-time cadence remains the embodiment's responsibility. + ## Registering for discovery Wrap a task factory with [`task`](/api/#inspect_robots.registry.task) so it resolves by name in diff --git a/plans/0026-seconds-based-horizons.md b/plans/0026-seconds-based-horizons.md new file mode 100644 index 0000000..0f96dc6 --- /dev/null +++ b/plans/0026-seconds-based-horizons.md @@ -0,0 +1,79 @@ +# 0026: Seconds-based task horizons + +Issue: [#160](https://github.com/robocurve/inspect-robots/issues/160) +Status: proposed (implementation follows the issue sketch and resolves its open questions) + +## 1. Problem + +`Task.max_steps` fixes the rollout budget before a task is paired with an +embodiment, but a step has a physical duration only after the embodiment's +`control_hz` is known. The same 600-step task therefore receives 60 seconds on +a 10 Hz embodiment and 40 seconds on a 15 Hz embodiment. Benchmarks that define +physical-time protocols cannot currently express their horizon without +hard-coding one embodiment's rate. + +## 2. Design + +`Task` accepts exactly one horizon: + +- `max_steps: int | None`, preserving the existing step-based contract; or +- keyword-only `max_seconds: float | None`, a finite positive physical-time + budget resolved by `eval()`. + +After policy binding, `eval()` runs compatibility checking. A seconds-based +task is incompatible with an embodiment whose `control_hz` is missing, +non-finite, or non-positive. Once compatibility passes, the task resolves a +`TaskEnvelope` with: + +```python +max_steps = ceil(max_seconds * embodiment.info.control_hz) +``` + +The resolved envelope is passed to `bind_task()` and the rollout. This changes +the hook order from bind-then-compatibility to compatibility-then-bind. The +ordering is required because an adapter must never receive a seconds-derived +step budget based on an invalid rate. + +`EvalSpec` records both values: + +- `max_seconds`: the declared physical-time budget, or `None` for a + step-based task; +- `max_steps`: the resolved integer budget used by the rollout. + +The schema version remains 1 because `max_seconds` is additive and defaults to +`None` when newer code reads an older log. + +## 3. Open-question resolutions + +- **Missing or zero rate:** reject at compatibility-check time. Event-driven + embodiments must use `max_steps` because physical seconds cannot be resolved + from their declared contract. +- **Scorers:** stay steps-only in this change. They already receive the recorded + trajectory, and adding task configuration to the scoring interface is a + separate API decision. +- **`eval-set`:** show the declared seconds and resolved steps on each task row. + Saved-log inspection and the HTML report show the same pair. + +## 4. Non-goals + +- No wall-clock pacing in `rollout()`; self-paced embodiments still own their + cadence. +- No CLI `--max-seconds` override for ad-hoc runs. This change lets benchmark + authors declare physical-time protocols; the existing ad-hoc `--max-steps` + control is unchanged. +- No scorer API or metric changes. +- No dependency changes. + +## 5. Validation + +- Task construction rejects missing, duplicate, non-positive, and non-finite + horizon declarations. +- Compatibility reports invalid embodiment rates before `bind_task()` or + rollout. +- Resolution uses `ceil`, including non-integral products, and the exact + resolved value reaches `TaskEnvelope`, `rollout()`, and `EvalLog`. +- Older schema-v1 logs without `max_seconds` still read with `None`. +- CLI run/inspect/eval-set output and HTML reports surface the declared and + resolved horizons without changing old step-only output. +- Ruff, formatting, strict mypy, 100% line and branch coverage, and the strict + Docusaurus build remain green. diff --git a/src/inspect_robots/CLAUDE.md b/src/inspect_robots/CLAUDE.md index 57effa9..b838fce 100644 --- a/src/inspect_robots/CLAUDE.md +++ b/src/inspect_robots/CLAUDE.md @@ -10,9 +10,9 @@ interfaces. The package is `mypy --strict` clean and ships `py.typed`. | `types.py` | `Observation`, `Action`, `ActionChunk`, `StepResult` (frozen, NumPy-native) | | `spaces.py` | `Box`, `ObservationSpace`, `ActionSemantics`, `StateSpec` + canonical state vocab | | `policy.py` | `Policy` Protocol + `PolicyBase` ABC, `PolicyInfo`, `PolicyConfig`; optional duck-typed `bind(embodiment_info)` hook for embodiment-adaptive policies plus `transcript()` and `transcript_delta()` hooks for complete and live per-trial audit records | -| `embodiment.py` | `Embodiment` Protocol + `EmbodimentBase` ABC, `EmbodimentInfo`, capability flags; optional duck-typed `bind_task(envelope)` hook for horizon-aware adapters (called by `eval()` before compat; optional input — never fires on direct `rollout()`, keep a fallback) | +| `embodiment.py` | `Embodiment` Protocol + `EmbodimentBase` ABC, `EmbodimentInfo`, capability flags; optional duck-typed `bind_task(envelope)` hook for horizon-aware adapters (called by `eval()` after compat with a resolved step envelope; optional input — never fires on direct `rollout()`, keep a fallback) | | `scene.py` | `Scene` (the Inspect `Sample` analog), `Target`, `ListSceneDataset` | -| `task.py` | `Task` (scenes + scorer + horizon), `Epochs`, `TaskEnvelope` (`Task.envelope` — the adapter-safe identity+limits view passed to `bind_task` hooks) | +| `task.py` | `Task` (scenes + scorer + exactly one `max_steps`/`max_seconds` horizon), `Epochs`, `TaskEnvelope` (`resolve_envelope(control_hz)` — the adapter-safe identity+resolved-step limits view passed to `bind_task` hooks) | | `scorer.py` | `Score`/`Scorer`, epoch reducers, builtin scorers (incl. operator/VLM) | | `controller.py` | `Controller` middleware: `DefaultController` (open-loop chunking), `SmoothingController` | | `approver.py` | `Approver` safety gate: `AutoApprover`, `ClampApprover`, `DeltaLimitApprover` (semantics-aware no-wild-swings limit), `ChainApprover` | diff --git a/src/inspect_robots/_html.py b/src/inspect_robots/_html.py index 2ff09e2..ad656f0 100644 --- a/src/inspect_robots/_html.py +++ b/src/inspect_robots/_html.py @@ -556,7 +556,11 @@ def render_html( ) if log.eval.seed is not None: definitions.append(_definition("seed", log.eval.seed)) - if log.eval.max_steps is not None: + if log.eval.max_seconds is not None: + definitions.append(_definition("max seconds", log.eval.max_seconds)) + if log.eval.max_steps is not None: + definitions.append(_definition("resolved max steps", log.eval.max_steps)) + elif log.eval.max_steps is not None: definitions.append(_definition("max steps", log.eval.max_steps)) if log.stats.mean_inference_latency_s is not None: definitions.append( diff --git a/src/inspect_robots/cli.py b/src/inspect_robots/cli.py index 44a6306..7d33b96 100644 --- a/src/inspect_robots/cli.py +++ b/src/inspect_robots/cli.py @@ -565,6 +565,43 @@ def _step_limit_count(log: EvalLog) -> int: ) +def _seconds_horizon(log: EvalLog) -> tuple[float, int, float | None] | None: + """Return a validated declared/resolved seconds horizon from a saved log.""" + max_seconds = log.eval.max_seconds + max_steps = log.eval.max_steps + if not ( + isinstance(max_seconds, (int, float)) + and not isinstance(max_seconds, bool) + and math.isfinite(max_seconds) + and max_seconds > 0 + and isinstance(max_steps, int) + and not isinstance(max_steps, bool) + and max_steps > 0 + ): + return None + + rate = log.eval.embodiment_info.get("control_hz") + valid_rate = ( + float(rate) + if isinstance(rate, (int, float)) + and not isinstance(rate, bool) + and math.isfinite(rate) + and rate > 0 + else None + ) + return float(max_seconds), max_steps, valid_rate + + +def _seconds_horizon_text(log: EvalLog) -> str | None: + """Format a seconds-based horizon for compact CLI metadata surfaces.""" + horizon = _seconds_horizon(log) + if horizon is None: + return None + max_seconds, max_steps, rate = horizon + text = f"{max_seconds:g}s -> {max_steps} steps" + return f"{text} at {rate:g} Hz" if rate is not None else text + + def _outcome_line(log: EvalLog) -> tuple[str, bool] | None: """Return an outcome digest and unmapped flag, or ``None`` with no reasons.""" reasons: list[object] = [ @@ -599,17 +636,31 @@ def _print_step_limit_notice(log: EvalLog, is_adhoc: bool) -> None: note = f"note: {count}/{log.results.total_trials} trials hit the step limit before terminating" max_steps = log.eval.max_steps + seconds_horizon = _seconds_horizon(log) + if seconds_horizon is not None: + max_seconds, resolved_steps, rate = seconds_horizon + parenthetical = f"max_seconds={max_seconds:g}, resolved max_steps={resolved_steps}" + if rate is not None: + parenthetical += f" at {rate:g} Hz" + note += f" ({parenthetical})" # Guards below reject bool/str values a hand-edited log can smuggle past # from_dict (bool is an int subclass, so isinstance alone lets True in). - if isinstance(max_steps, int) and not isinstance(max_steps, bool): + elif isinstance(max_steps, int) and not isinstance(max_steps, bool): parenthetical = f"max_steps={max_steps}" rate = log.eval.embodiment_info.get("control_hz") - if isinstance(rate, (int, float)) and not isinstance(rate, bool) and rate > 0: + if ( + isinstance(rate, (int, float)) + and not isinstance(rate, bool) + and math.isfinite(rate) + and rate > 0 + ): parenthetical += f", ~{max_steps / rate:g}s at {rate:g} Hz" note += f" ({parenthetical})" print(_styled(note, _YELLOW)) if is_adhoc: hint = "hint: raise it with --max-steps N or: inspect-robots config set max_steps N" + elif seconds_horizon is not None: + hint = f"hint: task {log.eval.task!r} defines its own max_seconds" else: hint = f"hint: task {log.eval.task!r} defines its own max_steps" print(_styled(hint, _DIM)) @@ -984,6 +1035,9 @@ def _print_eval_set_summary(success: bool, logs: Sequence[EvalLog], log_dir: str ) detail = metrics or (log.error or "") row = f" [{_styled(_display_status(log.status), _GREEN if ok else _RED)}] {log.eval.task}" + horizon = _seconds_horizon_text(log) + if horizon is not None: + row += f" [{horizon}]" print(f"{row} {detail}" if detail else row) print(f"{_styled('log dir:', _CYAN)} {_styled(log_dir, _DIM)}") if not success: @@ -1105,6 +1159,9 @@ def _cmd_inspect(path: str, *, transcript: bool = False) -> int: print(line) print(f"created: {log.eval.created}") print(f"git: {log.eval.git_commit}") + horizon = _seconds_horizon_text(log) + if horizon is not None: + print(f"horizon: {horizon}") trials = f"trials: {log.results.total_trials}" if log.results.errored_trials: trials += f" ({log.results.errored_trials} errored)" diff --git a/src/inspect_robots/compat.py b/src/inspect_robots/compat.py index 52996d5..dc7672d 100644 --- a/src/inspect_robots/compat.py +++ b/src/inspect_robots/compat.py @@ -11,6 +11,7 @@ from __future__ import annotations +import math from dataclasses import dataclass, field from inspect_robots.embodiment import Embodiment @@ -169,11 +170,46 @@ def check_compatibility( ) if task is not None: + _check_task_horizon(task, embodiment, issues) _check_scenes_realizable(task, embodiment, issues) return report +def _check_task_horizon(task: Task, embodiment: Embodiment, issues: list[CompatIssue]) -> None: + """Require a usable embodiment rate when a task declares physical seconds.""" + if task.max_seconds is None: + return + + control_hz = embodiment.info.control_hz + if ( + control_hz is None + or isinstance(control_hz, bool) + or not math.isfinite(control_hz) + or control_hz <= 0 + ): + issues.append( + CompatIssue( + "error", + "task_horizon_control_rate", + f"task {task.name!r} declares max_seconds={task.max_seconds:g}, but " + f"embodiment {embodiment.info.name!r} has control_hz={control_hz!r}; " + "seconds-based tasks require a finite positive embodiment rate", + ) + ) + return + + if not math.isfinite(task.max_seconds * control_hz): + issues.append( + CompatIssue( + "error", + "task_horizon_control_rate", + f"task {task.name!r} max_seconds={task.max_seconds:g} at embodiment " + f"control_hz={control_hz:g} does not yield a finite step budget", + ) + ) + + def _check_scenes_realizable(task: Task, embodiment: Embodiment, issues: list[CompatIssue]) -> None: sup_setups = embodiment.info.supported_setups sup_targets = embodiment.info.supported_target_kinds diff --git a/src/inspect_robots/embodiment.py b/src/inspect_robots/embodiment.py index 99be055..e4bfc41 100644 --- a/src/inspect_robots/embodiment.py +++ b/src/inspect_robots/embodiment.py @@ -70,15 +70,16 @@ class Embodiment(Protocol): Embodiments may additionally define an **optional** ``bind_task(envelope)`` hook (not part of this Protocol, so existing embodiments stay conformant): - ``eval()`` calls it with the task's + after compatibility checking, ``eval()`` calls it with the task's resolved [`TaskEnvelope`][inspect_robots.task.TaskEnvelope] before the - compatibility check, letting adapters learn the rollout horizon — e.g. an - operator countdown showing elapsed/total. The hook is optional *input*, - not a guarantee: it never fires on direct ``rollout()`` calls or on older - cores, so adapters must keep a graceful fallback. It fires once per - ``eval()``, which can be several times over an embodiment's lifetime; - each call replaces the previous envelope. ``EmbodimentBase`` ships a - no-op default. + first reset, letting adapters learn the rollout horizon — e.g. an operator + countdown showing elapsed/total. A seconds-based task is converted to + integer steps using ``info.control_hz`` before the hook fires. The hook is + optional *input*, not a guarantee: it never fires on direct ``rollout()`` + calls or on older cores, so adapters must keep a graceful fallback. It + fires once per ``eval()``, which can be several times over an embodiment's + lifetime; each call replaces the previous envelope. ``EmbodimentBase`` + ships a no-op default. """ info: EmbodimentInfo diff --git a/src/inspect_robots/eval.py b/src/inspect_robots/eval.py index 9228884..076a190 100644 --- a/src/inspect_robots/eval.py +++ b/src/inspect_robots/eval.py @@ -239,16 +239,19 @@ def _run_eval( if callable(bind): bind(embodiment.info) + # Fail fast on incompatible pairings before touching any hardware/sim. + # This also validates the embodiment rate needed by a seconds-based task + # before the resolved horizon is exposed to an adapter (plan 0026). + assert_compatible(policy, embodiment, task, remap=remap) + task_envelope = task.resolve_envelope(embodiment.info.control_hz) + # Horizon-aware embodiments (plan 0013): an optional bind_task() hook runs - # here too, so the adapter can learn the rollout envelope (e.g. for an - # operator countdown) before any hardware is touched. Duck-typed — - # bind_task is not part of the Embodiment Protocol. + # after compatibility so the adapter receives the resolved rollout + # envelope (e.g. for an operator countdown) before any hardware is + # touched. Duck-typed — bind_task is not part of the Embodiment Protocol. bind_task = getattr(embodiment, "bind_task", None) if callable(bind_task): - bind_task(task.envelope) - - # Fail fast on incompatible pairings before touching any hardware/sim. - assert_compatible(policy, embodiment, task, remap=remap) + bind_task(task_envelope) epoch_spec = task.epoch_spec scorers = task.scorers @@ -291,7 +294,8 @@ def _run_eval( "capabilities": sorted(embodiment.info.capabilities), }, seed=seed, - max_steps=task.max_steps, + max_steps=task_envelope.max_steps, + max_seconds=task.max_seconds, ) bus.on_eval_start(spec) @@ -329,7 +333,7 @@ def _run_eval( policy, embodiment, scene, - max_steps=task.max_steps, + max_steps=task_envelope.max_steps, seed=trial_seed, epoch=epoch, controller=controller, diff --git a/src/inspect_robots/log.py b/src/inspect_robots/log.py index 6bbf5e4..faa2209 100644 --- a/src/inspect_robots/log.py +++ b/src/inspect_robots/log.py @@ -26,7 +26,12 @@ @dataclass(frozen=True) class EvalSpec: - """Top-level identity and configured horizon of a reproducible eval.""" + """Top-level identity and configured horizon of a reproducible eval. + + ``max_steps`` is always the resolved integer budget used by the rollout. + ``max_seconds`` preserves a benchmark's declared physical-time budget when + that integer was derived from the embodiment's control rate. + """ task: str policy: str @@ -38,6 +43,7 @@ class EvalSpec: embodiment_info: dict[str, Any] = field(default_factory=dict) seed: int | None = None max_steps: int | None = None + max_seconds: float | None = None @dataclass(frozen=True) diff --git a/src/inspect_robots/task.py b/src/inspect_robots/task.py index dddf70c..9d508aa 100644 --- a/src/inspect_robots/task.py +++ b/src/inspect_robots/task.py @@ -7,6 +7,7 @@ from __future__ import annotations +import math from collections.abc import Mapping, Sequence from dataclasses import dataclass, field from typing import Any, cast @@ -55,18 +56,35 @@ class Task: ``scorer`` accepts scorer objects or **registry names** (e.g. ``scorer="success_at_end"``), or a sequence mixing both. + + Declare exactly one rollout horizon. ``max_steps`` is already resolved; + ``max_seconds`` is keyword-only and is resolved against the paired + embodiment's positive finite ``control_hz`` by ``eval()``. """ name: str scenes: Sequence[Scene] scorer: Scorer | str | Sequence[Scorer | str] - max_steps: int + max_steps: int | None = None epochs: int | Epochs = 1 metadata: Mapping[str, Any] = field(default_factory=dict) + max_seconds: float | None = field(default=None, kw_only=True) def __post_init__(self) -> None: - if self.max_steps < 1: + if (self.max_steps is None) == (self.max_seconds is None): + raise ConfigError( + f"Task {self.name!r}: declare exactly one of max_steps or max_seconds" + ) + if self.max_steps is not None and self.max_steps < 1: raise ConfigError(f"Task {self.name!r}: max_steps must be >= 1, got {self.max_steps}") + if self.max_seconds is not None and ( + isinstance(self.max_seconds, bool) + or not math.isfinite(self.max_seconds) + or self.max_seconds <= 0 + ): + raise ConfigError( + f"Task {self.name!r}: max_seconds must be finite and > 0, got {self.max_seconds!r}" + ) _ = self.epoch_spec # validates an int epochs count via Epochs @property @@ -95,5 +113,41 @@ def epoch_spec(self) -> Epochs: @property def envelope(self) -> TaskEnvelope: - """The adapter-safe view of this task handed to ``bind_task`` hooks.""" - return TaskEnvelope(name=self.name, max_steps=self.max_steps) + """Return the already-resolved adapter view of a step-based task. + + Seconds-based tasks require an embodiment rate and must use + [`resolve_envelope`][inspect_robots.task.Task.resolve_envelope]. + """ + return self.resolve_envelope(None) + + def resolve_envelope(self, control_hz: float | None) -> TaskEnvelope: + """Resolve the adapter-safe task view against an embodiment rate. + + Compatibility checking normally rejects an invalid rate before this + method is called. The validation here is a defensive public-API guard + for callers resolving envelopes directly. + """ + if self.max_steps is not None: + return TaskEnvelope(name=self.name, max_steps=self.max_steps) + + if ( + control_hz is None + or isinstance(control_hz, bool) + or not math.isfinite(control_hz) + or control_hz <= 0 + ): + raise ConfigError( + f"Task {self.name!r}: max_seconds requires an embodiment control_hz " + f"that is finite and > 0, got {control_hz!r}" + ) + + assert self.max_seconds is not None # exactly-one invariant from __post_init__ + raw_steps = self.max_seconds * control_hz + if not math.isfinite(raw_steps): + raise ConfigError( + f"Task {self.name!r}: max_seconds={self.max_seconds!r} at " + f"control_hz={control_hz!r} does not yield a finite step budget" + ) + # Both factors are positive, so the mathematical ceiling is at least + # one even if their binary-float product underflows to 0.0. + return TaskEnvelope(name=self.name, max_steps=max(1, math.ceil(raw_steps))) diff --git a/tests/test_compat.py b/tests/test_compat.py index 9dcadc3..920a52b 100644 --- a/tests/test_compat.py +++ b/tests/test_compat.py @@ -2,6 +2,8 @@ from __future__ import annotations +from dataclasses import replace + import numpy as np import pytest @@ -113,6 +115,48 @@ def test_scene_target_realizability() -> None: assert any(i.code == "scene_target" for i in report.errors) +def test_seconds_task_with_valid_control_rate_is_compatible() -> None: + task = Task( + name="timed", + scenes=[Scene(id="s", instruction="x")], + scorer=success_at_end(), + max_seconds=120.0, + ) + report = check_compatibility(ScriptedPolicy(), CubePickEmbodiment(), task) + assert not any(i.code == "task_horizon_control_rate" for i in report.errors) + + +@pytest.mark.parametrize("control_hz", [None, True, 0.0, -1.0, float("nan"), float("inf")]) +def test_seconds_task_requires_positive_finite_control_rate( + control_hz: float | None, +) -> None: + embodiment = CubePickEmbodiment() + embodiment.info = replace(embodiment.info, control_hz=control_hz) + task = Task( + name="timed", + scenes=[Scene(id="s", instruction="x")], + scorer=success_at_end(), + max_seconds=120.0, + ) + report = check_compatibility(ScriptedPolicy(), embodiment, task) + issue = next(i for i in report.errors if i.code == "task_horizon_control_rate") + assert "finite positive embodiment rate" in issue.message + + +def test_seconds_task_rejects_nonfinite_resolved_step_budget() -> None: + embodiment = CubePickEmbodiment() + embodiment.info = replace(embodiment.info, control_hz=1e308) + task = Task( + name="timed", + scenes=[Scene(id="s", instruction="x")], + scorer=success_at_end(), + max_seconds=1e308, + ) + report = check_compatibility(ScriptedPolicy(), embodiment, task) + issue = next(i for i in report.errors if i.code == "task_horizon_control_rate") + assert "finite step budget" in issue.message + + def test_assert_compatible_raises() -> None: policy = _StubPolicy( PolicyInfo( diff --git a/tests/test_eval_log.py b/tests/test_eval_log.py index f5d6b3e..2a8b287 100644 --- a/tests/test_eval_log.py +++ b/tests/test_eval_log.py @@ -105,6 +105,7 @@ def test_v1_log_without_additive_fields_reads_back(tmp_path: Path) -> None: # Older schema-v1 logs missing additive fields must remain readable. data = _golden_log().to_dict() del data["eval"]["max_steps"] + del data["eval"]["max_seconds"] for sample in data["samples"]: del sample["instruction"] del sample["operator_judgements"] @@ -121,6 +122,31 @@ def test_v1_log_without_additive_fields_reads_back(tmp_path: Path) -> None: assert restored.samples[0].termination_reasons == () assert restored.samples[0].policy_transcripts == () assert restored.eval.max_steps is None + assert restored.eval.max_seconds is None + + +def test_seconds_horizon_round_trips_declared_and_resolved_values() -> None: + spec = EvalSpec( + task="timed", + policy="scripted", + embodiment="cubepick", + created="2026-07-21T00:00:00+00:00", + inspect_robots_version="0.0.0", + embodiment_info={"control_hz": 15.0}, + max_steps=1800, + max_seconds=120.0, + ) + restored = EvalLog.from_dict( + EvalLog( + version=SCHEMA_VERSION, + status="success", + eval=spec, + results=EvalResults(total_scenes=0, total_trials=0), + stats=EvalStats(started_at="a", completed_at="b", duration_s=0.0, total_steps=0), + ).to_dict() + ) + assert restored.eval.max_seconds == 120.0 + assert restored.eval.max_steps == 1800 def test_eval_log_and_friends_are_frozen() -> None: diff --git a/tests/test_eval_orchestration.py b/tests/test_eval_orchestration.py index 69a5230..e70710b 100644 --- a/tests/test_eval_orchestration.py +++ b/tests/test_eval_orchestration.py @@ -10,6 +10,7 @@ from inspect_robots import eval, eval_set, read_eval_log from inspect_robots.errors import ( + CompatibilityError, ConfigError, EmbodimentFault, PolicyError, @@ -538,6 +539,30 @@ def reset(self, scene: Scene, *, seed: int | None = None) -> Observation: assert [c for c in aware.calls if c != "reset"] == [TaskEnvelope(name="t", max_steps=7)] +def test_eval_resolves_seconds_horizon_into_envelope_rollout_and_log(tmp_path: Path) -> None: + class _HorizonAware(CubePickEmbodiment): + def __init__(self) -> None: + super().__init__() + self.envelopes: list[TaskEnvelope] = [] + + def bind_task(self, envelope: TaskEnvelope) -> None: + self.envelopes.append(envelope) + + task = Task( + name="timed", + scenes=[Scene(id="s", instruction="reach")], + scorer=success_at_end(), + max_seconds=0.1, + ) + aware = _HorizonAware() + (log,) = eval(task, ScriptedPolicy(), aware, log_dir=str(tmp_path)) + + assert aware.envelopes == [TaskEnvelope(name="timed", max_steps=1)] + assert log.samples[0].termination_reasons == ("max_steps",) + assert log.eval.max_seconds == 0.1 + assert log.eval.max_steps == 1 + + def test_bind_task_rebinds_per_eval_latest_wins(tmp_path: Path) -> None: class _HorizonAware(CubePickEmbodiment): def __init__(self) -> None: @@ -578,8 +603,8 @@ def test_raising_bind_task_aborts_before_any_rollout(tmp_path: Path) -> None: assert _CLOSED == ["closed"] # registry-owned embodiment still released -def test_bind_task_runs_before_the_compatibility_check(tmp_path: Path) -> None: - """The hook fires pre-compat: a raising bind_task wins over an incompatible pairing.""" +def test_compatibility_check_runs_before_bind_task(tmp_path: Path) -> None: + """An incompatible pair fails before the embodiment receives an envelope.""" class _WidePolicy(ScriptedPolicy): def __init__(self) -> None: @@ -589,10 +614,34 @@ def __init__(self) -> None: action_space=Box(shape=(9,), semantics=ActionSemantics("joint_pos")), ) - with pytest.raises(RuntimeError, match="refusing this task"): + with pytest.raises(CompatibilityError, match="action_dim"): eval(_task(max_steps=5), _WidePolicy(), "bind-raises-cubepick", log_dir=str(tmp_path)) +def test_invalid_seconds_rate_fails_before_bind_task(tmp_path: Path) -> None: + from dataclasses import replace + + class _RateMissing(CubePickEmbodiment): + def __init__(self) -> None: + super().__init__() + self.info = replace(self.info, control_hz=None) + self.envelopes: list[TaskEnvelope] = [] + + def bind_task(self, envelope: TaskEnvelope) -> None: + self.envelopes.append(envelope) + + task = Task( + name="timed", + scenes=[Scene(id="s", instruction="reach")], + scorer=success_at_end(), + max_seconds=120.0, + ) + embodiment = _RateMissing() + with pytest.raises(CompatibilityError, match="task_horizon_control_rate"): + eval(task, ScriptedPolicy(), embodiment, log_dir=str(tmp_path)) + assert embodiment.envelopes == [] + + def test_embodiment_base_bind_task_is_a_noop() -> None: from inspect_robots.embodiment import EmbodimentBase diff --git a/tests/test_html_view.py b/tests/test_html_view.py index cf99032..a43b0aa 100644 --- a/tests/test_html_view.py +++ b/tests/test_html_view.py @@ -179,6 +179,27 @@ def test_absent_optional_fields_and_empty_scene_sequences_are_omitted() -> None: assert "Operator judgements" not in document +def test_seconds_horizon_shows_declared_and_resolved_limits() -> None: + log = _log() + spec = dataclasses.replace(log.eval, max_seconds=12.5, max_steps=188) + document = render_html(dataclasses.replace(log, eval=spec), title="timed") + + assert "
max seconds
" in document + assert "
12.5
" in document + assert "
resolved max steps
" in document + assert "
188
" in document + assert "
max steps
" not in document + + +def test_seconds_horizon_without_resolved_steps_omits_step_limit() -> None: + log = _log() + spec = dataclasses.replace(log.eval, max_seconds=12.5, max_steps=None) + document = render_html(dataclasses.replace(log, eval=spec), title="incomplete") + + assert "
max seconds
" in document + assert "
resolved max steps
" not in document + + def test_every_foreign_text_surface_is_escaped_exactly_once() -> None: attack = "" transcript = _chat( diff --git a/tests/test_registry_cli.py b/tests/test_registry_cli.py index 99666e7..2751d70 100644 --- a/tests/test_registry_cli.py +++ b/tests/test_registry_cli.py @@ -794,6 +794,15 @@ def test_cli_eval_set_zero_scene_task_has_no_metric_or_error_detail( assert "[completed] kb/a\n" in out # no trailing metric/error detail +def test_eval_set_summary_surfaces_seconds_horizon( + capsys: pytest.CaptureFixture[str], +) -> None: + log = _step_limit_log(task="timed", max_seconds=120.0, control_hz=10.0) + cli._print_eval_set_summary(True, [log], "logs") + out = capsys.readouterr().out + assert "[completed] timed [120s -> 1200 steps at 10 Hz]" in out + + def test_cli_eval_set_ctrl_c_reports_partial_logs_and_exits_130( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] ) -> None: @@ -888,6 +897,7 @@ def _step_limit_log( task: str = "adhoc", reasons: tuple[str | None, ...] = ("max_steps",), max_steps: int | None = 1200, + max_seconds: float | None = None, control_hz: object = 10.0, ) -> EvalLog: return EvalLog( @@ -901,6 +911,7 @@ def _step_limit_log( inspect_robots_version="0", embodiment_info={"control_hz": control_hz}, max_steps=max_steps, + max_seconds=max_seconds, ), results=EvalResults( total_scenes=1, @@ -1725,6 +1736,35 @@ def test_run_summary_uses_registered_task_hint(capsys: pytest.CaptureFixture[str assert "--max-steps N" not in out +def test_run_summary_surfaces_seconds_horizon_for_registered_task( + capsys: pytest.CaptureFixture[str], +) -> None: + log = _step_limit_log(task="timed-task", max_seconds=120.0, control_hz=10.0) + cli._print_run_summary(log, "run.json", is_adhoc=False) + out = capsys.readouterr().out + assert "(max_seconds=120, resolved max_steps=1200 at 10 Hz)" in out + assert "hint: task 'timed-task' defines its own max_seconds" in out + + +def test_run_summary_surfaces_seconds_horizon_without_logged_control_rate( + capsys: pytest.CaptureFixture[str], +) -> None: + log = _step_limit_log(task="timed-task", max_seconds=120.0, control_hz=None) + cli._print_run_summary(log, "run.json", is_adhoc=False) + out = capsys.readouterr().out + assert "(max_seconds=120, resolved max_steps=1200)" in out + assert "resolved max_steps=1200 at" not in out + + +def test_seconds_horizon_text_tolerates_missing_rate_and_malformed_limits() -> None: + assert ( + cli._seconds_horizon_text(_step_limit_log(max_seconds=120.0, control_hz=None)) + == "120s -> 1200 steps" + ) + assert cli._seconds_horizon_text(_step_limit_log(max_seconds=0.0)) is None + assert cli._seconds_horizon_text(_step_limit_log(max_seconds=120.0, max_steps=None)) is None + + def test_run_summary_omits_step_limit_note_without_truncation( capsys: pytest.CaptureFixture[str], ) -> None: @@ -1777,6 +1817,12 @@ def test_inspect_tolerates_non_numeric_max_steps_in_hand_edited_log( assert "max_steps=1200" not in out +def test_inspect_prints_seconds_horizon(tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None: + path = _write_log(_step_limit_log(max_seconds=120.0, control_hz=10.0), tmp_path, "timed.json") + assert main(["inspect", str(path)]) == 0 + assert "horizon: 120s -> 1200 steps at 10 Hz" in capsys.readouterr().out + + def test_bare_instruction_runs_adhoc_task_from_env_defaults( monkeypatch: pytest.MonkeyPatch, tmp_path: Path, capsys: pytest.CaptureFixture[str] ) -> None: diff --git a/tests/test_types_spaces.py b/tests/test_types_spaces.py index ee0f916..ac30e1d 100644 --- a/tests/test_types_spaces.py +++ b/tests/test_types_spaces.py @@ -127,15 +127,81 @@ def test_observation_space_rejects_inconsistent_state_keys() -> None: def test_task_envelope_is_a_frozen_view_of_the_horizon() -> None: + from inspect_robots.errors import ConfigError from inspect_robots.scene import Scene from inspect_robots.task import Task, TaskEnvelope scene = Scene(id="s", instruction="x") task = Task(name="t", scenes=[scene], scorer="success_at_end", max_steps=80) assert task.envelope == TaskEnvelope(name="t", max_steps=80) + assert task.resolve_envelope(None) == task.envelope with pytest.raises(AttributeError): task.envelope.max_steps = 81 # type: ignore[misc] + seconds_task = Task(name="timed", scenes=[scene], scorer="success_at_end", max_seconds=1.01) + assert seconds_task.resolve_envelope(10.0) == TaskEnvelope(name="timed", max_steps=11) + with pytest.raises(ConfigError, match="requires an embodiment control_hz"): + _ = seconds_task.envelope + + +@pytest.mark.parametrize("max_seconds", [True, 0.0, -1.0, float("nan"), float("inf")]) +def test_task_rejects_invalid_seconds_horizon(max_seconds: float) -> None: + from inspect_robots.errors import ConfigError + from inspect_robots.scene import Scene + from inspect_robots.task import Task + + with pytest.raises(ConfigError, match="max_seconds must be finite and > 0"): + Task( + name="timed", + scenes=[Scene(id="s", instruction="x")], + scorer="success_at_end", + max_seconds=max_seconds, + ) + + +@pytest.mark.parametrize("control_hz", [None, True, 0.0, -1.0, float("nan"), float("inf")]) +def test_seconds_horizon_rejects_invalid_control_rate(control_hz: float | None) -> None: + from inspect_robots.errors import ConfigError + from inspect_robots.scene import Scene + from inspect_robots.task import Task + + task = Task( + name="timed", + scenes=[Scene(id="s", instruction="x")], + scorer="success_at_end", + max_seconds=120.0, + ) + with pytest.raises(ConfigError, match="control_hz"): + task.resolve_envelope(control_hz) + + +def test_seconds_horizon_rejects_nonfinite_resolved_budget() -> None: + from inspect_robots.errors import ConfigError + from inspect_robots.scene import Scene + from inspect_robots.task import Task + + task = Task( + name="timed", + scenes=[Scene(id="s", instruction="x")], + scorer="success_at_end", + max_seconds=1e308, + ) + with pytest.raises(ConfigError, match="finite step budget"): + task.resolve_envelope(1e308) + + +def test_seconds_horizon_underflow_still_resolves_to_one_step() -> None: + from inspect_robots.scene import Scene + from inspect_robots.task import Task, TaskEnvelope + + task = Task( + name="timed", + scenes=[Scene(id="s", instruction="x")], + scorer="success_at_end", + max_seconds=5e-324, + ) + assert task.resolve_envelope(0.5) == TaskEnvelope(name="timed", max_steps=1) + def test_task_validation_and_scorer_names() -> None: from inspect_robots.errors import ConfigError @@ -143,6 +209,16 @@ def test_task_validation_and_scorer_names() -> None: from inspect_robots.task import Epochs, Task scene = Scene(id="s", instruction="x") + with pytest.raises(ConfigError, match="exactly one"): + Task(name="t", scenes=[scene], scorer="success_at_end") + with pytest.raises(ConfigError, match="exactly one"): + Task( + name="t", + scenes=[scene], + scorer="success_at_end", + max_steps=5, + max_seconds=1.0, + ) with pytest.raises(ConfigError, match="max_steps"): Task(name="t", scenes=[scene], scorer="success_at_end", max_steps=0) with pytest.raises(ConfigError, match="Epochs count"):