refactor(configs): organize Hydra configuration hierarchy

[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

• refactor(configs): organize Hydra configuration hierarchy #560 👈 (View in Graphite)
• feat(configs): hydra config groups for the batchflow stack #558 : 1 other dependent PR (#564 )
• test: batchflow test suite and retained tooling #557
• feat(eval): batchflow evaluation stack #556
• feat(algo): H-Net, diffusion and BC algorithms #555
• feat(models): H-Net and diffusion model zoo #554
• refactor(sim): give agents their own interface so the env stops branching on them #559
• sim: Tsimulation with sim_v1 and sim_v2 #553
• feat(infra): batchflow pipeline core, rldb unions, collapsed embodiment enum #552
• main

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

[github-actions]

Claude Code Review

Review of PR #560: refactor(configs): organize Hydra configuration hierarchy

Summary

Large-scale reorganization of Hydra config files into a hierarchical directory structure (e.g., data/aria.yaml → data/debug/single_episode/human_cartesian.yaml, callbacks/checkpoints.yaml → callbacks/checkpoint/base.yaml). Mostly file renames plus some refactoring of callback bundles to use defaults: composition.

Key concerns

1. 🚨 Syntax error in eval_sim.py

# Backward-compat alias — old configs / docs may reference HNetSimEval;
# the class is algo-agnostic so the rename to PackedSimEval is purely
        # cosmetic. Existing evaluator/hnet/sim.yaml and downstream callers keep working.
HNetSimEval = PackedSimEval

The third comment line has stray indentation (8 spaces instead of 0). While Python tolerates this on comment-only lines at module scope, it's a lint/style regression that was likely introduced by an automated find/replace. Fix indentation.

2. 🚨 Broken callback composition in bundle/chunker.yaml

defaults:
  - /callbacks/scheduler/chunker_residual@_here_
  - _self_

chunker_residual_scheduler:
  schedule:
    - [0, 0.0]
    - [959, 0.0]
    ...

The bundle no longer defines _target_ for chunker_residual_scheduler (removed in this refactor), relying on the include. But the original file had model_checkpoint defined inline without pulling in /callbacks/checkpoint/base — now the bundle is missing the base model_checkpoint unless it's already there. Please verify: does this bundle still emit both callbacks correctly? Suggest instantiating it (hydra.utils.instantiate) as a smoke test.

Also worth double-checking: the original ckpt_chunker.yaml had every_n_epochs: 100 — is that preserved by whatever checkpoint/base sets, or has the default silently changed?

3. ⚠️ Silent semantic drift risk

Several renames slightly rescope files:

• data/aria.yaml → data/debug/single_episode/human_cartesian.yaml — this is now labeled as "debug/single_episode" but it's referenced in CONTRIBUTING_DATA.md as the canonical head-mounted Aria config to copy from. Users onboarding new data will see it under debug/ and reasonably assume it's not production-ready.
• data/eva.yaml → data/debug/single_episode/eva_cartesian.yaml — same concern.

If these are still the canonical templates, they shouldn't live under debug/. If they're strictly debug configs, then CONTRIBUTING_DATA.md needs a different pointer.

4. ⚠️ Grep for stale references

This PR renames ~50+ files. The diff shows updates to AGENTS.md, README.md, CONTRIBUTING_DATA.md, and a couple of docstrings. But please grep the whole tree (including .py files that build config paths dynamically, launch scripts, submitit sweeps, wandb sweep configs, notebooks) for:

• tsimulation (many references still likely exist)
• cotrain_pi_base, cotrain_pi_lang, cotrain_pickplace_qwen*
• ckpt_chunker, ckpt_ratio_loss_scheduler, checkpoints_ema, etc.
• eval_hnet, eval_bcrnn, data_schematic/default, data_schematic/hpt

The data_schematic/default.yaml and data_schematic/hpt.yaml files were deleted — where did they move? I don't see the corresponding rename in the diff (though it's truncated). If they weren't renamed, this is a hard break.

5. ⚠️ _here_ semantics

Many bundles now use - /callbacks/base@_here_ or - checkpoint/base@_here_. Please confirm you tested that override precedence still works — e.g., bundle/ratio_loss_700.yaml overrides switch_fraction: 0.35, and the merged result actually reaches RatioLossScheduler.__init__. The old flat configs made this trivially obvious; the new composed configs move the _target_ into an included file, so a typo in the include path yields "no callback instantiated" rather than an error.

6. ⚠️ Test coverage

No tests added. Given this touches every training entrypoint, at minimum please:

• Run hydra.compose + hydra.utils.instantiate on every renamed group (there's precedent in tests/)
• Run one smoke training for each affected algo family (HNet packed, HPT, Pi cotrain)

Suggestions

1. Fix the stray indentation in eval_sim.py.
2. Add a tests/regression/test_config_composition.py that iterates every leaf yaml in hydra_configs/ and asserts hydra.compose(...) succeeds.
3. Reconsider debug/single_episode/ naming for what appear to be canonical single-embodiment configs, OR update CONTRIBUTING_DATA.md to point to a non-debug template.
4. Confirm data_schematic/{default,hpt}.yaml were moved, not just deleted.
5. Do a repo-wide rg sweep for old config names and update remaining references (docs, scripts, sweep files, comments in Python).
6. In the PR description, list the renames so reviewers/researchers can update their local launch scripts. This will silently break every in-flight experiment otherwise.

Verdict

Request Changes

The reorganization is reasonable but this is a research codebase and this PR has high blast radius: it renames the entry points used by every active experiment

---

Reviewed by Claude · Review workflow