[AMD] Fix GLM-5.2 MTP Quark excludes - #30265
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces support for the GlmMoeDsaForCausalLMNextN model architecture, enabling draft model configuration and shape derivation. It refactors the quantization configuration resolution for NextN models into a helper method, which is then overridden in GlmMoeDsaForCausalLMNextN to handle specific weight mappings and MoE expert exclusions. Feedback on the changes highlights a potential issue where modifying quant_config.exclude_layers in-place could cause side-effects if the configuration instance is shared, suggesting to copy the configuration before modification.
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.
GlmMoeDsaForCausalLMNextN didn't override hf_to_sglang_mapper, so it inherited DeepseekV3ForCausalLMNextN's hardcoded "model.layers.61" -> "model.decoder" substr mapping. This generic mapping runs on quant_config.exclude_layers before model construction and would incorrectly rewrite GLM-5.2's real main-model layer 61 (a valid decoder layer distinct from its layer-78 MTP block), corrupting quantization excludes for both the main model and the MTP decoder. Override it with an empty WeightsMapper() so the generic step is a no-op, leaving exclude_layers remapping fully to the existing _resolve_nextn_quant_config, which already handles GLM's MTP layer prefix correctly.
layer_quant_config patterns are keyed by checkpoint names like "model.layers.<N>.self_attn.*" (N = MTP layer index), but SGLang queries per-module quant schemes with runtime prefixes such as "model.decoder.self_attn.*". Without remapping, _find_matched_config never matches these MTP-specific patterns and silently falls back to the layer-type/global scheme, which can pick the wrong scheme (e.g. MXFP4 instead of PTPC-FP8) or the wrong packed weight shape. Reuse the same model.layers.<N> -> model.*/model.decoder.* mapping already used for exclude_layers, factored into _map_mtp_ckpt_name, and apply it to the layer_quant_config dict keys as well.
Fix hf_to_sglang_mapper and layer_quant_config remap for GLM MTP Quark
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
Hey — thanks for the fix. Heads up on some overlap: the main change here in Your PR does more than ours (the Thanks! |
Thanks for pointing this out. I added a reference to #29781 in the PR description and also noted that the ROCm This PR keeps the GLM-specific pieces here: |
|
/tag-and-rerun-ci |
|
@amd-bot ci-status |
CI Status for PR #30265Merge verdict: ⛔ Do not merge on a green tick. PR CI is incomplete and this PR's core change is not exercised by any PR-CI test. On AMD, Caution The GLM-5.2 Quark MXFP4 MTP path this PR fixes has zero PR-CI coverage.
Caution PR CI is incomplete — required downstream stages were fast-fail skipped, not tested:
Changed files: Executed CI failure attribution: AMD: 0 executed test failures (all AMD tests skipped — pipeline never ran) · Others: 5 root-cause failures, 0 related to this PR. Other Executed Failures
Details / what to do before merge
Generated by amd-bot using Claude Code CLI |
|
@Raiden-Makoto please add unit test file in follow-up PR, and instantiate it in nightly and show results in PR. |
HaiShaw
left a comment
There was a problem hiding this comment.
Change is gated by quark quant.
Co-authored-by: zhaolin <zhaolin@amd.com> Co-authored-by: Raiden-Makoto <Raiden-Makoto@users.noreply.github.com> Co-authored-by: HAI <hixiao@gmail.com>
|
#30379 adds a CPU unit-test for the change. Can work with @fanxingran to get that instantiated in the nightly. I also noticed that #30265 actually fails one of the tests in the unit test file (specifically, |
…de tests GlmMoeDsaForCausalLMNextN._resolve_nextn_quant_config (sgl-project#30265) remapped layer_quant_config keys by mutating the shared quant_config in place, which can corrupt the main model's per-layer scheme selection (flagged in the sgl-project#30265 review). Copy quant_config and its nested dict before any mutation, and only copy when a remap/exclude is actually needed. Tests (the missing sgl-project#30265 coverage): - test/registered/unit/models/test_nextn_quark_exclude.py (CPU CI, base-a-test-cpu): base DeepseekV3ForCausalLMNextN passthrough plus the GLM-5.2 MTP exclude / layer_quant_config remap and no-mutation guarantees. - test/registered/amd/accuracy/mi35x/test_glm52_mxfp4_mtp_eval_mi35x.py (nightly-amd-8-gpu-mi35x-glm52-mxfp4-mtp): 8-GPU GLM-5.2-MXFP4 EAGLE-MTP GSM8K eval verifying the bf16 nextn layer loads under Quark MXFP4; wired into nightly-test-amd.yml and nightly-test-amd-rocm720.yml. Fix + CPU test originally by @fanxingran (sgl-project#30379); references sgl-project#29781. Co-authored-by: fanxingran <xingran.fan@amd.com>
Co-authored-by: zhaolin <zhaolin@amd.com> Co-authored-by: Raiden-Makoto <Raiden-Makoto@users.noreply.github.com> Co-authored-by: HAI <hixiao@gmail.com> (cherry picked from commit 07ef650)
) Image v0.5.15.post1-rocm720-mi35x-20260722 includes sgl-project#30265 (GlmMoeDsaForCausalLMNextN + quark excludes), fixing the MTP accuracy drop seen on 20260623. Re-validated on MI355X 2-node/4-GPU 1P1D, GSM8K 1319Q: base 0.930, mtp 0.933, dp8ep8 0.945 (all PASS >= 0.91). dp8ep8-mtp crashes in the DSA topk indexer under NextN+DP-attention (source bug, flagged).
…t #30265) The sglang v0.5.15.post1 base predates sgl-project/sglang#30265, so GLM-5.2 EAGLE/MTP crashes at draft weight-load: the MTP layer (index=num_hidden_layers) is entirely bf16, and its eh_proj is listed in the quark `exclude`, but sglang's DeepseekV3ForCausalLMNextN checks the BARE layer prefix `model.layers.<N>` (not the submodule-level `...<N>.eh_proj` exclude entry), so should_ignore_layer() returns False, eh_proj is built as an MXFP4 uint8 param, and load asserts (param [6144,6144] uint8 vs bf16 [6144,12288]). Add a self-locating, idempotent Python patch (deploy/docker/patches/sglang/) that probes the eh_proj submodule instead of the bare layer -> nextn_quant_config=None -> whole bf16 MTP layer. Wire a patch loop into Dockerfile.sglang (mirrors the vLLM one). Verified: coherent GLM-5.2 MTP output on the v0.5.15.post1 base. Temporary: no-ops once the base sglang carries #30265 (merged upstream 2026-07-08); drop deploy/docker/patches/sglang/ + the loop when we bump the base sglang. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pw7JSvdQb796xh5pEcctLz Signed-off-by: Zhang, Jiejing <jiejing.zhang@amd.com>
Co-authored-by: zhaolin <zhaolin@amd.com> Co-authored-by: Raiden-Makoto <Raiden-Makoto@users.noreply.github.com> Co-authored-by: HAI <hixiao@gmail.com>
Co-author: @Raiden-Makoto
Summary
This PR fixes GLM-5.2 MXFP4 MTP/NextN loading with Quark quantization.
GLM-5.2 uses
GlmMoeDsaForCausalLM, but its draft/MTP path currently reuses the DeepSeek NextN implementation. Quark records MTP excluded weights under the checkpoint prefixmodel.layers.78.*, while SGLang builds the draft runtime modules undermodel.*,model.decoder.*, and the fused MoE prefixmodel.decoder.mlp.experts.Because of this prefix and granularity mismatch, Quark
exclude_layerscan fail to match bf16 MTP weights. Some MTP modules may then be incorrectly built as MXFP4 parameters, causing shape mismatches when loading bf16 weights.This issue matches the MTP prefix mismatch described in Wafer's GLM-5.2 AMD writeup: https://www.wafer.ai/blog/glm52-amd.
What Changed
GlmMoeDsaForCausalLMNextN.GlmMoeDsaForCausalLMtoGlmMoeDsaForCausalLMNextNinstead ofDeepseekV3ForCausalLMNextN._resolve_nextn_quant_config()so GLM can override the behavior cleanly.exclude_layersat runtime for GLM-5.2 MTP:model.layers.78.eh_proj->model.eh_projmodel.layers.78.enorm->model.enormmodel.layers.78.hnorm->model.hnormmodel.layers.78.shared_head.norm->model.shared_head.normmodel.decoder.*model.decoder.mlp.expertsThis keeps mixed quantization behavior: excluded MTP modules remain bf16, while non-excluded draft modules can still use their Quark quant config.
Why This Is Needed
The existing DeepSeek NextN mapper only handles:
GLM-5.2 MTP uses:
Also, the fused routed experts module is queried by SGLang using the coarse runtime prefix:
but Quark may record expanded leaf names such as:
Mapping only the leaf names is not enough, because the fused MoE module checks the coarse module prefix. This PR adds the runtime names that SGLang actually queries.
Validation
Tested with GLM-5.2 MXFP4 on 4 AMD GPUs.
Without MTP:
With MTP:
Observed improvement:
TTFT remains roughly unchanged, which is expected because MTP primarily accelerates the decode phase rather than prefill.
Test Commands
Server without MTP:
Server with MTP:
Benchmark command:
Reference
Wafer GLM-5.2 AMD performance writeup:
https://www.wafer.ai/blog/glm52-amd
Related Work
Part of the generic DeepSeek NextN quant-config handling overlaps with the earlier fix proposed in #29781. This PR builds on that direction and extends it for GLM-5.2 by adding
GlmMoeDsaForCausalLMNextN, GLM-specific runtime prefix remapping, fused MoE exclude handling, andlayer_quant_configkey remapping.The ROCm draft-depth
cuda_runtime.hguard mentioned in the Wafer writeup was already addressed by #29373.CI States
Latest PR Test (Base): ❌ Run #28928760909
Latest PR Test (Extra): 🚫 Run #28929421551