Skip to content

[Fix] [AMD] quark: Keep MXFP8 shared experts out of the MXFP4 fused slot on dense-first models - #46800

Merged
BBuf merged 2 commits into
sgl-project:mainfrom
ZhengGong-amd:fix/quark-shared-expert-gate-per-layer
Oct 30, 2026
Merged

BBuf merged 2 commits into
sgl-project:mainfrom
ZhengGong-amd:fix/quark-shared-expert-gate-per-layer

Conversation

@ZhengGong-amd

@ZhengGong-amd ZhengGong-amd commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

amd/MiniMax-M3.1-Flash-Quark-MXFP4 crashes at load because its MXFP8 shared experts are fused into the packed MXFP4 routed-expert slot, so this PR makes the Quark fusion gate probe every named MoE layer and has the MiniMax-M3 hooks ask that gate.

RuntimeError: The size of tensor a (384) must match the size of tensor b (768) at non-singleton dimension 1

QuarkConfig.can_fuse_shared_expert() compares routed and shared specs only at layer 0 and the NextN block. When layer 0 is a dense MLP (MiniMax-M3, DeepSeek-V3, GLM-5.x, Kimi-K2.x), both lookups fall back to the global spec and agree, whatever the MoE layers hold. The MiniMax-M3 hooks never ask the gate anyway: they refuse only modelopt_mixed and minimax_m3_nvfp4.

Modifications

  • quark: can_fuse_shared_expert() also probes every target decoder layer whose routed or shared experts are named in layer_quant_config, so one mismatched layer refuses fusion in any entry order. Draft entries (mtp.*, NextN layers past num_hidden_layers, non-layers.<N> bases such as model.mtp.0) are skipped, so an FP8 MTP block still vetoes fusion only for the draft.
  • minimax: both MiniMax-M3 hooks call quant_blocks_shared_experts_fusion(quant_config), as DeepSeek-V2/V3/V4 do. The name list stays because MiniMaxM3Nvfp4Config does not implement the gate.

The checkpoint now loads unfused and logs why, the path --disable-shared-experts-fusion selects today. Across 13 local Quark checkpoints only this one changes (fused → unfused); MiniMax-M3, GLM-5.x, DeepSeek-R1-0528, Kimi-K2.6, Muse-Spark and Qwen3.5 MXFP4 exports keep their decision.

Correct output also needs the MXFP8 scale handling from #46693, #46587 and #44080 (on main 2160 weight_scale not found in params_dict). #46587 instead requantizes the shared expert to MXFP4 to keep fusion; Quark leaves lossy conversion opt-in via --enforce-shared-experts-fusion.

Accuracy Tests

4×MI355X, TP4, SGLANG_USE_AITER=1, main@6393e66b86 plus the same scale handling in every arm. GSM8K via sglang.test.run_eval, 5-shot chat, temperature 0, 1314 items.

Arm Load GSM8K
default crash (fusion enabled) —
--disable-shared-experts-fusion ok 0.9665, 0.9673
this PR, default ok, fusion disabled 0.9627, 0.9665

Both loading arms run the same unfused path; differences are within noise (stderr ±0.005).

New tests in test_quark_config.py and test_shared_experts_fusion_gates.py cover dense leading layers, both entry orders, named routed experts, NextN drafts and a real QuarkConfig in M3.1's layout; 106 pass.

Speed Tests and Profiling

Not a speed change: the fused path does not load on main.

Checklist


Found by Hyperloom.


CI States

Latest PR Test (Base): ❌ Run #40823923906
Latest PR Test (Extra): ❌ Run #40823923805
Latest PR Test (AMD ROCm 10): ⏳ Run #40823923811

@BBuf BBuf 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.

@ZhengGong-amd _first_named_shared_expert_layer() makes this global fusion decision depend on quant-config insertion order. I reproduced this with the actual config helpers: routed experts use FP4, layer 3 shared experts use FP4, and layer 4 shared experts use FP8. Listing layer 3 first returns True; reversing the same entries returns False. The True case still enables fusion for the incompatible later layer. Could you check every distinct matching target-model MoE layer, or conservatively disable fusion when their schemes differ? One compact regression covering both entry orders should be enough.

…fusion gate

can_fuse_shared_expert() compared the routed and shared-expert specs only at
layer 0 and the NextN draft block. Where layer 0 is a dense MLP (DeepSeek-V3,
MiniMax-M3) both lookups fall back to the global spec, so the gate allowed
fusion whatever the MoE layers hold. On amd/MiniMax-M3.1-Flash-Quark-MXFP4 that
answer would copy MXFP8 shared experts into the packed MXFP4 fused slot.

Also probe every target decoder layer whose routed or shared experts are named
in layer_quant_config, skipping draft-stack entries so an MTP block still
vetoes fusion only for the draft.

Generated-by: Hyperloom
Co-authored-by: Claude
…xpert can fuse

MiniMax-M3's fusion gates refused only the mixed-precision configs they named
(modelopt_mixed, minimax_m3_nvfp4). Every other shared-expert-fusing family
asks the quantization config through quant_blocks_shared_experts_fusion(), so a
Quark export with MXFP8 shared experts beside MXFP4 routed experts, such as
amd/MiniMax-M3.1-Flash-Quark-MXFP4, was fused and failed at load copying the
shared expert into the packed MXFP4 slot.

Generated-by: Hyperloom
Co-authored-by: Claude
@ZhengGong-amd
ZhengGong-amd force-pushed the fix/quark-shared-expert-gate-per-layer branch 2 times, most recently from 5ecf8f1 to 99eccc4 Compare October 29, 2026 05:31
@chaojhou

Copy link
Copy Markdown
Contributor

/tag-and-rerun-ci

@github-actions github-actions Bot added the run-ci CI: run the baseline test suite on this PR label Oct 29, 2026

@BBuf BBuf 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.

Thanks, checking all named target MoE layers resolves the ordering and later-layer mixed-precision issues. The regression covers both entry orders and excludes draft layers. LGTM; please wait for the relevant quantization CI before merging.

@BBuf
BBuf merged commit 16a3bf3 into sgl-project:main Oct 30, 2026
305 of 342 checks passed
Yuxingwang-intel pushed a commit to Yuxingwang-intel/sglang that referenced this pull request Nov 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-ci CI: run the baseline test suite on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants