Add correctness-contract tests for train-data-pair-dispatch (+ extract schedule helpers) - #1
Open
zhihengy wants to merge 4 commits into
Conversation
Rebase the train-data-pair-dispatch refactor (flat train-pair dispatch in RolloutManager + scaler/KL/needs_timestep_scaling) onto the new upstream main. Adopt upstream's --update-weight-target-module CLI arg (drop the refactor's per-model train_pipeline_config variant). [reward stats] stdout now comes from upstream. Squashed due to the upstream pre-commit lint reformat touching every file the refactor changed.
Move build_microbatch_schedule and validate_same_microbatch_counts_across_dp out of actor.py (which imports ray/diffusers/sglang at module load) into miles/utils/train_data_utils.py (torch + types only), so they can be unit-tested on a CPU CI runner without dragging in the heavy import chain. Also factor the DP count comparison into a pure microbatch_counts_agree() helper, separated from the all_gather_object collective, so the comparison logic is testable without a process group. No behavior change: the functions are moved verbatim and actor.py imports the same names. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
zhihengy
force-pushed
the
feat/train-data-dispatch-ci
branch
from
June 22, 2026 18:52
8bdc5f7 to
7e374c0
Compare
Add tests/fast/utils/test_train_data_utils.py (register_cpu_ci,
suite=stage-a-cpu, labels=[] -> runs on every PR). 23 tests covering the
pure-logic invariants of the flat train-pair path:
- flatten: one pair per selected step, sample-major order, latent/next_latent
offset, advantage broadcast (vs separate raw_reward), scheduler meta from
the first sample; required-step-indices and empty-output guards.
- DP split: contiguous equal shards, drop-tail + warning, scheduler-meta
broadcast, sigmas-optional, raises on num_pairs < dp_size / dp_size <= 0.
- microbatch schedule: golden, perfect-cover, short trailing batch, absolute
offsets; equal shards -> equal per-rank micro-batch counts.
- scheduler-meta reconstruction: fallback sigmas, stored sigmas, missing
timesteps.
CPU-only, ~1s.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add tests/e2e/test_train_pair_cond_equivalence.py (register_cuda_ci). Not
wired into any PR stage -- no GPU runner -- so CI stays CPU-only; run locally.
10 tests on a tiny random-weight QwenImageTransformer2DModel:
- conditioning assembly: expand vs collate build identical encoder_hidden_states
for a same-sample micro-batch (exact); collate pads + masks variable lengths.
- a same-sample forward is identical whichever path assembled the cond (exact).
- batching different-length samples together (pad + mask) leaves each sample's
forward unchanged vs running it alone (tolerance: cross-batch fp rounding).
- classifier-free guidance: joint positive+negative collate order/mask, and the
cfg_combine formula including the true-cfg rescale branch.
- the per-pair forward is invariant to micro_batch_size (tolerance).
- driving the real _forward_train_pair_batch: accumulated gradient is invariant
to micro_batch_size (tolerance).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
zhihengy
force-pushed
the
feat/train-data-dispatch-ci
branch
from
June 22, 2026 19:26
7e374c0 to
23ac44f
Compare
Rockdu
force-pushed
the
refactor/train-data-pair-dispatch
branch
3 times, most recently
from
June 27, 2026 06:54
51e451e to
f351d6c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
PR radixark#10 (train-data-pair-dispatch) reshaped diffusion-GRPO training data from a 2D (sample x SDE-step) grid + tiling into a flat list of train-pairs: rollout flattens samples into pairs -> contiguous equal split across DP ranks -> the trainer feeds contiguous micro-batches to the forward, normalizing gradients as
loss_sum / num_local_pairs. Correctness rests on a set of invariants (flatten completeness, contiguous DP split + drop-tail, micro-batch schedule coverage, conditioning assembly equivalence, ...), none of which were guarded by tests. A regression would either silently change the gradient scale or make DP ranks run a different number of FSDP collectives and deadlock.This PR adds tests for those invariants, plus a small enabling refactor. No behavior change, and CI stays CPU-only.
1. Refactor (commit 1)
Move
build_microbatch_scheduleandvalidate_same_microbatch_counts_across_dpout ofactor.py(which imports ray / diffusers / sglang at module load) intomiles/utils/train_data_utils.py(torch + types only), so they can be unit-tested on a CPU runner without the heavy import chain. Also factor the DP count comparison into a puremicrobatch_counts_agreehelper, decoupled from theall_gather_objectcollective so it is testable without a process group. Functions are moved verbatim;actor.pyimports the same names.2. CPU tests (commit 2) —
tests/fast/utils/test_train_data_utils.pyregister_cpu_ci(suite="stage-a-cpu", labels=[])-> runs on every PR. 23 tests:num_pairs < dp_size/dp_size <= 0.3. GPU tests (commit 3) —
tests/e2e/test_train_pair_cond_equivalence.pyregister_cuda_ci— not wired into any PR stage (no GPU runner; CI stays CPU-only). Run locally:pytest tests/e2e/test_train_pair_cond_equivalence.py. 10 tests on a tiny random-weightQwenImageTransformer2DModel:encoder_hidden_statesfor a same-sample micro-batch (exact,torch.equal); collate pads + masks variable lengths.cfg_combineformula including the true-cfg rescale branch.micro_batch_size(tolerance)._forward_train_pair_batch: the accumulated gradient is invariant tomicro_batch_size(tolerance).torch.equalis used where the comparison keeps batch size + inputs constant;allcloseonly where it spans different batch sizes (on GPU, cuBLAS picks different kernels/reduction orders per shape, so cross-batch results differ at fp rounding — exact equality is unattainable there).Local test results
run_suite.py --hw cpu --suite stage-a-cpudiscovers the file.CI impact
No changes to
.github/workflows— CI remains CPU-only. The CPU tests auto-run on every PR viastage-a-cpu; the GPU tests areregister_cuda_ciand run manually until a GPU runner exists.🤖 Generated with Claude Code