Skip to content

feat(qwen-image): bitwise train<->rollout parity via the qwen_image rollout patch group - #108

Merged
Rockdu merged 9 commits into
mainfrom
feat/qwen-image-bitwise-parity
Aug 18, 2026
Merged

feat(qwen-image): bitwise train<->rollout parity via the qwen_image rollout patch group#108
Rockdu merged 9 commits into
mainfrom
feat/qwen-image-bitwise-parity

Conversation

@zhihengy

@zhihengy zhihengy commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

With the conditions below, model_output_max_abs_diff / model_output_mean_abs_diff between the training forward and the rollout snapshot are exactly 0.0 at every optimizer step, verified across 3 rollouts including LoRA IPC weight syncs. scripts/run_diffusion_grpo_qwen_image_max_alignment_4gpu.py reproduces 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.
  • The text attention mask must be None: collate collapses an all-true mask to None so both sides hit the same mask-less SDPA flash kernel; a non-trivial mask changes the kernel dispatch.
  • No legacy whole-window cond pad: the maybe_legacy_window_pad_len override is removed; batch-local padding reproduces the rollout request widths.
  • The train micro-batch must reproduce a rollout microgroup exactly: micro_batch_size_sample == rollout_microgroup_size, micro_batch_size_tstep == 1, and --train-dp-split-mode contiguous (the default; stride round-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.
  • Rollout 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 in monkey_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 bf16 c + a * (k + b) instead of the fused fp32 kernel.
  • qk-norm + RoPE: replace the fused CUDA kernel with the patched norms plus diffusers' complex-multiplication RoPE.
  • split_seqs: contiguize the joint-attention output slices before to_out/to_add_out, so batch>1 out-projections run the same flattened GEMM as diffusers.
  • LoRA: keep IPC-synced adapters unmerged and evaluate them with PEFT's exact arithmetic — see below.

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 *WithLoRA layer). The patch group still overrides two things, for different reasons:

  • set_lora_weights forced to merge_weights=False: the IPC weight-sync path miles uses (post_training/weights_updater.py) hardcodes merge_weights=True and never consults lora_merge_mode, so without the override every sync merges W + 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 respect lora_merge_mode), after which it can be dropped — miles already forwards any --sglang-lora-merge-mode flag to ServerArgs.
  • LoRA forward rewrites: sglang's native unmerged forwards are not bitwise-PEFT-equal even at TP=1 — BaseLayerWithLoRA/LinearWithLoRA are @torch.compiled (inductor fusion changes bf16 rounding) and RowParallelLinearWithLoRA computes (Wx + delta) + bias where 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 with gather_output=True), which would silently drop the other ranks' delta shares under TP>1 (with the default targets, to_out.0 is RowParallel). The forward patches are therefore gated per layer: at base_layer.tp_size > 1 they 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

  • Collapse an all-true encoder_hidden_states_mask to None and 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.
  • The sgld patch group is dropped (it only served Qwen-Image); --rollout-patch-group now offers qwen_image and ltx, one self-contained group per model. The aligned pickscore recipe switches to qwen_image accordingly and is otherwise unchanged.

@zhihengy
zhihengy force-pushed the feat/qwen-image-bitwise-parity branch 3 times, most recently from 221e7f6 to a328471 Compare August 11, 2026 00:49
@zhihengy zhihengy changed the title feat(qwen-image): bitwise train<->rollout parity via autocast-parity norms and a pinned flash kernel feat(qwen-image): bitwise train<->rollout parity via the qwen_image rollout patch group Aug 11, 2026
@zhihengy
zhihengy force-pushed the feat/qwen-image-bitwise-parity branch 2 times, most recently from 298fa9f to 47fd1c1 Compare August 11, 2026 02:39
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 Rockdu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, may refactor LoRA patch later

@Rockdu

Rockdu commented Aug 16, 2026

Copy link
Copy Markdown
Collaborator

Let me add CI for qwen-image as well so we get everything e2e-verified

@zhihengy
zhihengy force-pushed the feat/qwen-image-bitwise-parity branch from c021559 to 1be4993 Compare August 17, 2026 22:34
@Rockdu Rockdu added the run-ci-e2e Run e2e metric-regression tests on this PR label Aug 17, 2026
@zhihengy
zhihengy force-pushed the feat/qwen-image-bitwise-parity branch from 63e98db to 1fa179e Compare August 18, 2026 02:57
@zhihengy
zhihengy force-pushed the feat/qwen-image-bitwise-parity branch from 1fa179e to 3344cff Compare August 18, 2026 02:59

@Rockdu Rockdu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

zhihengy and others added 2 commits August 18, 2026 09:20
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>
@Rockdu
Rockdu merged commit 4830763 into main Aug 18, 2026
34 of 35 checks passed
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-ci-e2e Run e2e metric-regression tests on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants