feat(qwen-image): bitwise train<->rollout parity via the qwen_image rollout patch group - #108
Merged
Merged
Conversation
zhihengy
force-pushed
the
feat/qwen-image-bitwise-parity
branch
3 times, most recently
from
August 11, 2026 00:49
221e7f6 to
a328471
Compare
zhihengy
force-pushed
the
feat/qwen-image-bitwise-parity
branch
2 times, most recently
from
August 11, 2026 02:39
298fa9f to
47fd1c1
Compare
zhihengy
added a commit
that referenced
this pull request
Aug 11, 2026
…-IPC path GEMM(W + s·BA) is not bitwise GEMM(W) + GEMM_B(GEMM_A(x))·s, so the default lora_merge weight sync caps train/rollout parity at the first step (lora_B starts at zero). Keep the engine-side adapters unmerged instead, mirroring the qwen_image patch group (#108): - recipes ship adapters with --lora-ipc-weight-sync — fp32 lora_A/lora_B masters through the engine's native LoRA-IPC path; no bespoke transport. - cosmos3_bitwise patches the native LoRA wrappers: set_lora_weights never merges and rounds A/B to the base weight dtype (the FSDP mixed-precision gather rounding the train forward sees); wrapper forwards run eager base(x) + lora_B(lora_A(x))·s in peft's exact op order (the stock forwards are @torch.compile'd, which re-fuses even the no-adapter base path). - fused targets (add_q/k/v -> to_qkv) all resolve to one wrapper whose set_lora_weights(clear_existing=True) calls would clobber each other; _resolve_lora_ipc_layer_dict_key is patched to route each prefix to its merge slot, and the delta lands on the matching output slice. - adapt the CFG-sequential patch to the omni-era _run_transformer kwargs and guard the new fused qknorm+rope kernel — current sglang main moved both. - fix the 5gpu recipe's stale --diffusion-init-lora-weight flag (renamed to --lora-init-weights on main). Verified on Cosmos3-Nano GRPO (3 train GPUs + pickscore): LoRA IPC sync resolves all 144 layer prefixes (unmapped=0) and train/model_output_{mean,max}_abs_diff stay 0.0 across steps 1-3, i.e. across two LoRA weight updates. Requires engine-side Cosmos3Pipeline LoRA support (the LoRAPipeline mixin), added to sgl-project/sglang#34197. Supersedes the bespoke unmerged-sync transport from #129. Co-authored-by: Cursor <cursoragent@cursor.com>
zhihengy
added a commit
that referenced
this pull request
Aug 12, 2026
…-IPC path GEMM(W + s·BA) is not bitwise GEMM(W) + GEMM_B(GEMM_A(x))·s, so the default lora_merge weight sync caps train/rollout parity at the first step (lora_B starts at zero). Keep the engine-side adapters unmerged instead, mirroring the qwen_image patch group (#108): - recipes ship adapters with --lora-ipc-weight-sync — fp32 lora_A/lora_B masters through the engine's native LoRA-IPC path; no bespoke transport. - cosmos3_bitwise patches the native LoRA wrappers: set_lora_weights never merges and rounds A/B to the base weight dtype (the FSDP mixed-precision gather rounding the train forward sees); wrapper forwards run eager base(x) + lora_B(lora_A(x))·s in peft's exact op order (the stock forwards are @torch.compile'd, which re-fuses even the no-adapter base path). - fused targets (add_q/k/v -> to_qkv) all resolve to one wrapper whose set_lora_weights(clear_existing=True) calls would clobber each other; _resolve_lora_ipc_layer_dict_key is patched to route each prefix to its merge slot, and the delta lands on the matching output slice. - adapt the CFG-sequential patch to the omni-era _run_transformer kwargs and guard the new fused qknorm+rope kernel — current sglang main moved both. - fix the 5gpu recipe's stale --diffusion-init-lora-weight flag (renamed to --lora-init-weights on main). Verified on Cosmos3-Nano GRPO (3 train GPUs + pickscore): LoRA IPC sync resolves all 144 layer prefixes (unmapped=0) and train/model_output_{mean,max}_abs_diff stay 0.0 across steps 1-3, i.e. across two LoRA weight updates. Requires engine-side Cosmos3Pipeline LoRA support (the LoRAPipeline mixin), added to sgl-project/sglang#34197. Supersedes the bespoke unmerged-sync transport from #129. Co-authored-by: Cursor <cursoragent@cursor.com>
zhihengy
added a commit
that referenced
this pull request
Aug 12, 2026
…-IPC path GEMM(W + s·BA) is not bitwise GEMM(W) + GEMM_B(GEMM_A(x))·s, so the default lora_merge weight sync caps train/rollout parity at the first step (lora_B starts at zero). Keep the engine-side adapters unmerged instead, mirroring the qwen_image patch group (#108): - recipes ship adapters with --lora-ipc-weight-sync — fp32 lora_A/lora_B masters through the engine's native LoRA-IPC path; no bespoke transport. - cosmos3_bitwise patches the native LoRA wrappers: set_lora_weights never merges and rounds A/B to the base weight dtype (the FSDP mixed-precision gather rounding the train forward sees); wrapper forwards run eager base(x) + lora_B(lora_A(x))·s in peft's exact op order (the stock forwards are @torch.compile'd, which re-fuses even the no-adapter base path). - fused targets (add_q/k/v -> to_qkv) all resolve to one wrapper whose set_lora_weights(clear_existing=True) calls would clobber each other; _resolve_lora_ipc_layer_dict_key is patched to route each prefix to its merge slot, and the delta lands on the matching output slice. - adapt the CFG-sequential patch to the omni-era _run_transformer kwargs and guard the new fused qknorm+rope kernel — current sglang main moved both. - fix the 5gpu recipe's stale --diffusion-init-lora-weight flag (renamed to --lora-init-weights on main). Verified on Cosmos3-Nano GRPO (3 train GPUs + pickscore): LoRA IPC sync resolves all 144 layer prefixes (unmapped=0) and train/model_output_{mean,max}_abs_diff stay 0.0 across steps 1-3, i.e. across two LoRA weight updates. Requires engine-side Cosmos3Pipeline LoRA support (the LoRAPipeline mixin), added to sgl-project/sglang#34197. Supersedes the bespoke unmerged-sync transport from #129. Co-authored-by: Cursor <cursoragent@cursor.com>
Rockdu
approved these changes
Aug 16, 2026
Rockdu
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, may refactor LoRA patch later
Collaborator
|
Let me add CI for qwen-image as well so we get everything e2e-verified |
zhihengy
force-pushed
the
feat/qwen-image-bitwise-parity
branch
from
August 17, 2026 22:34
c021559 to
1be4993
Compare
zhihengy
force-pushed
the
feat/qwen-image-bitwise-parity
branch
from
August 18, 2026 02:57
63e98db to
1fa179e
Compare
…ollout patch group
Co-authored-by: Cursor <cursoragent@cursor.com>
zhihengy
force-pushed
the
feat/qwen-image-bitwise-parity
branch
from
August 18, 2026 02:59
1fa179e to
3344cff
Compare
Registers the flowgrpo-aligned recipe in stage-c-5-gpu-h200 (opt-in via run-ci-e2e, nightly). --diffusion-debug-mode joins the recipe so the model_output parity metrics emit; with the qwen_image patch group + dynamic LoRA merge they must be exactly 0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Recorded on the 5gpu runner (run 32121222016, sglang main 70ee6b17). Bitwise train<->rollout equality is not yet expected for Qwen-Image, so the standard pins the current residual instead of asserting zero. est_time from the recorded run: ~16 min. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
zhihengy
added a commit
that referenced
this pull request
Aug 18, 2026
…e-align
Conflicts: the LoRA merge-mode warning in arguments.py (both sides added
it for their own patch group — unified over {qwen_image, cosmos3_bitwise});
the registered-groups test (sgld was renamed qwen_image by #108).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
With the conditions below,
model_output_max_abs_diff/model_output_mean_abs_diffbetween the training forward and the rollout snapshot are exactly0.0at every optimizer step, verified across 3 rollouts including LoRA IPC weight syncs.scripts/run_diffusion_grpo_qwen_image_max_alignment_4gpu.pyreproduces this end to end on 4 GPUs (train + rollout + colocated pickscore reward, debug mode on).Necessary conditions for bitwise equality
--num-steps-per-rollout 1: only on-policy steps can match; later steps train on updated weights.None: collate collapses an all-true mask toNoneso both sides hit the same mask-less SDPA flash kernel; a non-trivial mask changes the kernel dispatch.maybe_legacy_window_pad_lenoverride is removed; batch-local padding reproduces the rollout request widths.micro_batch_size_sample == rollout_microgroup_size,micro_batch_size_tstep == 1, and--train-dp-split-mode contiguous(the default;strideround-robins pairs across ranks, mixing prompt lengths into one micro-batch, which makes the mask non-trivial and changes cuBLAS kernel selection).--fsdp-attention-backend _native_flash: pins the train forward to the same SDPA flash kernel the rollout executes.tp_size == 1: under TP the sharded GEMMs and all-reduce summation order change bf16 rounding, so bitwise parity is a TP=1 property by construction. The patch group stays safe to enable at TP>1 (see the LoRA section), it just cannot be bitwise there.Patches (
--rollout-patch-group qwen_image, all inmonkey_patches/patch_qwen_image.py)RMSNorm: round to weight dtype before the weight mul, as diffusers does.LayerNormScaleShift/ScaleResidualLayerNormScaleShift: fp32 LayerNorm, bf16(1 + scale)rounding, fp32 modulation — the train-side autocast semantics.MulAdd: eager bf16c + a * (k + b)instead of the fused fp32 kernel.split_seqs: contiguize the joint-attention output slices beforeto_out/to_add_out, so batch>1 out-projections run the same flattened GEMM as diffusers.LoRA: why patch instead of sglang's native unmerged mode, and the TP gate
sgl-d does support unmerged LoRA natively (
--lora-merge-mode dynamic, with TP-aware unmerged forwards on every*WithLoRAlayer). The patch group still overrides two things, for different reasons:set_lora_weightsforced tomerge_weights=False: the IPC weight-sync path miles uses (post_training/weights_updater.py) hardcodesmerge_weights=Trueand never consultslora_merge_mode, so without the override every sync mergesW + s·(B@A)in bf16, which rounds differently from PEFT's unmerged path. This override is a stopgap: the clean fix is upstream (make the sgl-d updater respectlora_merge_mode), after which it can be dropped — miles already forwards any--sglang-lora-merge-modeflag to ServerArgs.forwardrewrites: sglang's native unmerged forwards are not bitwise-PEFT-equal even at TP=1 —BaseLayerWithLoRA/LinearWithLoRAare@torch.compiled (inductor fusion changes bf16 rounding) andRowParallelLinearWithLoRAcomputes(Wx + delta) + biaswhere PEFT computes(Wx + bias) + delta. The patched forwards run PEFT's exact ordering: base output with bias first, then the scaled delta.The PEFT-ordered forward adds the rank-local delta after
base_layer()has already all-reduced (RowParallel) or all-gathered (ColumnParallel withgather_output=True), which would silently drop the other ranks' delta shares under TP>1 (with the default targets,to_out.0is RowParallel). The forward patches are therefore gated per layer: atbase_layer.tp_size > 1they dispatch back to sglang's native TP-aware unmerged forwards, which add the delta before the collective. TP>1 rollouts with the patch group + LoRA are numerically correct, just not bitwise-equal to the training forward (nothing can be, per the conditions above).Other changes
encoder_hidden_states_masktoNoneand drop the legacy whole-window pad override in the Qwen-Image train pipeline config.clamp_min(1e-12)on the CFG rescale norm, mirroring the engine.sgldpatch group is dropped (it only served Qwen-Image);--rollout-patch-groupnow offersqwen_imageandltx, one self-contained group per model. The aligned pickscore recipe switches toqwen_imageaccordingly and is otherwise unchanged.