Repository navigation
Conversation
e642184 to
bd04b38
Compare
45d3f6f to
e47c29b
Compare
f5b66c3 to
ca35d13
Compare
6f6e2ad to
3207022
Compare
3207022 to
bf08604
Compare
ca35d13 to
64165e7
Compare
RyanPCo
left a comment
RyanPCo
left a comment
There was a problem hiding this comment.
Automated adversarial review (read-only; findings were checked by a second skeptical pass before posting): 3 low, 1 nit.
| return deliveries, rejected | ||
|
|
||
|
|
||
| def load_eva_deliveries( |
There was a problem hiding this comment.
low (duplication): load_eva_deliveries repeats about 40 lines of load_deliveries: the recording, task and alias dicts, the steps_key check and the Delivery build. Only the metadata source differs. Extract a _delivery(d, m, steps, fields) helper that both loaders call with their own metadata fields, so a new RECORDINGS column is added in one place. The aria video list (("front_1", "front_1_480")) is also hard-coded in both check_delivery and here.
| this splits both slots and keeps the robot's arms. | ||
| """ | ||
| feats = EvaHD5Extractor.process_episode(raw, arm=arm, with_images=False) | ||
| numeric = { |
There was a problem hiding this comment.
low (duplication): This comprehension re-implements eva_to_zarr._separate_numeric_and_image. With with_images=False there are no image keys, so numeric, _ = _separate_numeric_and_image(feats) gives the same result and keeps the "same path as eva_to_zarr" promise in the docstring. The "images" not in k clause is redundant either way.
| """ffmpeg reading rgb24 frames on stdin into the delivery-spec AV1 MP4.""" | ||
| cmd = [ | ||
| ffmpeg, "-hide_banner", "-loglevel", "error", "-y", | ||
| "-f", "rawvideo", "-pix_fmt", "rgb24", "-s", f"{w}x{h}", "-r", str(int(fps)), "-i", "-", |
There was a problem hiding this comment.
low (correctness): str(int(fps)) truncates a float fps. eva_to_lance's --fps is a float and the step timestamps use 1e9 / fps, so --fps 29.97 writes an MP4 tagged 29 fps while the steps are spaced at 29.97. Pass str(fps) (ffmpeg accepts decimals), or make the parameter an int and validate --fps.
| rid = recording_id("eva", args.vendor_recording_id) | ||
|
|
||
| cols, dropout = numeric_columns(raw, arm) | ||
| n = len(cols["left.obs_ee_pose" if arm != "right" else "right.obs_ee_pose"]) |
There was a problem hiding this comment.
nit (simplification): n = len(dropout) gives the same frame count without the conditional key lookup.
One HDF5 demo becomes one recording with one task over it. Numerics go through EvaHD5Extractor and eva_to_zarr's per-arm split, so bimanual samples match zarr to float32 precision; all-zero pose frames stay forward-filled and are flagged invalid per arm. Frame times are the demo start plus i/30 s, since the HDF5 has none. Cameras are encoded at 480x360 AV1. A one-arm robot's data sits in its own slot of the (T, 14) arrays; eva_to_zarr reads a lone right arm from the left slot (zeros), so this splits both slots and keeps the robot's arms. The pilot committer takes --lab eva and reads catalog rows from meta.json. check_eva_samples compares Lance and zarr samples. Claude-Session: https://claude.ai/code/session_01Kt1E8T2x8kdV9akAvbJyVd
The Lance lab column is now the collecting lab, so deliveries carry the metadata lab as lab (not site) and end_effector parallel_jaw_gripper, and the committer rejects deliveries without a valid one. The HDF5 has no per-frame times, so the frame rate is a required flag rather than a hardcoded 30. The pilot committer's --lab (delivery format) becomes --source, and the EVA config and sample check drop the resolver's lab argument. Claude-Session: https://claude.ai/code/session_01HjCQBqQCr2igFEMR7yUovw # Conflicts: # egomimic/scripts/eva_process/eva_to_lance.py
A demo whose metadata has no operator (or an empty or placeholder one) was written with operator null; lab and task_name were passed through the same way. All three are never waivable, as in ZarrWriter and the MCAP/zarr validators, so convert() now checks them with required_metadata_error before doing any work and fails the episode naming each bad field. scene and objects may still be missing. Claude-Session: https://claude.ai/code/session_01HjCQBqQCr2igFEMR7yUovw
bf08604 to
8deb758
Compare
64165e7 to
6278aec
Compare
|
Review findings: status after the fixup pass. The fixes are squashed into this PR's commits, and the whole stack was force-pushed. Fixed
Not fixed
|
One HDF5 demo becomes one recording with one task over it. Numerics go
through EvaHD5Extractor and eva_to_zarr's per-arm split, so bimanual
samples match zarr to float32 precision; all-zero pose frames stay
forward-filled and are flagged invalid per arm. The HDF5 has no frame
times, so the frame rate is a required --fps and frame i is at the demo
start plus i/fps. Cameras are encoded at 480x360 AV1. Deliveries carry
the metadata lab as
laband end_effector gripper; the committer rejectsone without a valid lab and end_effector.
A one-arm robot's data sits in its own slot of the (T, 14) arrays;
eva_to_zarr reads a lone right arm from the left slot (zeros), so this
splits both slots and keeps the robot's arms. The pilot committer takes
--source eva and reads catalog rows from meta.json. check_eva_samples
compares Lance and zarr samples.
https://claude.ai/code/session_01HjCQBqQCr2igFEMR7yUovw