[cosmos3] Bitwise parity across LoRA updates: unmerged weight sync + engine-side adapter replay - #129
Closed
zhihengy wants to merge 1 commit into
Closed
Conversation
…via unmerged weight sync Merged LoRA sync caps parity at step 1: the engine's GEMM(W + sBA) rounds differently from peft's base(x) + lora_B(lora_A(x))*s, so the first optimizer step raised model_output_mean_abs_diff to ~0.04 (36 layers + CFG amplify a per-layer bf16 rounding gap). Trainer (--lora-unmerged-weight-sync, opt-in): ship base weights untouched plus per-layer lora_A/lora_B (pre-rounded to the forward dtype, matching the bits FSDP's mixed-precision gather feeds the train forward) and an fp64 scaling. Engine (cosmos3_bitwise patch group): intercept the adapter tensors at the weight-sync loader — buffering parts across per-dtype bucket calls — resolve targets through the model's own param-name mapping (add_q/k/v -> to_qkv slices 0/1/2, to_add_out -> to_out), and replay peft's exact op sequence after each base GEMM. Verified: standalone GEMM probe bitwise-equal on all cosmos3 LoRA shapes; 130-step Cosmos3-Nano GRPO run holds model_output diff at exactly 0.0 on every step (previously 0.0 only at step 1). Co-authored-by: Cursor <cursoragent@cursor.com>
zhihengy
force-pushed
the
feat/cosmos3-lora-unmerged-sync
branch
from
August 10, 2026 03:52
85a1630 to
a2d07f0
Compare
zhihengy
force-pushed
the
feat/cosmos3-bitwise-align
branch
from
August 10, 2026 03:52
a326ddb to
f9d5371
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>
Collaborator
Author
|
Superseded by the rework now in #109 (commit 6d60e3d), which follows the #108 (qwen_image) patch style instead of a bespoke transport:
Engine prerequisite (Cosmos3Pipeline + LoRAPipeline mixin) landed on sgl-project/sglang#34197. Verified on Cosmos3-Nano GRPO: all 144 LoRA layer prefixes resolve (unmapped=0) and |
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>
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.
Stacked on #109. Extends bitwise train/rollout parity from "before the first
optimizer step" to "after every weight update".
Problem
With the default merged LoRA sync, the rollout engine runs
GEMM(W + sBA)(one GEMM) while the trainer's peft forward runs
base(x) + lora_B(lora_A(x))*s(three GEMMs). The two are mathematicallyequal but round differently in bf16, so
model_output_mean_abs_diffjumpsfrom 0.0 to ~0.04 at step 2 (per-layer rounding gap amplified by 36 layers +
CFG) and stays there. A standalone probe confirms the mismatch on all cosmos3
LoRA shapes and confirms base+adapter replay is bitwise-exact.
Fix
Trainer —
--lora-unmerged-weight-sync(opt-in, generic):DiffusionUpdateWeightFromTensorLoRAships base weights untouched, plus perlayer
<prefix>.lora_A.weight/<prefix>.lora_B.weight(pre-rounded to theforward dtype — the same bits FSDP's mixed-precision gather feeds the train
forward) and
<prefix>.lora_scaling(fp64, so the engine recovers the exactpython float peft multiplies by).
Engine — cosmos3_bitwise patch group:
warn-and-drop name filtering; parts are buffered across calls because the
sender flushes one flattened bucket per dtype.
add_q/k/v_proj->to_qkvslices 0/1/2 (composes with the unfused-GEMMpatch from [cosmos3] Bitwise train/rollout parity: FSDP precision spec + cosmos3_bitwise rollout patch group #109),
to_add_out->to_out(instance-level wrap).out + F.linear(F.linear(x, A), B) * s.Verification
probe_lora_adapter_bitwise.py): adapter replay bitwise-equalto the peft path on all four cosmos3 LoRA shapes, with and without fused
weight slicing; merged path differs at mean|diff| ~1.4e-1.
parts arriving across two dtype buckets; attach points, slice ids, base
loading, and forward bits all verified.
train/model_output_mean_abs_diff = 0.0andmax_abs_diff = 0.0on everystep (previously 0.0 only at step 1). 144 adapters re-attached on each of
the syncs; log-prob ratio deviation stays at the 1e-5 sampler-path floor.