Skip to content

[diffusion] fix: keep a LoRA unmerged under merge_mode auto when merging would round its update away - #43385

Merged
mickqian merged 4 commits into
mainfrom
mick/diffusion-lora-auto-dynamic-when-merge-rounds
Oct 10, 2026
Merged

mickqian merged 4 commits into
mainfrom
mick/diffusion-lora-auto-dynamic-when-merge-rounds

Conversation

@mickqian

@mickqian mickqian commented Oct 9, 2026 •

Copy link
Copy Markdown
Collaborator

Motivation

--lora-merge-mode auto, the default, merges an adapter into regular BF16 weights. Distilled adapters carry updates far smaller than a BF16 ulp of the weights they modify, so the merge rounds most of the update away. The MiniMax-H3 Turbo LoRA (larryvrh/MiniMax-H3-Turbo-Lora) is one: a merge drops 77–97% of the update in typical layers, and 75% over the whole transformer.

Nothing fails or warns when this happens. The output just comes out softer than the adapter's reference, which applies the LoRA at runtime for exactly this reason. So the default silently degrades these adapters.

Modifications

  • BaseLayerWithLoRA.merge_rounding_norms() estimates, over an even sample of rows, how much of the active update a merge into the weight dtype would round away. It uses the same slicing and scaling as the merge, and writes nothing.
  • Under auto, set_lora with one adapter now does the following:
    • attaches the adapter unmerged and measures the share of the update a merge would lose;
    • if that share is above 50% (AUTO_MERGE_MAX_ROUNDING_LOSS), i.e. the merge would destroy most of the update, keeps the adapter dynamic and logs why;
    • otherwise merges as before.
  • The result is cached per (module, paths, strengths, alphas), so repeated set_lora calls do not measure again.
  • Why 50%. The threshold has to separate adapters a merge destroys from adapters it only trims. CI's style LoRAs lose 14% (Cseti/Wan-LoRA-Arcane-Jinx-v1) and 31% (reverentelusarca/elusarca-anime-style-lora-z-image-turbo) in a merge. Earlier cuts at 10% and 30% made them dynamic, and the dynamic path then cost Wan2.1 1.3B +52% and Z-Image +44% per step, failing both cases' latency checks. At 50% they merge exactly as before, while the distilled H3 Turbo adapter (75%) stays dynamic.
  • Explicit merge and dynamic are unchanged, and so are FSDP-sharded and quantized weights, which auto already keeps dynamic.
  • The --lora-merge-mode help and the MiniMax-H3 cookbook describe the new rule.

Accuracy Tests

  • test_lora_auto_merge_rounding.py (new, CPU) checks:

    • the estimate against the rounding of an actual merge;
    • that an update below a BF16 ulp is mostly lost and one above it survives;
    • that FP32 weights report nothing;
    • that the pipeline decision measures once and follows the threshold.

    The existing LoRA unit tests still pass (27 passed).

  • End to end on 2×H200, MiniMax-H3 T2VA at Ulysses 2 with the Larry Turbo v4 adapter, num_inference_steps: 9, same seed:

    --lora-merge-mode log MP4 md5 latency
    auto (this PR) "stays unmerged on transformer: merging it into the base weights would round away 75% of its update" 95a418705f7b 27.60 s
    dynamic — 95a418705f7b 27.59 s
    merge — 342c3151970f 25.78 s

    auto now gives the dynamic output byte for byte. Side by side, the merged video is visibly softer: the cats blur and the scene loses detail that the dynamic run keeps.

Speed Tests and Profiling

Keeping the Turbo LoRA dynamic costs 1.8 s on the 9-point schedule (27.6 against 25.8 s), which is the price of getting the adapter's full effect. A faster dynamic path, for example adding the LoRA delta inside the second GEMM, can follow separately. Adapters whose updates survive a merge, which covers the usual style LoRAs, still merge as before.

Checklist

🤖 Generated with Claude Code


CI States

Latest PR Test (Base): ✅ Run #38026407983
Latest PR Test (Extra): ❌ Run #38026407773
Latest PR Test (AMD ROCm 10): ⏳ Run #38026407934

…ing would round its update away

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mickqian mickqian added the run-ci CI: run the baseline test suite on this PR label Oct 9, 2026
@mickqian
mickqian requested a review from ping1jing2 as a code owner October 9, 2026 13:56
@mickqian mickqian added the diffusion SGLang Diffusion label Oct 9, 2026
@mintlify

mintlify Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated
sglang-doc 🟢 Ready View Preview Oct 9, 2026, 2:00 PM

💡 Tip: Enable Automations to automatically generate PRs for you.

mickqian and others added 3 commits October 10, 2026 09:07
…yle LoRAs keep merging

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… the LoRA update

CI's style LoRAs lose 14% (Wan Arcane-Jinx) and 31% (Z-Image anime) in a
BF16 merge and pay up to +50% per step on the dynamic path, while distilled
adapters such as MiniMax-H3 Turbo lose 75%.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mickqian
mickqian merged commit d8c5eca into main Oct 10, 2026
136 of 149 checks passed
@mickqian
mickqian deleted the mick/diffusion-lora-auto-dynamic-when-merge-rounds branch October 10, 2026 07:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

diffusion SGLang Diffusion documentation Improvements or additions to documentation lora run-ci CI: run the baseline test suite on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant