From ddf19e3eba7b93bc8f49b9aed732f0febfdb7ccb Mon Sep 17 00:00:00 2001 From: Kayvan Zahiri Date: Mon, 13 Jul 2026 17:10:46 -0700 Subject: [PATCH 1/2] fix: derive scene seed from instance_id, not sorted index tasks.py seeded each Scene with its sorted position, and derive_seed hashes only (eval_seed, scene_seed, epoch), so task identity never entered the seed. Instance i of every task therefore drew from a byte-identical PCG64 stream: 21% of the suite's continuous setup draws were exact cross-task duplicates. Seed from instance_id (unique and task-qualified) instead. Scene seeds go from 5 distinct to 50; cross-task duplicate draws go to zero. This CHANGES EVERY REALIZED SCENE and therefore every published number. Landing it needs a version bump and a changelog note that results are not comparable across the change. test_canonical_instruction_matches_epoch_zero re-derived the seed from index rather than reading scene.init_seed, so it pinned the seeding scheme instead of the invariant it meant to check. It now reads the seed off the Scene. --- src/kitchenbench/tasks.py | 17 ++++++++++++++--- tests/test_tasks.py | 7 ++++++- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/kitchenbench/tasks.py b/src/kitchenbench/tasks.py index 7c69a84..4b9f655 100644 --- a/src/kitchenbench/tasks.py +++ b/src/kitchenbench/tasks.py @@ -14,6 +14,7 @@ from __future__ import annotations import re +import zlib from dataclasses import asdict from typing import Any @@ -40,15 +41,25 @@ def build_scenes(spec: TaskSpec) -> list[Scene]: """Build one Scene per task instance (5 per task).""" scenes: list[Scene] = [] for index, inst in enumerate(spec.instances): + # Derive the scene seed from the instance id, not its sorted position. + # + # instance_id is "/" and is unique across the suite, so this + # decorrelates the per-instance RNG streams. Seeding on `index` gave + # instance i of EVERY task a byte-identical PCG64 stream (derive_seed + # hashes only (eval_seed, scene_seed, epoch) -- task identity never + # entered the payload), so same-shaped distributions at the same sorted + # position realized the SAME number across tasks. + scene_seed = zlib.crc32(inst.instance_id.encode()) & 0xFFFFFFFF # The displayed instruction is the realization of epoch 0 under the default - # eval(seed=0), so it equals the first actual rollout's instruction. - canonical = inst.realize(derive_seed(0, index, 0)) + # eval(seed=0), so it equals the first actual rollout's instruction. It has + # to use the same scene seed the Scene carries, or the two drift apart. + canonical = inst.realize(derive_seed(0, scene_seed, 0)) scenes.append( Scene( id=_slug(inst.instance_id), instruction=canonical.instruction, target=Target(kind=inst.target_kind, spec=dict(inst.static)), - init_seed=index, + init_seed=scene_seed, metadata={ "benchmark": "kitchenbench", "task": spec.key, diff --git a/tests/test_tasks.py b/tests/test_tasks.py index 9f0c8b7..b347f2a 100644 --- a/tests/test_tasks.py +++ b/tests/test_tasks.py @@ -57,7 +57,12 @@ def test_scene_metadata_is_json_native() -> None: def test_canonical_instruction_matches_epoch_zero() -> None: spec = SPEC_BY_KEY["pour_pasta"] for index, scene in enumerate(build_scenes(spec)): - expected = spec.instances[index].realize(derive_seed(0, index, 0)).instruction + # Read the seed off the Scene rather than re-deriving it, so this test + # pins the INVARIANT (displayed instruction == epoch-0 realization) and + # not a particular seeding scheme. + expected = spec.instances[index].realize( + derive_seed(0, scene.init_seed, 0) + ).instruction assert scene.instruction == expected From 62b81bebbe52a758cbb402badf7a48cf9a33ef68 Mon Sep 17 00:00:00 2001 From: Kayvan Zahiri Date: Mon, 13 Jul 2026 21:02:48 -0700 Subject: [PATCH 2/2] chore: bump task versions and add changelog for the seeding change Every Scene stamps spec.version into its metadata, so leaving the versions alone would ship two different scene sets under identical version strings. That is the silent non-comparability this change exists to prevent, so the nine tasks on version=1 go to 2 and scoop_pasta goes 2 -> 3. The changelog states plainly that results before and after this change are not comparable, and records the measurement that justified the break. Note: the PACKAGE version is single-sourced from git tags via hatch-vcs ('nothing is committed to main'), so the minor bump to v0.4.0 has to be a tag cut on your side rather than something this PR can carry. --- CHANGELOG.md | 43 +++++++++++++++++++++++++++++++++++++++ src/kitchenbench/specs.py | 4 ++-- tests/test_tasks.py | 4 +--- 3 files changed, 46 insertions(+), 5 deletions(-) create mode 100644 CHANGELOG.md diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..33ed75d --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,43 @@ +# Changelog + +## Unreleased + +### Changed + +- **Scene seeds are now derived from `instance_id` instead of the instance's + sorted position.** ([#3](https://github.com/robocurve/kitchenbench/issues/3)) + + **Results from before this change are not comparable to results after it.** + Every realized scene is different. Do not compare a policy's score across the + boundary, and do not aggregate runs from either side of it. + + `tasks.py` seeded each `Scene` with `index`, and `derive_seed` hashes only + `(eval_seed, scene_seed, epoch)` — task identity never entered the payload. So + instance *i* of every task drew from a byte-identical PCG64 stream, and 21% of + the suite's continuous setup draws were exact duplicates across tasks + (`stack/jitter_x_cm`, `open_container/container_x_cm` and + `seal_container/lid_offset_cm` were all `+1.0889041282` cm at instance 0). + + Because the tasks shared draws, their scores shared error. Measured over 3000 + eval seeds with a bootstrap CI on the variance ratio, the aggregate score's + variance was inflated 1.5x to 1.9x in the range where the benchmark actually + separates policies (score ~0.35–0.50). The 50 trials carried the weight of + roughly 26–33 independent ones, so confidence intervals computed as if the + trials were independent were about 40% too narrow. + + Seeding from `instance_id` (unique and task-qualified) takes the distinct scene + seeds from 5 to 50 and cross-task duplicate draws to zero. + +- **All task versions bumped** to mark the scene change as machine-readable, not + just documented: the nine tasks on `version="1"` are now `"2"`, and + `scoop_pasta` goes `"2"` -> `"3"`. Every `Scene` stamps `spec.version` into its + metadata, so without this bump two different scene sets would ship under + identical version strings — which is the silent non-comparability this change + exists to prevent. + +### Fixed + +- `test_canonical_instruction_matches_epoch_zero` recomputed the seed from `index` + rather than reading `scene.init_seed`, so it pinned a particular seeding scheme + instead of the invariant it meant to check (displayed instruction == the epoch-0 + realization). It now reads the seed off the `Scene` and is seeding-agnostic. diff --git a/src/kitchenbench/specs.py b/src/kitchenbench/specs.py index 729046f..2b1cf37 100644 --- a/src/kitchenbench/specs.py +++ b/src/kitchenbench/specs.py @@ -34,7 +34,7 @@ class TaskSpec: bimanual: bool max_steps: int instances: tuple[TaskInstance, ...] - version: str = "1" + version: str = "2" description: str = "" @@ -1422,7 +1422,7 @@ def _sort_fixtures( bimanual=True, max_steps=120, instances=_SCOOP_PASTA, - version="2", + version="3", description="Tool-mediated granular handling: manage fill level, then transfer.", ), ) diff --git a/tests/test_tasks.py b/tests/test_tasks.py index b347f2a..7e5d347 100644 --- a/tests/test_tasks.py +++ b/tests/test_tasks.py @@ -60,9 +60,7 @@ def test_canonical_instruction_matches_epoch_zero() -> None: # Read the seed off the Scene rather than re-deriving it, so this test # pins the INVARIANT (displayed instruction == epoch-0 realization) and # not a particular seeding scheme. - expected = spec.instances[index].realize( - derive_seed(0, scene.init_seed, 0) - ).instruction + expected = spec.instances[index].realize(derive_seed(0, scene.init_seed, 0)).instruction assert scene.instruction == expected