test: batchflow test suite and retained tooling - #557
Conversation
|
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.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
088a9cf to
48db95d
Compare
0b796a7 to
ae704d6
Compare
3806b2a to
0c3a107
Compare
The tests covering the preceding layers, plus the scripts worth keeping: the CUDA kernel build, ops/, the eval entrypoint, and the sim replay tooling. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
They exercise egomimic.algo.diffusion / egomimic.eval.dfot, which move to the DFoT PR stacked above. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
No test referenced MultiDataModuleWrapper at all, which is why the missing pack_collate dispatch survived: test_packed_pipeline covers the *other* pack_collate call site (norm-stat inference), so the subsystem looked tested. Asserts the dispatch itself and that it reaches the DataLoader, for both the train and valid loaders, and that unpacked datasets still get annotation_collate. Uses MagicMock(spec=ZarrEpisodePackedDataset) -- the same pattern test_packed_pipeline already uses -- so it needs no dataset on disk. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
All four groups were stale tests, not broken code. Nothing under egomimic/ is touched by this commit. test_training_recipe (7): PackedAlgoBase takes an assembled outer_stage now -- action_dim / action_horizon / d_model / cond_encoder / hnet moved onto HNetOuterStage. The tests still passed them flat, so __init__ raised TypeError for the missing positional. They now build the HNetOuterStage and pass it; what each test asserts (lr stamping, parameter_groups, init_weights_range) is unchanged. test_pi (4): skipped at module level, not repaired. They describe a PI.visualize_preds API that exists nowhere -- no branch of this repo defines it (main included), egomimic.algo.pi.algo exposes no draw_actions to monkeypatch, and neither EgoVerse2 nor EgoVerse-gmm-dualstream implements it, so it was never ported into this lineage rather than dropped from it. Kept rather than deleted so the intent stays on record. (Their first error was a stale aria_bimanual domain, renamed to human_bimanual by the human/eva collapse; fixing that only exposed the missing method underneath.) test_core_defaults_byte_identical (1): the tx forward checksum was never a valid invariant. tx's output cancels from an absolute sum of 7163 down to a signed sum of ~1e-5, so the value moves with BLAS reduction order -- measured -1.21e-05 at one thread vs -4.43e-05 at two, on identical weights -- while the test compared it to a fixed ~1e-6 absolute bound. The reference simply captured one machine's rounding noise; refreshing it would fail again elsewhere. The guard is now the ABSOLUTE sum, stable to ~1e-8 relative across thread counts and still sensitive to any real forward change, with the signed sum kept at a tolerance scaled to the magnitude summed. lstm and hnet were unaffected because their sums are O(1) and O(100) and do not cancel. test_packed_pipeline (1): PUSHT_FOLDER pointed one directory level too high. circle/ now holds a basic/ subfolder rather than .zarr episodes and the resolver does not recurse, so the isdir() skip-guard passed while the resolver matched nothing. Same drift as data/pushshapes/packed_episode/simulation/delta.yaml. Correcting it also un-skipped three dataset-gated tests, which pass. Suite: 13 failed / 297 passed -> 0 failed / 309 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude Code ReviewReview of PR #557SummaryAdds a batch of standalone smoke/regression test scripts under Key concerns
Suggestions
Verdict: Request ChangesThe intent (retain useful tooling + smoke coverage of the packed pipeline) is good and doesn't touch training code, but as landed these are unrunnable outside your workstation and don't actually integrate into the test suite. Address the path portability + the tests-vs-scripts distinction and this is a clean merge. Reviewed by Claude · Review workflow |
1 participant
The tests covering the preceding layers, plus the scripts worth keeping: the
CUDA kernel build, ops/, the eval entrypoint, and the sim replay tooling.
Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com