feat(eval): batchflow evaluation stack

[ElmoPA]

eval/core, eval/dfot and the explorer. The sim landed earlier in this stack, so
the lazy 'from Tsimulation.pushshapes import ...' call sites here resolve.

Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com

[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(configs): hydra config groups for the batchflow stack #558 : 2 dependent PRs (#560 , #564 )
• test: batchflow test suite and retained tooling #557
• feat(eval): batchflow evaluation stack #556 👈 (View in Graphite)
• 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

PR Review: batchflow evaluation stack

Summary

Large refactor moving flat eval/eval_*.py files into role-bucketed subpackages (core/, probes/, hpt/, pi/, act/, dfot/), plus new composite/sim/VAE evaluators and standalone CLI loaders. Backward-compat facade in eval/__init__.py keeps legacy imports alive.

Key concerns

1. ckpt_loading.py has a misleading module docstring — the file is described as a smoke_sim_eval.py script but is imported as a library by boundary_viz.py and overlay_loading.py (they use _MockTrainer and load_algo_from_ckpt). The main() and CLI parsing at module bottom is dead weight for the library role and confusing. Suggest either splitting the library helpers into a dedicated module or clearly marking the file as dual-use.

2. _sys.modules facade registration is unguarded — if any bucketed module fails to import (missing Tsimulation, absent dfot deps, etc.), the whole egomimic.eval package fails to import, cascading into trainHydra. The commit message says "sim landed earlier in this stack, so the lazy from Tsimulation.pushshapes import ... call sites here resolve" — but eval_sim.py does from egomimic.rldb.embodiment.pushshapes_sim import _ENV_TO_ZARR at module top, which itself may import sim. Recommend wrapping each _importlib.import_module in try/except with a soft-fail warning so unused evaluators don't break training entrypoints.

3. del _legacy, _home, _mod at module bottom will NameError if _MODULE_HOMES is empty — currently it isn't, but it's a fragile pattern. Guard with if _MODULE_HOMES:.

4. SimRolloutEval uses signal.SIGALRM on the main thread — this will conflict with PyTorch Lightning's own signal handlers (SLURM requeue, graceful shutdown) and will silently no-op if Lightning is running validation from a non-main thread (e.g., some DDP configurations). At minimum add a check for threading.current_thread() is threading.main_thread() before installing the handler. Also, restoring prev_handler via signal.signal is correct, but if the previous handler was Lightning's SIGALRM callback for something else, we've swallowed a signal delivered during the rollout window.

5. _masked_mse in eval_hnet.py divides by valid.sum().clamp(min=1) — if all rows are entirely padding (shouldn't happen but…) you'd return 0 silently instead of erroring. Consider asserting seq_lens.min() > 0.

6. report_max_coverage conflates two metrics under one key — Valid/emb{emb_id}_sim_coverage means "final IoU" or "peak IoU" depending on a constructor flag. This will silently break metric comparisons across runs if the flag differs. Suggest emitting BOTH under distinct key names (_sim_coverage_final, _sim_coverage_peak) and let downstream pick.

7. obs_stride and replan_every are monkey-patched onto the algo (algo.obs_stride = int(args.obs_stride)) — this bypasses whatever the model's own config declared. Fine for a smoke script but if ckpt_loading.load_algo_from_ckpt is reused inside training-time evals this pattern could leak. Confirm it's script-only.

8. _env_to_zarr_pushshapes / _state_to_init re-exported from pushshapes_sim — the noqa comment says these are used by eval_dfot_self_rollout and scripts/verify_*. Make sure the diff also updates those importers, or the facade keeps them resolvable — I can't verify from the truncated diff.

9. CombinedRowsEval.compute_metrics_and_viz returns ({}, {}) — its actual output goes through on_validation_end. If any orchestrator (e.g., EvalList) calls compute_metrics_and_viz and expects metrics/images, this silently drops them. Document the contract explicitly, or move the vstack logic into compute_metrics_and_viz.

Suggestions

• Test coverage: I see no tests in this diff. For a refactor this large touching the facade sys.modules mechanism, please add:
  • test_eval_facade.py: assert from egomimic.eval.eval_sim import PackedSimEval and from egomimic.eval.core.eval_sim import PackedSimEval return the SAME class object (is check).
  • test_masked_mse.py: sanity on _masked_mse and _unpack_to_padded against a hand-computed case.
  • test_eval_composite.py: EvalVideoList panel concat with mismatched heights/widths under both pad_h modes.
• Add a top-level docstring in SimRolloutEval explicitly listing which algo methods it depends on (inference_step(obs_zarr, t, emb_id, T_max=...)) — right now the contract is only discoverable by reading the rollout loop.
• The trail overlay uses BGR/RGB-ambiguous colors ("cyan-ish (BGR/RGB readable)") — pymunk/gym render typically returns RGB. Just document which color space you're writing in.
• Consider a deprecation_warning on the HNetSimEval alias if we intend to remove it later.
• pi/ docstring block was truncated in what I could see — verify eval_pi.py is actually delegating to _viz_shared.cam_frame_mse_and_viz_batches as advertised in the module comment.

Verdict: Request Changes

The refactor itself is well-organized and the facade pattern matches prior collap

---

Reviewed by Claude · Review workflow