[diffusion, fsdp, cfg] feat: stage Qwen-Image training inputs by timestep - #549
Conversation
…step Release unused PPO/NFT training outputs after backward and add opt-in synchronous CPU timestep staging for Qwen-Image FSDP training. Keep shared conditions at micro-batch scope and copy only matching current step inputs while preserving loss ordering and gradient accumulation. Cover ownership, failure recovery, configuration wiring, and the real Qwen adapter/scheduler contracts. Document the bounded supported scope and memory versus transfer-cost tradeoff. Signed-off-by: Wenzhe Wang <w_wenzhe@163.com>
Preserve the upstream merged-LoRA synchronization and DAPO performance reference alongside timestep staging. Resolve only the two overlapping insertion points; keep existing behavior and measured evidence unchanged. Signed-off-by: Wenzhe Wang <w_wenzhe@163.com>
Preserve upstream condition-row restoration in PPO and NFT while keeping timestep staging, output lifetime, and existing performance evidence intact. Resolve the performance-reference date conflict without changing results. Validated tree16003ef7 with 167 CPU tests, 12 pre-commit hooks, and all four generated trainer configurations. Independent merge review passed. Signed-off-by: Wenzhe Wang <w_wenzhe@163.com>
|
@cr-gao @zhtmike @wtomin The merge conflict is resolved in The exact committed tree passed 167 CPU tests, all 12 pre-commit hooks, and regeneration checks for all four trainer configurations (CPU job Could you please review the updated PR? The existing review requests are retained. |
|
Ping @knlnguyen1802 for this |
knlnguyen1802
left a comment
There was a problem hiding this comment.
Thank you for the optimization, left some minor comment
Signed-off-by: Wenzhe Wang <w_wenzhe@163.com>
|
@WenzheWang Please check CI |
Register the example README through the required documentation symlink and toctree entry. Keep cross-page links valid from both source and documentation paths. Signed-off-by: Wenzhe Wang <w_wenzhe@163.com>
|
@knlnguyen1802 Fixed in The exact committed tree passed the full 19-README check, all six source/docs link checks and all 12 pre-commit hooks. This is documentation-only; the training code and existing performance evidence are unchanged. I restored |
What does this PR do?
Reduce retained diffusion-training tensors in two bounded steps:
model_outputafter backward by default; explicitreturn_model_output=Trueand inference keep their outputs.PPO stages the current/next latent pair and matching loss inputs. NFT shares the clean latent/static noise and stages per-step noise when supplied. Dtypes, ordering, gradient accumulation and loss implementations are unchanged. Central trainer config validation rejects staging with sequence parallelism; the validated model/backend scope is documented in the Qwen-Image README rather than enforced through a constructor allowlist. DPO, other models/backends, sequence parallelism, async prefetch/pinned buffers and rollout transport are out of scope.
Checklist Before Starting
[{modules}] feat: ....API and Usage Example
Consumed tensor inputs must already be CPU tensors without input gradients; there is no implicit full-trajectory GPU-to-CPU round trip. Inference bypasses staging. Explicitly retaining training outputs still incurs timestep-dependent output memory. Transfers are synchronous, so this is a memory/transfer-cost tradeoff, not a throughput guarantee.
Test
Documentation CI fix (2026-09-10)
Fixed in
7de0250099d87d7023a993bc6d22d4f57abb5e3e, after the main refreshb134e233(main6dc0fc03). This is documentation-only: register the Qwen-Image README as a Git symlink underdocs/examples, add its Examples toctree entry, and keep its cross-page links valid from both source and documentation paths.example_docs_linksfailures onb134e233.python3 tests/special_sanity/check_example_docs_symlinks.py: all 19 README pages passed on final tree641bea6ddfb6adb58a710cf2497cfaf88e5822df.pre-commit run --all-fileshooks passed.349568completed0:0in 45s; independent review found no actionable issues. No training code or historical performance evidence changed.The previous GitHub CPU, docs, pre-commit and other successful checks were on
b134e233; new-head CI is triggered separately. Its GPU smoke was cancelled, not a test failure.Reviewer follow-up (2026-09-10)
Updated in
870a4034f018f1228f7d7f747042741a768514d1, on the maintainer's main mergec4eb4117(main4e89d4c9). Exact committed and tested tree:5766bf971efa70e40380f1ff40649f370e148089.FSDPEngineConfig. The worker forwards the option through per-request batch metadata.validate_config, exercised through both diffusion entrypoints. Removed the model/algorithm/backend/device allowlist from the shared constructor and documented the validated scope in the Qwen-Image README.pre-commit run --all-fileshooks passed;bash scripts/generate_trainer_config.shverified all 4 configs with no residual diff. Independent review found no actionable issues.349191completed0:0in 2m16s, using 3 CPUs / 4 GiB in the reused A800 CPU-partition runtime. This remains separate from official CI and does not claim a new GPU benchmark.The historical sections and GPU harness below are preserved unchanged. Their old config subclass/API reproduces the original
678d855drevision; use the actor-level override above for the current head. Original measurements have not been replaced or relabeled.Main refresh (2026-09-10)
Merged
mainatbcf81ab26e8da631aa5b06fcdcc29c209e9422a7inc709bc7572a10af7a12a0c8d53a9a85781663b56. The committed tree (16003ef71e091e40a37875ce0c5311c7b9dd7ea4) is exactly the validated tree. The performance-reference date conflict is resolved; upstream PPO/NFT condition-row restoration and all existing performance evidence are preserved.pre-commit run --all-files.bash scripts/generate_trainer_config.shregenerated and verified all four configurations without a diff.349096completed0:0in 2m22s, using 3 CPUs and 4 GiB on the A800 cluster's CPU partition. This reused validation runtime is not official CI parity.The original GPU measurements below remain tied to
678d855d/ tree8b4000b3; they were not rerun or replaced by this main refresh.Main refresh (2026-09-08)
Merged main
23a5fc4f26a525dc559c968cf9807be00ae9eb27into this branch asba66c044cbca5eb9314bc184fb3de34c6d4b49c9; exact tested tree8b32b8117690c830829faef112478d61f3e54118. Both the upstream merged-LoRA synchronization (#553) and the DAPO performance reference (#456) are retained alongside this PR's original timestep-staging/output-lifetime changes.On the refreshed tree, 163 CPU tests passed, all 12 literal
pre-commit run --all-fileshooks passed, and all 4 trainer configs regenerated with no residual diff. CPU-only job completed0:0in 3m04s. The focused pytest command below was rerun withtests/workers/test_diffusers_fsdp_merged_lora_on_cpu.pyadditionally included. AST comparisons verify unchanged timestep/output-loss methods against the original PR and unchanged merged-LoRA methods against main; both documentation/evidence sections are preserved verbatim. Independent review: SAFE.These refreshed CPU checks reused the runtime documented below. The original GPU matrix and measurements remain attached to
678d855d/ tree8b4000b3; they were not rerun or relabeled as measurements of this merge. No new algorithm, runtime, or feature scope is introduced. Official CI remains separate from these standalone checks.Original implementation and GPU measurements (unchanged)
Base:
03655dc422b0437371dd856de00f3fd9fce91af9; exact declared verl pin:fefb080262e1c015a0ea05f958822a6a512dc795.Validated candidate tree:
8b4000b353db491c7ed40351b33b9414ecb689bb(local tree independently matched).Commit
678d855d8af837393a3cb836ccc1529e98027709was created after validation and has exactly this tree.156 passed, all 12 literal all-file pre-commit hooks passed, all 4 flattened configs generated with no skip or residual diff. Coverage includes weakref ownership through backward, repeated updates, failure recovery, default/retention/inference behavior, row-distinct NFT timesteps and all three noise layouts, optional PPO loss inputs, invalid scope/input rejection, and documented top-level Hydra override propagation. A focused CPU test exercises real engine
forward_step, registered Qwen adapters and PPO scheduler with a tiny linear projection; it is not transformer/FSDP hardware evidence.GPU: all four profiles passed on 2 x NVIDIA A800 80 GB. Real Qwen transformer, adapters, scheduler, diffusion losses, FSDP/FSDP2 backward and AdamW updates. The immutable randomly initialized fixture has two layers, two heads, head dimension 32; native attention, BF16 model, SP=1, per-rank batch 4 / micro-batch 2, 1024x64 latent tokens (512x512 equivalent), positive/negative prompts and True-CFG 2.0. FP32 LoRA except explicitly BF16 for FSDP2/NFT.
Three arms (actual main, A-only output release, A+B input staging), four backend/algorithm profiles, and 1/8/32 timesteps: 36 arm-cases; 108 measured distributed optimizer updates plus 36 warmups. Both ranks passed all comparisons: 216 measured rank observations and 18 completion markers, not 216 independent replicates. Each arm consumes the same main inference outputs/old_log_probs; all inference tensors, training losses, local gradients and updated parameters compare at
rtol=1e-5, atol=1e-6. Updates are finite/nonzero. State/optimizer/RNG are reset between paired observations. The exact-tree job completed0:0in 28m13s.At 32 timesteps, values are descriptive medians of three repetitions, taking the maximum rank for each repetition. Starting allocated bytes match across arms within every profile/length.
The observed step-input CUDA storage is 16.50 MiB -> 1.00 MiB from A to A+B (about 93.9% lower) and stays near 1.00 MiB at 1/8/32 timesteps in this fixed-shape grid. B's incremental peak-allocated reduction is 15.50 MiB here. NFT uses supplied per-step noise in this GPU grid; that input-saving number does not apply to NFT runs that already generate noise on demand.
Tradeoff: FSDP2/NFT update time increases about 11.7% relative to A. Other timings also vary; arms ran in fixed order, so apparent speedups are not a controlled production-throughput claim. This small random model establishes the input-residency mechanism and numerical integration, not large-model peak-memory savings. The default-off flag lets users make a workload-specific memory/transfer-cost choice.
All measured-case aggregates (36 rows; three repeated max-rank observations per row)
The reused Linux validation runtime has PyTorch
2.11.0+cu130, diffusers0.38.0and older vLLM-Omni compatibility dependencies; it is not full current official-CI dependency parity (current diffusers requirement is >=0.40). These checks do not validate rollout transport, reward-training convergence, pretrained numerical alignment, SP>1, NPU or other architectures. The existing FSDP2/NFT FP32-LoRA backward dtype failure was reproduced on unmodified main in this runtime; the declared FSDP2/NFT profile uses BF16 LoRA. This contribution does not fix or claim the FP32 profile.Design & Code Changes
The diffusion-owned FSDP config subclass exposes the flag through the existing actor engine config. The shared PPO/NFT loop validates an explicit field map before training, selects CPU slices before device transfer, rebases all temporal fields together to local step zero, and releases step-owned inputs after backward. Shared inputs live only for the current micro-batch; no persistent cache or new adapter abstraction is added.
Checklist Before Submitting
*_on_cpu.pyregression tests discovered by the existing CPU workflow. GPU integration is manual; no CI workflow or runner policy changed.AI assistance was used. The human submitter has reviewed every changed line and authorized publication.
Complete standalone GPU reproducer and A-only control patch
Save the Python script as
gpu_validate.pyand the diff asa-control.patchoutside the repository. Use a compatible Linux CUDA environment with the exact verl pin and versions above; this does not install dependencies or download model weights. From this PR checkout:A-only production control (documentation/tests do not affect this GPU harness):
Harness SHA256:
acc900870c96dd97b743cb5cbbf67a9e3196f3ceca31aa40f73674d0d388b063.