Skip to content

add tests to ensure training works between every model and data vendor - #626

Merged
RyanPCo merged 12 commits into
mainfrom
ryanco/train-step-tests
Sep 20, 2026
Merged

RyanPCo merged 12 commits into
mainfrom
ryanco/train-step-tests

Conversation

@RyanPCo

@RyanPCo RyanPCo commented Sep 12, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Two test tiers that prove HPT and Pi0.5 build and take optimizer steps for every vendor / robot that has configs (eva, aria, mecka, scale), plus the repo fixes they needed.

CPU tier (every PR, ~46 s on a compute node) — tests/unit/test_train_step.py: synthetic zarr episodes written through the real ZarrWriter (tests/fixtures/synthetic_episodes.py), the real top-level + per-vendor data + model configs composed with only the resolver swapped to LocalEpisodeResolver (tests/fixtures/recipes.py), the real trainHydra.train() for 2 steps. Asserts a finite loss and that the policy saw the canonical batch keys. Pi's openpi network is a stub that asserts on every observation field the EgoVerse wrapper must fill (state dim, image masks per camera, 32-wide action chunk).

GPU tier (opt-in) — tests/integration/test_train_step_gpu.py, pytest --integration -m gpu on a GPU node: real configs through the S3 resolver + SQL filters, real Pi weights, 5 bf16 steps, checkpoint written and reloaded.

