Contribution Update

[ElmoPA]

No description provided.

[ElmoPA]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• feat(zarr): persist camera intrinsics in zarr.json #386
• feat: add LightWheel embodiment #381
• Contribution Update #382 👈 (View in Graphite)
• refactor: move intrinsics/extrinsics into embodiment classes #380
• main

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

[github-actions]

Claude Code Review

PR Review

Summary

Documentation-only PR adding two new subsections to CONTRIBUTING_DATA.md: §8.1 on registering an embodiment class with a LightWheel example, and §10.3 on visual verification via the zarr viz notebook. Also adds two checklist items.

Key concerns

1. Incomplete get_transform_list in the example. The example only handles mode == "cartesian" and implicitly returns None for all four keypoints_* modes, even though the Literal type signature advertises them. This is misleading to copy-paste contributors — they will silently get None back when training a keypoint config and hit a confusing downstream error. Either:

   • Delegate all modes to Aria.get_transform_list(mode=mode) (recommended, since the prose explicitly says "Delegating … to Aria is the recommended starting point"), or
   • Add an explicit else: raise NotImplementedError(...) and narrow the Literal to just "cartesian".

   Right now the code example contradicts the prose immediately below it.

2. ACTION_STRIDE = 3 justification is off. The note says "typically 3 for ~30 fps egocentric data," but ACTION_STRIDE is a temporal subsampling stride on action chunks, not a function of fps alone — it depends on the policy's target control rate. Worth either dropping the "~30 fps" rationale or pointing to where it's actually consumed so contributors don't cargo-cult the wrong value for, e.g., 60 fps capture.

3. Class name / embodiment string consistency. §8 enumerates the canonical embodiment strings that must match the enum exactly. The example introduces LightWheel as a class but doesn't show the corresponding embodiment string (lightwheel_bimanual? lightwheel?) or note that the class must be wired into whatever registry maps embodiment-string → class. New contributors will register a class and still not have it discoverable. Worth either showing the registry/enum addition or linking to it.

Suggestions

• Fix the example to return Aria.get_transform_list(mode=mode) for all modes, or raise on unsupported modes.
• In §10.3, mention that the notebook should be run on a post-transform sample (head-frame) vs raw zarr (SLAM world frame) — misalignment can come from running viz on the wrong representation, and that's a common gotcha worth pre-empting.
• Consider adding a one-liner that EXTRINSICS = None is only valid because Aria's transform pipeline re-expresses SLAM-world poses into head frame at load time; otherwise a reader might think "no extrinsics = no frame conversion needed."
• Minor: §8.1 references egomimic/rldb/embodiment/human.py — confirm that's the actual filename (vs humans.py or similar) before merging.

Verdict: Request Changes

Docs-only, so low blast radius, but concern #1 is a real footgun — the snippet contributors will copy will silently break non-cartesian training. Quick fix and this is good to merge.

---

Reviewed by Claude · Review workflow