fix: derive scene seed from instance_id, not sorted index

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.

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.",
     ),
 )

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 "<task>/<name>" 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,

tests/test_tasks.py

@@ -57,7 +57,10 @@ 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