Also — tests/unit/test_data_configs_compose.py: every data/*.yaml composes, resolves, and its key_map / transform_list nodes instantiate; known-broken configs are strict xfails with a reason.

Repo fixes the tests required

  • pl_model.py: the two torch.distributed.barrier() calls crashed every single-process run; now guarded (and the guard is tested for the DDP path too).
  • 14 data configs used an absolute ${train_datasets.X} for valid_datasets that cannot resolve under the data: mount (only data/human.yaml composed). Converted to the relative form eva_pi/cotrain_pi_base already use on ryanco/infra-fixes, line for line.
  • has_head_pose: false was stale: scale exports do write obs_head_pose (external/scale/scripts/sfs_to_egoverse_zarr.py), and the cartesian transform pivots on it, so scale cartesian had never been trainable. Dropped from scale, scale_pi, mecka_scale_cotrain_pi, industry_eva_pi; docstring + CONTRIBUTING_DATA.md corrected.
  • cotrain_pi_base.yaml human domain: keymap_mode: cartesian_pi (Pi reads base_0_rgb).
  • Test harness: hermetic_env set HF_HUB_OFFLINE with monkeypatch, but huggingface_hub/transformers read it once at import, so the Pi tests actually ran online and 401'd in CI probing the gated PaliGemma repo for chat_template.jinja. It now patches the imported constants too, and the CI unit-test step sets HF_HUB_OFFLINE=1.
  • GPU tier: last.ckpt is opened with torch.load(mmap=True) and its keys/shapes checked against the trained module, instead of reading the ~21 GB Pi checkpoint back just to assert a non-empty state_dict.
  • MultiDataset: norm-stat samples with NaN/Inf are dropped (with a warning) before np.percentile. Found by the GPU tier: eva episode 2026-03-28-19-49-52-786000 has an all-NaN right.cmd_gripper; its NaN went into quantile_1/99 for one action dim and every normalized sample of the run became NaN.

Findings not fixed here (maintainer calls)

  • Shipped pins are stale: data/scale.yaml pins 2026-03-16-01-22-26-448000, which app.episodes records as eva_bimanual; data/mecka.yaml pins 69199812208123403bbdb24f, which is not in the table. The GPU tier pins the eva/aria episodes their configs use and falls back to lab + debug=2 for mecka/scale.
  • aria_pi, mecka_pi, scale_pi, industry_eva_pi, mecka_scale_cotrain_pi call get_keymap without keymap_mode (TypeError at instantiate); cotrain_pi_latent does not compose. All listed as strict xfails.
  • A fully corrupt episode still exhausts the in-episode retry loop and crashes training (Entire dataset bad); left as-is so bad data surfaces rather than being skipped silently.
  • human.py / zarr_dataset_multi.py carry ruff-format churn because pre-commit formats every touched file; the real changes are one docstring sentence and _drop_nonfinite_rows.

Test plan

Run (Slurm) Result
pytest tests/unit on a CPU node 91 passed, 7 xfailed
pytest tests/unit/test_train_step.py 8 passed (4 HPT + 4 Pi stub) in 46 s
pytest tests/unit in a CI-equivalent venv (openpi --no-deps, tokenizer-only HF cache, no token, HF_HUB_OFFLINE=1) 264 passed, 5 xfailed
GPU tier, eva/aria/scale x {hpt, pi}, A40 6 passed
GPU tier, mecka x {hpt, pi}, A40 (post-review code) 2 passed
Final full GPU run on the final branch code, A40 (hoffman-lab) 8 passed in 29 min 30 s
GPU tier after the HF-offline fix, A40 (rl2-lab) 8 passed in 51 min (full checkpoint reload, basetemp on NFS)
GPU tier with the mmap checkpoint check, A40 (rl2-lab) 8 passed in 35 min

Spike that chose the Pi stub over openpi's tiny variant: stub 15-28 s / 1.3 GB vs dummy-gemma 71 s / 13 GB (and openpi's dummy variant needs a projector patch to run at all); both caught the same wiring bugs.

🤖 Generated with Claude Code

https://claude.ai/code/session_01N254Pmk9beYNooGou6T29R

@RyanPCo RyanPCo mentioned this pull request Sep 12, 2026
@RyanPCo
RyanPCo changed the base branch from ryanco/tests-dir to graphite-base/626 September 12, 2026 18:49
@RyanPCo
RyanPCo force-pushed the ryanco/train-step-tests branch from 36f7a14 to 8533391 Compare September 12, 2026 18:49
@RyanPCo
RyanPCo changed the base branch from graphite-base/626 to ryanco/phase1-ci September 12, 2026 18:49
@RyanPCo
RyanPCo force-pushed the ryanco/train-step-tests branch from 8533391 to adabfc3 Compare September 12, 2026 19:39
@RyanPCo
RyanPCo force-pushed the ryanco/phase1-ci branch 2 times, most recently from 3c6f23e to 553a840 Compare September 12, 2026 21:28
@RyanPCo
RyanPCo force-pushed the ryanco/train-step-tests branch from adabfc3 to 3ff83e0 Compare September 12, 2026 21:28
@RyanPCo RyanPCo changed the title spec: train-step tests (HPT + Pi per vendor) design tests: CPU train-step tier, HPT + Pi(stub) x eva/aria/mecka/scale Sep 12, 2026
@RyanPCo
RyanPCo force-pushed the ryanco/train-step-tests branch from 3ff83e0 to 9944c7f Compare September 12, 2026 21:32
@RyanPCo
RyanPCo force-pushed the ryanco/train-step-tests branch from 9944c7f to aafdb1c Compare September 12, 2026 22:01
@RyanPCo
RyanPCo force-pushed the ryanco/train-step-tests branch from aafdb1c to 4d13e75 Compare September 12, 2026 22:21
@RyanPCo
RyanPCo force-pushed the ryanco/train-step-tests branch from 4d13e75 to 4a9c80e Compare September 12, 2026 22:22
@RyanPCo
RyanPCo force-pushed the ryanco/phase1-ci branch 2 times, most recently from 13a2eee to ce80d63 Compare September 12, 2026 22:23
@RyanPCo
RyanPCo force-pushed the ryanco/train-step-tests branch from 4a9c80e to 55dc089 Compare September 12, 2026 22:23
@RyanPCo
RyanPCo force-pushed the ryanco/train-step-tests branch from 985c225 to 580ca5f Compare September 13, 2026 17:16
RyanPCo added a commit that referenced this pull request Sep 22, 2026
## Summary
Every dataset had a `_pi` twin of its data config that differed in exactly one line: `keymap_mode: cartesian_pi`, which made the dataset emit PaliGemma camera names (`base_0_rgb`, `*_wrist_0_rgb`) instead of the dataset names HPT reads. The model's input naming had leaked into the data configs, and three of the twins (aria, mecka, scale) had drifted to not even instantiate (the xfails in #626).

This PR moves the renaming into the Pi wrapper, where it belongs, and collapses the twins.

**Pi wrapper** — `PI_CAMERA_SLOTS` (`egomimic/models/preprocess_pi_obs.py`) maps openpi's three slots onto the dataset keys every embodiment's keymap emits; `gather_pi_images` pulls them from the batch, still accepting the slot names themselves as a fallback so robot rollout (which already emits both) and pre-remap checkpoints keep working. Missing wrist slots are filled from the front camera and masked out, as before. `_fill_missing_images` and the `pi_cam_keys` kwarg are gone (`pi_camera_slots` overrides the map if a model ever needs to).

**Keymaps** — the `cartesian_pi` branch in `Eva._get_keymap` and the `_pi`-suffix handling in `Human._get_keymap` are removed; a `_pi` mode is now a deprecated alias for the base mode (`FutureWarning`), so configs saved by earlier Pi runs still rebuild for eval and resume.

**Data configs**
- `aria_pi`, `mecka_pi`, `scale_pi` deleted; Pi trains on the per-vendor configs. At this commit those still carry #626's pins (stale for `scale` and `mecka`); the validated pins land in #628 (`ryanco/phase2-pins`), which stacks on top of this PR.
- Every remaining `cartesian_pi` (`cotrain_pi_base`, `cotrain_pi_lang`, `cotrain_pi_no_lang`, `eva_pi_lang`, `cotrain_pi_pickplace_qwen`) is now `cartesian`.
- `industry_eva_pi` and `mecka_scale_cotrain_pi` are distinct cotrain selections, so they stay, and get the `keymap_mode`/`mode` they never passed. Both instantiate now.
- `eva_pi` stays as `train_zarr_cartesian_pi`'s default data, now plain `cartesian`: every `eva_bimanual` episode with annotations. `data=eva` is a one-episode smoke config with no annotations, so it can't be the Pi default.
- Pi's `default_prompt` (`pi0.5_base`) is now empty instead of "This is a bad action.". The three `pi_cartesian*` viz configs read the dataset front key.

**Tests**
- `tests/unit/test_pi_camera_slots.py`: the slot gather (dataset names, legacy names, precedence, human front-only, BHWC, no-camera error) and the keymap rejection.
- `test_data_configs_compose.py`: the five xfails are gone; new cases assert no data config uses a `_pi` mode and that `train_zarr_cartesian_pi` defaults to unpinned, annotated eva data.
- Train-step recipes: Pi rows use the same vendor configs as HPT. They opt into `annotation_key=annotations` because the plain vendor configs load no language field. The tests now assert the wrapper saw dataset camera names while `StubPI0` still asserts openpi's slot names, which is the remap working end to end.
- `eval_latent.py` updated for the new `_robomimic_to_pi_data` signature.

## Findings not fixed here
- `PI._sample_prompts` silently falls back to `default_prompt` when `annotation_key` is set on the model but the data config never loaded that field. `data=eva model=pi0.5_bc_eva` trains on the empty default prompt for every sample without a warning. Worth a loud error or at least a log line.
- The scale HPT GPU row failed twice in a row on `Entire dataset bad` for two different scale episodes (`2026-05-02-17-05-10-632372`, `2026-05-02-16-51-26-226166`) picked by the pre-pin `debug=2` fallback. Same data-quality class as the eva episode #626 reported; the pins in #628 (`ryanco/phase2-pins`, above this PR) sidestep it.
- Branches in the main clone (`ryanco/infra-fixes`, `aidan/kp-wristframe-port`) still say `cartesian_pi` in `mecka_all_pi_6d` and `obj_gen_pi_lang`; they will log a deprecation warning once this merges. One-line fixes.

## Test plan
| Run (Slurm) | Result |
|---|---|
| `pytest tests/unit` on a CPU node (final restacked commit) | 301 passed, 0 failed |
| GPU tier, `tests/integration/test_train_step_gpu.py`, A40, real Pi weights | Pi rows eva, aria, mecka, scale: 4 passed. HPT rows eva, aria, mecka: 3 passed. HPT scale: failed on a corrupt episode (findings). |

GPU runs were on the pre-restack commit (`2d3f408c6`). `ryanco/phase2-pins` now sits above this branch, so the restack reverted my edits to the `eva`/`mecka`/`scale.yaml` filters back to #626's pins; the GPU test overrides the episode filter anyway, and the CPU tier uses synthetic episodes. Three GPU jobs were needed: the first filled the node's local `/tmp` with Pi checkpoints (pytest's default basetemp), the second lacked `s5cmd` on PATH for uncached episodes. Both were harness setup, not code.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01KxKxffCeQb3aTgvia4cLYT
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant