feat(zarr): persist camera intrinsics in zarr.json

[ElmoPA]

Add an intrinsics parameter to ZarrWriter (and create_and_write) that
serializes a single np.ndarray K matrix or a {camera_key: K} dict into
zarr metadata. Expose ZarrEpisode.intrinsics / ZarrDataset.intrinsics
that read it back as ndarray(s), or None when not written.

This unblocks per-episode intrinsics so viz no longer has to rely on
class-level Embodiment.INTRINSICS constants.

[ElmoPA]

• feat(zarr): persist camera intrinsics in zarr.json #386 : 2 dependent PRs (#482 , #483 ) 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[github-actions]

Claude Code Review

Summary

Massive PR that combines three orthogonal changes: (1) the advertised feature — persisting camera intrinsics in zarr.json, (2) collapsing all human embodiments (aria_*, mecka_*, scale_*) into a single human_* embodiment, and (3) hardening PI._get_control_mode to remove the aria fallback. The scope is far broader than the PR title/description advertise.

Key concerns

1. PR scope mismatch — this is not a single-feature PR

The title says "persist camera intrinsics in zarr.json" but the diff also:

• Renames the entire aria_*/mecka_*/scale_* embodiment enum to human_* (breaking change, ids re-numbered).
• Collapses the Aria/Mecka/Scale embodiment classes into a single Human class.
• Reassigns integer embodiment ids (Eva was 6/7/8 → now 4/5/6; aria_bimanual was 5 → gone).
• Removes HPT.camera_transforms assignment.
• Removes the aria fallback in PI._get_control_mode.

Each of these deserves its own PR + review. Please split.

2. Embodiment integer id renumbering is a silent data-corruption risk

Old eva_bimanual = 8, new eva_bimanual = 6. Any checkpoint / cached norm-stats / logged artifact that stored the integer id is now silently mis-mapped to a different embodiment. If integer ids are used anywhere in checkpoints, replay buffers, or persisted training state, existing runs will train against the wrong per-embodiment head/stats without erroring. This needs either (a) preserving old integer assignments and only appending new ones, or (b) an explicit audit of every persisted use of the int id.

3. SQL migration is assumed but not included

human.yaml's own comment says: "this ASSUMES the SQL embodiment/robot_name column has been migrated from the old vendor values ('aria_bimanual'). Until that migration lands, this dataset matches zero rows." Shipping data configs that match zero rows will silently produce empty dataloaders (or crash at first-batch depending on MultiDataset behavior). The SQL migration script + a preflight check should land before or with this PR, not after.

Related: the README changelog tells users to re-download data, but there's no migration path for the DB itself, and robot_name conventions in CONTRIBUTING_DATA.md changed (aria_bimanual → human_bimanual) — existing rows with robot_name="aria_bimanual" will be filtered out by any config that switches to row['embodiment'] == 'human_bimanual'.

4. Intrinsics contract is stricter than the codebase can honor today

ZarrWriter.create_and_write now raises if intrinsics isn't a non-empty dict, and CONTRIBUTING_DATA.md calls it MANDATORY. But:

• The README changelog itself says "Mecka and Scale will be asked to add intrinsics to their exports." So today's Mecka/Scale conversion scripts presumably cannot call create_and_write without erroring. Have you verified mecka_to_zarr.py / scale_to_zarr.py pass intrinsics?
• 3×4 K with a zero column is unusual — standard convention is 3×3. Enforcing 3×4 in the writer will bite anyone constructing K from calibration output. At minimum, accept 3×3 in the writer and pad internally, or make the shape check crystal clear in the ValueError.

5. Judgment calls buried in configs

industry_eva_pi.yaml and mecka_scale_cotrain_pi.yaml comments say: "mecka has a head pose and scale does not, so the merged human keymap uses has_head_pose=false (scale-compatible) for a consistent representation." This silently drops the head-pose signal from Mecka episodes in every training run using these configs. That's a real regression for anyone whose current runs relied on mecka head pose. Flag this loudly in the changelog, not in a yaml comment.

6. HPT.camera_transforms removal

self.camera_transforms = camera_transforms was deleted from HPT.__init__ with no replacement. If any downstream code (viz, eval) reads self.camera_transforms on an HPT instance, it now AttributeErrors. Was the attribute unused everywhere, or only in the paths you happened to hit? Grep confirmation needed.

7. PI._get_control_mode — behavior change

Removing the aria default is correct in spirit, but any existing pi model yaml that omitted an explicit control_mode entry for aria will now hard-crash at first forward pass. Make sure every checked-in pi yaml has explicit control_mode entries for every embodiment it can encounter (including human_* post-rename).

8. Tests are shallow for a change this size

The test_pi.py diff is purely a string-rename (aria_bimanual → human_bimanual). There's no new test for:

• ZarrWriter round-trip of intrinsics (create_and_write → read back via ZarrEpisode.intrinsics).
• ZarrWriter rejection of malformed intrinsics (missing, 3×3 instead of 3×4, empty dict, non-dict).
• ZarrEpisode.intrinsics returning None for legacy episodes.
• get_embodiment_id behavior on legacy strings (should hard-crash per README — is there a test?).

Suggestions

• Split the PR. Land intrinsics-writer feature first (with tests), then the embodiment collapse (with SQL migration + id-preservation), then the pi/hpt cleanups.
• Preserve old integer ids — never renumber. Assign new ids sequentially past the existing max.
• Add a preflight check in `M

---

Reviewed by Claude · Review workflow