Skip to content

[AMD] move shared expert check function to quark - #27057

Merged
HaiShaw merged 6 commits into
sgl-project:mainfrom
mqhc2020:marv/move_shared_expert_fusion_feasibility_check_func
Jun 13, 2026
Merged

[AMD] move shared expert check function to quark#27057
HaiShaw merged 6 commits into
sgl-project:mainfrom
mqhc2020:marv/move_shared_expert_fusion_feasibility_check_func

Conversation

@mqhc2020

@mqhc2020 mqhc2020 commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Motivation

For shared expert fusion feature, precise checks are added. This is aimed for a more general use for all models.

Modifications

There are two purposes behind this PR:

1. Add precise checks for shared expert fusion feasibility

So far most MXFP4 models have shared experts unquantized, so shared expert fusion cannot be applied. We add two checks for this:

  • a. The original check (see [AMD] Qwen3.5 MXFP4 breaks after shared expert fusion is enabled #22948): in the exclude list of the quantization config, do any of the shared expert layers appear?
  • b. Config comparison: compare the configs of routed experts and shared experts using _find_matched_config and deep_compare. The config contains quantization information for each layer. Note that if the model doesn't give each layer a specific config (e.g. Qwen3.5), there will be no difference and we still rely on check (a). So this check is for general use.
    We also move the check function can_fuse_shared_expert from the model code to quark.py, because it's more like quantization code and calls into some quantization-related utility functions.

2. Enable #25885 multi-streaming even without disable_shared_experts_fusion

#25885 can improve performance by multi-streaming the transformer flow. The problem is that #25885 uses the server arg disable_shared_experts_fusion, but if the model is not run with disable_shared_experts_fusion, shared expert fusion is treated as "enabled". So even when the model is not run with the flag, this PR uses the check function can_fuse_shared_expert to ensure that shared expert fusion is disabled. Therefore, #25885 can still benefit the model from multi-streaming.

Accuracy Tests

No impact because it just enables or disables a performance feature.

Speed Tests and Profiling

1. Serve

  GPU_MAX_HW_QUEUES=5 \
  SGLANG_USE_AITER=1 SGLANG_USE_AITER_UNIFIED_ATTN=1 \
  SGLANG_ALT_STREAM=1 SGLANG_GDN_QKVZ_BA_ALT_STREAM=1 SGLANG_QK_NORM_ALT_STREAM=0 \
  AITER_CONFIG_GEMM_BF16=/sgl-workspace/aiter/aiter/configs/qwen_bf16_tuned_gemm.csv \
  python3 -m sglang.launch_server \
    --model-path /data/amd/Qwen3.5-397B-A17B-MXFP4/ --trust-remote-code \
    --model-loader-extra-config '{"enable_multithread_load": true}' \
    --host 0.0.0.0 --port 8888 --tensor-parallel-size 2 \
    --attention-backend aiter --kv-cache-dtype fp8_e4m3 --page-size 16 \
    --chunked-prefill-size 8192 --mem-fraction-static 0.8 --watchdog-timeout 1200 \
    --disable-radix-cache \
    --enable-aiter-allreduce-fusion

Notes:

  • ROCM_QUICK_REDUCE_QUANTIZATION is left unset.

2. Serving (benchmark)

cd /sgl-workspace/sglang && python3 -m sglang.bench_serving \
  --host 0.0.0.0 --port 8888 --model /data/amd/Qwen3.5-397B-A17B-MXFP4/ \
  --dataset-name random --random-input 1024 --random-output 1024 \
  --random-range-ratio 0.8 --max-concurrency <cc> --num-prompts <cc*10> --seed 0

Checklist

Review and Merge Process

  1. Ping Merge Oncalls to start the process. See the PR Merge Process.
  2. Get approvals from CODEOWNERS and other reviewers.
  3. Trigger CI tests with comments or contact authorized users to do so.
    • Common commands include /tag-and-rerun-ci, /tag-run-ci-label, /rerun-failed-ci
  4. After green CI and required approvals, ask Merge Oncalls or people with Write permission to merge the PR.

CI States

Latest PR Test (Base): ✅ Run #27400593453
Latest PR Test (Extra): ❌ Run #27400593176

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a mechanism to determine if shared expert fusion can be performed, specifically adding a can_fuse_shared_expert check in the Quark quantization configuration and integrating it into Qwen MoE and Qwen 3.5 models. It also adds automatic disabling of shared expert fusion in Qwen 3.5 if it is not supported. The review feedback highlights a potential issue where replacing the original exclude_layers check in qwen2_moe.py with the new can_fuse_shared_expert method call will bypass the safety check for other quantization configurations that do not implement this new method, and suggests adding a fallback to the original check.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread python/sglang/srt/models/qwen2_moe.py Outdated
@mqhc2020
mqhc2020 marked this pull request as ready for review June 2, 2026 14:59
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@HaiShaw

HaiShaw commented Jun 7, 2026

Copy link
Copy Markdown
Collaborator

/tag-and-rerun-ci

@github-actions github-actions Bot added the run-ci label Jun 7, 2026
@HaiShaw

HaiShaw commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

@amd-bot ci-status

@amd-bot

amd-bot commented Jun 8, 2026

Copy link
Copy Markdown

@HaiShaw

CI Status for PR #27057

Merge verdict: All 5 real failures are unrelated to this PR — they are pre-existing/infra issues on AMD-allreduce, RDMA disaggregation, NPU perf, XPU, and CPU-benchmark paths, none of which touch Quark shared-expert fusion. However, this PR's actual new code is NOT exercised by any PR-CI test that ran — green CI does not verify the change. Functionally safe to merge from a "did I break CI" standpoint, but the behavior itself is unverified by PR CI.

Caution

This PR's core new logic — QuarkConfig.can_fuse_shared_expert() and Qwen3_5..._maybe_autodisable_shared_experts_fusion — runs only on AMD aiter + Quark-quantized MoE with shared experts. No PR-CI test hits that path: the AMD Quark MXFP4 suite (stage-b-test-1-gpu-small-amd-mi35x, ✅ passed) uses Qwen3-8B (dense) and Qwen3-30B-A3B (qwen3_moe, no shared experts), so can_fuse_shared_expert is never reached; the Qwen3.5-35B-A3B LoRA test is register_cuda_ci + BF16 (no aiter, no Quark). The only suite that exercises QuarkConfig.can_fuse_shared_expert is nightly-amd-8-gpu-mi35x-glm5-mxfp4 (nightly=True, does NOT run on PR CI). Before merge, run the GLM5-MXFP4 nightly suite and/or a Qwen3.5 MoE + Quark model on AMD aiter to confirm fusion is auto-disabled/enabled correctly.

Changed files: quark.py (+40), qwen2_moe.py (+6/-11), qwen3_5.py (+16/-1) — all 149 checks completed, no pending jobs.

AMD: 2 failures (0 related) · Others: 3 failures (0 related)

AMD CI Failures

Job Test File Test Function Error Related? Why
stage-c-test-large-8-gpu-amd (mi325-8gpu, 3) test/registered/ops/test_aiter_allreduce_fusion_amd.py test_fused_ar_rms_residual_accuracy AssertionError: Residual accuracy check failed 🟢 PR touches Quark shared-expert MoE fusion; this tests aiter allreduce+RMS residual fusion — disjoint code path
stage-b-test-large-8-gpu-mi35x-disaggregation-amd (mi35x-8.fabric) test/registered/amd/disaggregation/test_mori_transfer_engine_e2e.py setUpClass (server start) RegisterRdmaMemoryRegion failed! errno:22 / Cannot allocate memory [12] 🟢 RDMA memory-registration infra failure on the fabric runner; no relation to model/quant code

Other CI Failures

Job Test File Test Function Error Related? Why
stage-b-test-1-npu-a2 (0) test/registered/ascend/basic_function/quant/test_npu_w8a8_quantization.py test_gsm8k AssertionError: 533.6 not >= 700 (throughput) 🟢 NPU W8A8 throughput threshold flake; PR adds no NPU code
stage-a-test-1-gpu-xpu test/registered/xpu/test_xpu_basic.py test_basic_generation AttributeError: 'NoneType' object has no attribute 'long' 🟢 XPU backend init failure; PR changes are AMD/Quark-only
build-test (xeon-gnr, base-b-test-cpu) test/registered/bench_fn/test_benchmark_datasets_api.py test_gen_mm_prompt_excludes_special_tokens AssertionError: {2056,2053,2054,2055} is not false 🟢 CPU benchmark-dataset util test; unrelated to MoE/quant

Details / what to do before merge

  • Coverage gap (the real action item): the diff is unverified by PR CI. To actually exercise it: (a) trigger nightly-amd-8-gpu-mi35x-glm5-mxfp4 (exercises QuarkConfig.can_fuse_shared_expert via the GLM5 MXFP4 + aiter path), and (b) ideally load a qwen3_5_moe_text Quark model on AMD aiter to confirm _maybe_autodisable_shared_experts_fusion flips disable_shared_experts_fusion only when layer quant configs diverge.
  • The refactor preserves behavior for non-Quark backends (they getattr(...can_fuse_shared_expert, None) → fall through as fusable), so no regression risk to CUDA/other paths — but this is also why nothing in standard PR CI changes behavior.
  • None of the 5 failures need investigation for this PR; they are pre-existing AMD-fusion-accuracy, RDMA-infra, NPU-perf, XPU, and CPU flakes that should be tracked independently.

Generated by amd-bot using Claude Code CLI

@HaiShaw
HaiShaw merged commit f288283 into sgl-project:main Jun 13, 2026
144 of 154 checks passed
Chronostasys pushed a commit to MindLab-Research/sglang that referenced this pull request Aug 24, 2026
jakki-amd pushed a commit to jakki-amd/sglang that referenced this pull request Sep 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants