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 👈 (View in Graphite)
• main

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

[github-actions]

Claude Code Review

Review of PR #386

Summary

This PR is described as "persist camera intrinsics in zarr.json" but actually does much more than that — it collapses all human embodiments (aria_*, mecka_*, scale_*) into a single human_* embodiment, rewrites coordinate conventions for Aria, restructures embodiment classes, and updates ~30 config files. The intrinsics work itself looks reasonable, but the scope creep is dangerous.

Key concerns

1. PR scope vs. title — major mismatch

The title says "persist camera intrinsics" but the diff includes:

• Embodiment collapse (aria_bimanual → human_bimanual, Aria/Mecka/Scale classes → single Human)
• Aria right-hand EE-pose / wrist-pose orientation fix (README admits this is a canonical convention change)
• Removal of CameraTransforms from HPT.__init__ (dead param now)
• Embodiment ID renumbering (1-3 for human, 4-6 for eva — was 3,4,5 for aria, 6,7,8 for eva)

Each of these is a separate, breaking change. They should be separate PRs, or at minimum the PR title/description must say so. As-is, reviewers will under-scrutinize the orientation fix and the embodiment renumbering.

2. Embodiment ID renumbering will silently break old checkpoints/data

Old IDs: aria_bimanual=5, eva_bimanual=8. New IDs: human_bimanual=3, eva_bimanual=6. Any code, checkpoint, or stored norm-stat keyed on the integer ID will silently mis-map. Was there a discussion to keep new IDs disjoint from old (e.g., start at 20)? Reusing IDs 1-6 with different meanings is a footgun.

3. SQL/DB inconsistency is now load-bearing and undocumented in code

human.yaml filter still queries robot_name == 'aria_bimanual' (correctly, since SQL hasn't been migrated), but the data config is keyed human_bimanual, and CONTRIBUTING_DATA.md tells contributors to write embodiment="human_bimanual" in both zarr.attrs and the DB row. So:

• Old episodes: DB says aria_bimanual, zarr says aria_bimanual (until reprocessed).
• New episodes per the new docs: DB says human_bimanual, zarr says human_bimanual.
• The human.yaml filter only matches the old. New contributors following the new docs will be filtered out.

This needs to be reconciled before merge — either migrate the SQL column now, or document a transitional dual-write/dual-read.

4. Aria orientation "fix" is a coordinate-frame change

README says: "fixed the right-hand EE-pose and wrist-pose orientations ... so that, after reprocessing, human (aria) hand orientations follow the same canonical right-handed convention as the robot (Eva)". This means:

• All existing Aria zarrs are now wrong in the new convention.
• Any training run using existing Aria data + new code will silently train on inconsistent right-hand orientations vs. the new transforms.
• Norm stats computed on old data won't match new data.

This must be gated explicitly. At minimum: a version field in zarr.attrs (e.g., convention_version: 2) and a runtime assertion in the loader. Otherwise mixed old/new caches will silently corrupt training.

5. Intrinsics validation in create_and_write

The doc claims create_and_write raises if intrinsics is not a non-empty dict, and validates 3×4 shape. I don't see the actual zarr_writer.py changes in this diff (truncated at 80k). Please confirm:

• The "front" key requirement from §10 validation is also enforced at write time (not just in validate_episode).
• extrinsics=None is the documented default for human, and the writer signature reflects that (not a required kwarg).
• The 3×4 (zero last column) convention is enforced, not 3×3. The doc is unusually emphatic about this — suggests it has bitten people before.

6. Test coverage looks thin for the scope

Only test_pi.py is updated, and only with string rename aria_bimanual → human_bimanual. For a change this large I'd expect:

• Tests that ZarrWriter.create_and_write raises on missing/malformed intrinsics.
• Tests that ZarrEpisode.intrinsics round-trips ndarray and dict correctly.
• Tests that Human.get_keymap(has_head_pose=False) produces the right keys.
• A test that the merged mecka + scale filter in industry_eva_pi.yaml actually loads both labs.

7. industry_eva_pi.yaml and mecka_scale_cotrain_pi.yaml — judgment call buried in comments

The comment "JUDGMENT CALL to sanity-check: mecka has a head pose and scale does not, so the merged human keymap uses has_head_pose=false (scale-compatible)" is exactly the kind of decision that silently regresses training. Mecka had a head pose; dropping it for cotrain means the model loses that signal on mecka data. Has anyone confirmed this doesn't regress the active Mecka+Eva cotrain experiments?

8. Minor

• hpt.py: self.camera_transforms = camera_transforms removed but the param is still in __init__ signature — fine, but worth a deprecation comment on the param itself (only in the YAML it has one).
• scale_pi.yaml valid filter is broken: task: - "lambda row: ..." is a list under a task key, not a filter_lambdas list. This was probably broken before, but worth fixing while touching.
• pi_cartesian.yaml: missing trailing newline

---

Reviewed by Claude · Review workflow