Refactor train-data-pair-dispatch - #10
Merged
Merged
Conversation
Collaborator
Author
|
/run-lint |
1 similar comment
Collaborator
Author
|
/run-lint |
Rockdu
force-pushed
the
refactor/train-data-pair-dispatch
branch
from
June 23, 2026 08:07
6e88f7f to
cb62e35
Compare
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.
Rockdu
force-pushed
the
refactor/train-data-pair-dispatch
branch
from
June 23, 2026 08:11
cb62e35 to
c7c2de4
Compare
train_data_utils: TrainDataDPSplitter gains a baseline_stride DP-split mode that reproduces the legacy range(rank, N, dp) sample-stride dispatch, so the refactored rollout-side grouping can feed each DP rank the exact same sample set/order as the old TrainRayActor path. qwen_image: collate_cond_for_sample_batch gains optional pad_to_len for legacy window-wide cond padding parity. CPU CI (no GPU, no model forward): test_grouping_parity (DP-split/converter/cond-pad), test_scaler_bf16_passthrough, and test_legacy_tile_grouping_golden -- the last replays REAL legacy tiles captured by executing origin/main _run_optim_window verbatim (fixture + generator included) and asserts the refactored baseline_stride + build_microbatch_schedule reproduces every tile cell-for-cell.
The flat-pair trainer collates cond per micro-batch, so to keep legacy window-wide padding parity it must pass pad_to_len uniformly to every model config. Promote pad_to_len into the base TrainPipelineConfig.collate_cond_for_sample_batch contract (default None) so concat-based configs do not TypeError when the trainer passes it. SD3 (torch.cat fixed-length embeds) now accepts and ignores pad_to_len; Qwen-Image (variable-length) honors it. Wan2.2/LTX live on separate upstream PR branches and use the same torch.cat pattern as SD3 -- they only need the one-line pad_to_len=None added to their collate signature when rebased onto this; the new test_cond_collate_pad_to_len_interface CPU test pins the contract (base+qwen+sd3) and that pad_to_len is a no-op for concat configs.
wan2_2.py and ltx.py live on separate upstream PR branches (feat/wan, feat/support_ltx) and use the same fixed-length torch.cat collate as SD3. They only need a one-line pad_to_len=None added to collate_cond_for_sample_batch to satisfy the new base contract (behavioural no-op). Ship the ready-to-apply patches + README rather than editing those out-of-tree files. Both verified with git apply --check against their target branch.
The pad_to_len signature tweak for wan2_2.py / ltx.py is a trivial one-liner on their own upstream PR branches; their authors will add it directly, so we do not need to carry the patches in this PR.
…tep_batch) A single-sample (timestep-stacked) micro-batch took a separate expand_cond_for_timestep_batch path that emitted no encoder_hidden_states_mask. Collating bsz copies of the one sample is bitwise-equivalent: qwen collate adds an all-True mask, and tests/manual/check_mask_equivalence.py verifies on GPU that an all-True encoder_hidden_states_mask is a bitwise no-op vs None in the QwenImageTransformer2DModel forward (max_abs_diff=0 under both MATH/NATIVE and default SDPA backends; RoPE text_seq_len is identical either way). For fixed-length concat configs the rows are identical regardless. So the same_sample_microbatch branch and the now-unused expand_cond_for_timestep_batch are removed; a CPU regression test locks the single-sample collate contract.
build_microbatch_schedule and validate_same_microbatch_counts_across_dp were train-data grouping helpers still living in actor.py; co-locate them with the rest of the train-data utils (RolloutTrainDataConverter / TrainDataDPSplitter / scheduler_meta_from_rollout / stack_train_pair_rollout_debug) in miles/utils/train_data_utils.py. actor.py now imports them (behaviour unchanged). The golden test imports build_microbatch_schedule from train_data_utils instead of actor, so it no longer pulls diffusers (CPU test drops from ~12s to sub-second). New tests/test_microbatch_schedule.py gives the pure function dedicated CPU coverage (even split, absolute offsets, partial last micro-batch, oversize micro-batch, contiguous coverage).
…hedule Move the flat tests/test_*.py CPU unit tests under tests/fast/ mirroring the source tree (tests/fast/utils for miles/utils/train_data_utils; tests/fast/backends/fsdp_utils[/configs] for the fsdp config/scaler tests) and register each with register_cpu_ci(suite=stage-a-cpu) so run_suite.py actually runs them on every PR (previously they were not discovered by the CI suite at all). build_microbatch_schedule now raises ValueError when num_pairs_per_optim_step is not a whole multiple of micro_batch_size (no ragged final micro-batch / no micro_batch_size larger than the step); covered by test_microbatch_schedule. Golden test imports build_microbatch_schedule from train_data_utils and resolves its fixture via parents[2] after the move.
… once Under fsdp_cfg_batching the code collated pos_cond_microbatch + neg_cond_microbatch and then ignored them, re-collating collate(pos_list + neg_list) inside _compute_noise_pred on every call (policy + KL reference). Now branch on cfg_batching up front: build the joint pos+neg cond once when batching, else build pos (and neg) separately; _compute_noise_pred consumes the precomputed cond and keeps the forward split (one joint forward vs two separate forwards vs single no-CFG forward). Bitwise-unchanged output; drops the wasted collate+cast and the per-call joint re-collate. (CPU CI can not exercise the DiT forward; verified import/lint + full fast suite.)
Org-batch (the micro-batch schedule) now lives entirely in the rollout manager, finishing the refactor intent. RolloutManager.generate builds each DP shard schedule after the split (build_shard_microbatch_schedule), validates all ranks split into the same micro-batch count centrally (validate_uniform_microbatch_schedule, replacing the per-rank dist.all_gather), and ships schedule[optim_step][micro_batch]=pair-indices with the shard. The train actor no longer builds the schedule or does the cross-DP collective -- it just consumes rollout_data[microbatch_schedule]. Re-add legacy --micro-batch-size-sample/--micro-batch-size-tstep/--diffusion-train-iter-order: when set, the schedule uses the legacy 2D sample x timestep tiling (build_tiled_microbatch_schedule) instead of contiguous 1D --micro-batch-size chunks. This restores configs the flat scheme could not express -- notably SD3.5 (tstep micro-batch=5 != SDE window=10); WAN2.2/LTX (1x1) also work. The tiling is cross-checked cell-for-cell against origin/main _run_optim_window run verbatim over 9 (sample_mb x tstep_mb x iter_order x M x T) configs (tstep<window, ==window, =1, timestep_major, ragged sample/tstep, single-cell, whole-window).
--micro-batch-size-sample / --micro-batch-size-tstep / --diffusion-train-iter-order are kept only for backward compatibility with legacy (pre-refactor) configs and are planned for removal; prefer --micro-batch-size. Help-text only, no behaviour change.
RolloutTrainDataConverter returns one scheduler_meta (timesteps/sigmas) for the whole batch, taken from sample 0. Replace the bare assume-comment with an actual per-sample check in the existing loop (reuses the already-fetched traj, single pass, negligible cost): every sample must carry the same scheduler_timesteps and scheduler_sigmas as sample 0, else raise. Test fixture _mk_sample now uses a shared sigmas seed (matching reality; timesteps were already shared), plus two tests asserting mismatched timesteps/sigmas raise.
Revert the schedule move into RolloutManager (option B). The micro-batch schedule is built per-rank in the train actor again (more decoupled, scales better). Legacy 2D sample x timestep tiling is supported by reorder_train_pairs_for_tiling: the rollout manager reorders each shard so tiles are contiguous, then the actor reproduces them with its plain contiguous schedule (micro_batch_size = sample_mb x tstep_mb, derived once in miles_validate_args). Actor stays pure (knows only micro_batch_size). build_tiled_microbatch_schedule (cross-checked vs real legacy over 9 configs) is reused to compute the reorder permutation; validate_same_microbatch_counts_across_dp restored. Reorder requires uniform tiles. Tests updated.
The Optimizer Windows and Microbatch Synchronization headers both just set up build_microbatch_schedule, so merge them into one Micro-batch schedule section. Replace 组batch with group-batch in test_grouping_parity docstring.
Replace the four repeated torch.stack([batch[i][key]...]).to(device, float32) calls with a local _stack(key) helper.
…shapes timesteps/log_prob_old are stacked scalars and log_prob_new comes out of sde_step_with_logprob already reduced over non-batch dims, so all of them are (bsz,); the .reshape(bsz) and .reshape(-1)[0] calls were no-ops. Removed them and annotated the per-pair microbatch tensor shapes. The two (bsz, -1) reshapes in the abs-diff stats helper are genuine and kept.
Rockdu
force-pushed
the
refactor/train-data-pair-dispatch
branch
from
June 27, 2026 06:07
18f8105 to
51e451e
Compare
… sglang stage-a-cpu installs sglang --no-deps plus a curated list of its pure-python runtime deps. A new config test imports a pipeline config, which via the FSDP backend package init pulls the actor and sglang.srt.utils.common, whose import pybase64 is unguarded. pybase64 was missing from the curated list, so collection failed with ModuleNotFoundError. Add pybase64 (pure-python, CPU-installable); the rest of sglang GPU-only chain is soft-probed and fine when absent. Restores the original fsdp_utils/__init__ (no production change). Also black-format gen_legacy_tile_2d_fixture.py.
Rockdu
force-pushed
the
refactor/train-data-pair-dispatch
branch
from
June 27, 2026 06:54
51e451e to
f351d6c
Compare
c7c2de4 added wandb_utils.log_sample_images but never wired it up: image logging goes through the pre-existing RolloutManager._log_images in rollout.py, which this function duplicates verbatim. It did not exist before the PR and has zero callers. Drop it and its now-unused imports (Namespace, Any, numpy, Sample).
actor.py builds DiffusionUpdateWeightFromTensor[LoRA](args, model), but the c7c2de4 rebase added a required target_module positional to the base/LoRA constructors whose body sets self.target_module = args.update_weight_target_module anyway (the param was dead). The 2-vs-3 arg mismatch raised TypeError in FSDPTrainRayActor.init, so the FSDP actor never initialized and no training step ran. Restore the 2-arg signature, matching the working baseline a48476c.
The legacy parity path was only half wired: when --micro-batch-size-sample/-tstep are set, RolloutManager.generate reordered each shard into sample x timestep tiles but still split the rollout across DP ranks with the default contiguous policy. Legacy TrainRayActor used a strided dispatch (rank r owns samples r, r+dp, ...), so ranks saw different samples per optim step and training metrics diverged from the baseline despite identical rollouts. Split with mode=baseline_stride whenever the legacy knobs are set so the contiguous-schedule actor replays the exact legacy tiles; keep the default contiguous split for the native 1D --micro-batch-size path.
These examples reproduce the flow_grpo baselines, so they should drive the legacy 2D compat path (baseline_stride DP split + tile reorder) rather than the native 1D --micro-batch-size. Use the per-script legacy split from the validated origin/main baseline: ocr-2gpu / ocr-4gpu / pickscore-2gpu use sample=4 tstep=2; pickscore-4gpu uses sample=8 tstep=1. The SD3 script already used legacy params.
The refactor added two debug abs-diff metrics over the legacy actor: prev_sample_mean and noise_std_dev. Both are useless as train/rollout consistency checks: std_dev_t is a deterministic function of the scheduler (model-independent), and prev_sample_mean is a deterministic linear function of model_output (same latent/timestep), so its diff is just model_output_diff scaled by a per-step scalar. noise_std_dev was also comparing mismatched quantities (train returns bare std_dev_t; rollout stores std_dev_t*sqrt(-dt)). Drop both; keep model_output (the only model-dependent check), named to match the legacy actor. The std_dev_t/prev_sample_mean tensors stay -- the KL loss still uses them.
The two legacy-tile goldens cover orthogonal axes; make each docstring lead with that distinction and cross-reference its complement. 1D (gen_legacy_tile_fixture / test_legacy_tile_grouping_golden): the only golden that exercises the DP split (baseline_stride range(rank,N,dp)) + 1D build_microbatch_schedule, one real OCR config. 2D (gen_legacy_tile_2d_fixture / test_tiled_microbatch_schedule): build_tiled_microbatch_schedule across many configs incl. non-degenerate tilings, no DP split.
Rockdu
force-pushed
the
refactor/train-data-pair-dispatch
branch
from
June 27, 2026 15:09
2236779 to
c44c055
Compare
…arity) The legacy tile path collated qwen-image cond once over the whole optimizer window and padded to that max seq_len; multi-sample tiles sliced from it. The flat-pair trainer collated per micro-batch, so it padded to the micro-batch max instead. For variable-length OCR prompts the two widths differ, so the masked bf16 attention (and thus model_output / log_prob_new / clipfrac) diverged from legacy at step 1 -- a real, reproducible difference confirmed by before==before2, after==after2, before!=after. Add a legacy_window_pad_len hook (base returns None; qwen returns the window max seq_len) and thread its result as pad_to_len through the actors
Rockdu
force-pushed
the
refactor/train-data-pair-dispatch
branch
from
June 27, 2026 17:10
458b864 to
94143cb
Compare
Collaborator
Author
Rockdu
added a commit
to Rockdu/miles_diffusion
that referenced
this pull request
Jul 6, 2026
The radixark#10 actor reads self.scheduler.config.num_train_timesteps in _train_core; the LTX holder had no .config, so LTX would AttributeError on the rebased stack (not covered by the pre-radixark#10 B200 runs). σ×1000 convention -> num_train_timesteps=1000. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015JFR2xAGHKE88Yh4LrTbAx
Rockdu
added a commit
to Rockdu/miles_diffusion
that referenced
this pull request
Jul 6, 2026
… removed in the radixark#10 rebase) The radixark#10 rebase dropped the LTX-specific ltx_sde_candidates strategy in favor of main's epoch_global_window (equivalent: draws --diffusion-sde-window-size indices from the candidate set); the run script still referenced the deleted function, so rollout crashed at GenerateState init. Surfaced only by an actual run. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015JFR2xAGHKE88Yh4LrTbAx
Rockdu
added a commit
to Rockdu/miles_diffusion
that referenced
this pull request
Jul 6, 2026
The radixark#10 actor reads self.scheduler.config.num_train_timesteps in _train_core; the LTX holder had no .config, so LTX would AttributeError on the rebased stack (not covered by the pre-radixark#10 B200 runs). σ×1000 convention -> num_train_timesteps=1000. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015JFR2xAGHKE88Yh4LrTbAx (cherry picked from commit b1cc5d1)
Rockdu
added a commit
to Rockdu/miles_diffusion
that referenced
this pull request
Jul 6, 2026
… removed in the radixark#10 rebase) The radixark#10 rebase dropped the LTX-specific ltx_sde_candidates strategy in favor of main's epoch_global_window (equivalent: draws --diffusion-sde-window-size indices from the candidate set); the run script still referenced the deleted function, so rollout crashed at GenerateState init. Surfaced only by an actual run. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015JFR2xAGHKE88Yh4LrTbAx (cherry picked from commit 032ac16)
Rockdu
added a commit
to Rockdu/miles_diffusion
that referenced
this pull request
Jul 6, 2026
The radixark#10 actor reads self.scheduler.config.num_train_timesteps in _train_core; the LTX holder had no .config, so LTX would AttributeError on the rebased stack (not covered by the pre-radixark#10 B200 runs). σ×1000 convention -> num_train_timesteps=1000. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015JFR2xAGHKE88Yh4LrTbAx (cherry picked from commit b1cc5d1) (cherry picked from commit 35a2203)
Rockdu
added a commit
to Rockdu/miles_diffusion
that referenced
this pull request
Jul 6, 2026
… removed in the radixark#10 rebase) The radixark#10 rebase dropped the LTX-specific ltx_sde_candidates strategy in favor of main's epoch_global_window (equivalent: draws --diffusion-sde-window-size indices from the candidate set); the run script still referenced the deleted function, so rollout crashed at GenerateState init. Surfaced only by an actual run. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015JFR2xAGHKE88Yh4LrTbAx (cherry picked from commit 032ac16) (cherry picked from commit 3550fc3)
Rockdu
added a commit
to Rockdu/miles_diffusion
that referenced
this pull request
Jul 6, 2026
The radixark#10 actor reads self.scheduler.config.num_train_timesteps in _train_core; the LTX holder had no .config, so LTX would AttributeError on the rebased stack (not covered by the pre-radixark#10 B200 runs). σ×1000 convention -> num_train_timesteps=1000. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015JFR2xAGHKE88Yh4LrTbAx (cherry picked from commit b1cc5d1) (cherry picked from commit 35a2203) (cherry picked from commit 262bd8f)
Rockdu
added a commit
to Rockdu/miles_diffusion
that referenced
this pull request
Jul 6, 2026
… removed in the radixark#10 rebase) The radixark#10 rebase dropped the LTX-specific ltx_sde_candidates strategy in favor of main's epoch_global_window (equivalent: draws --diffusion-sde-window-size indices from the candidate set); the run script still referenced the deleted function, so rollout crashed at GenerateState init. Surfaced only by an actual run. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015JFR2xAGHKE88Yh4LrTbAx (cherry picked from commit 032ac16) (cherry picked from commit 3550fc3) (cherry picked from commit de7d7fd)
Rockdu
added a commit
to Rockdu/miles_diffusion
that referenced
this pull request
Jul 6, 2026
The radixark#10 actor reads self.scheduler.config.num_train_timesteps in _train_core; the LTX holder had no .config, so LTX would AttributeError on the rebased stack (not covered by the pre-radixark#10 B200 runs). σ×1000 convention -> num_train_timesteps=1000. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015JFR2xAGHKE88Yh4LrTbAx (cherry picked from commit b1cc5d1) (cherry picked from commit 35a2203) (cherry picked from commit 262bd8f) (cherry picked from commit 7b0f8e9)
Rockdu
added a commit
to Rockdu/miles_diffusion
that referenced
this pull request
Jul 6, 2026
… removed in the radixark#10 rebase) The radixark#10 rebase dropped the LTX-specific ltx_sde_candidates strategy in favor of main's epoch_global_window (equivalent: draws --diffusion-sde-window-size indices from the candidate set); the run script still referenced the deleted function, so rollout crashed at GenerateState init. Surfaced only by an actual run. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015JFR2xAGHKE88Yh4LrTbAx (cherry picked from commit 032ac16) (cherry picked from commit 3550fc3) (cherry picked from commit de7d7fd) (cherry picked from commit 988e18a)
Rockdu
added a commit
to Rockdu/miles_diffusion
that referenced
this pull request
Jul 7, 2026
The radixark#10 actor reads self.scheduler.config.num_train_timesteps in _train_core; the LTX holder had no .config, so LTX would AttributeError on the rebased stack (not covered by the pre-radixark#10 B200 runs). σ×1000 convention -> num_train_timesteps=1000. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015JFR2xAGHKE88Yh4LrTbAx (cherry picked from commit b1cc5d1) (cherry picked from commit 35a2203) (cherry picked from commit 262bd8f) (cherry picked from commit 7b0f8e9)
Rockdu
added a commit
to Rockdu/miles_diffusion
that referenced
this pull request
Jul 7, 2026
… removed in the radixark#10 rebase) The radixark#10 rebase dropped the LTX-specific ltx_sde_candidates strategy in favor of main's epoch_global_window (equivalent: draws --diffusion-sde-window-size indices from the candidate set); the run script still referenced the deleted function, so rollout crashed at GenerateState init. Surfaced only by an actual run. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015JFR2xAGHKE88Yh4LrTbAx (cherry picked from commit 032ac16) (cherry picked from commit 3550fc3) (cherry picked from commit de7d7fd) (cherry picked from commit 988e18a)
Rockdu
added a commit
to Rockdu/miles_diffusion
that referenced
this pull request
Jul 7, 2026
The radixark#10 actor reads self.scheduler.config.num_train_timesteps in _train_core; the LTX holder had no .config, so LTX would AttributeError on the rebased stack (not covered by the pre-radixark#10 B200 runs). σ×1000 convention -> num_train_timesteps=1000. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015JFR2xAGHKE88Yh4LrTbAx (cherry picked from commit b1cc5d1) (cherry picked from commit 35a2203) (cherry picked from commit 262bd8f) (cherry picked from commit 7b0f8e9)
Rockdu
added a commit
to Rockdu/miles_diffusion
that referenced
this pull request
Jul 7, 2026
… removed in the radixark#10 rebase) The radixark#10 rebase dropped the LTX-specific ltx_sde_candidates strategy in favor of main's epoch_global_window (equivalent: draws --diffusion-sde-window-size indices from the candidate set); the run script still referenced the deleted function, so rollout crashed at GenerateState init. Surfaced only by an actual run. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015JFR2xAGHKE88Yh4LrTbAx (cherry picked from commit 032ac16) (cherry picked from commit 3550fc3) (cherry picked from commit de7d7fd) (cherry picked from commit 988e18a)
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.

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.