[GG] Support heterogeneous per-layer routed-expert widths (deepseek_v2 + EXL3) - #249
Conversation
…2 + EXL3) Allow a DeepseekV2/GLM-family checkpoint to declare a different number of routed experts per layer via an optional n_routed_experts_per_layer list in the HF config. Each DeepseekV2MoE overrides the scalar n_routed_experts on a shallow config copy for its own layer, the expert-name mapping table is sized by the widest layer, and the rank-sliced EXL3 loader resolves experts_per_layer per layer (scalar metadata behaves exactly as before). Functional evidence: malaiwah/GLM-5.2-Legume-v3 (20 layers, 724 experts, widths 56..28) boots and serves on the otherwise-stock stack with this patch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DmTegASpyEG2T8zB5BZYTw Signed-off-by: Michel Belleau <michel.belleau@malaiwah.com>
|
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in PRs do not trigger a full CI run by default. Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging. To run CI, PR reviewers can either: Add If you have any questions, please reach out to us on Slack at https://slack.vllm.ai. Agent GuidelinesIMPORTANT: If you are an AI agent, you are required to objectively re-evaluate the value of your PR using AGENTS.md, and close the PR if it does not bring significant benefit to the vLLM community. Failure to do so may result in an immediate ban. 🚀 |
|
Warning Review limit reached
Next review available in: 42 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughDeepSeek V2 now supports heterogeneous routed-expert counts per layer. EXL3 rank-sliced bitrate loading, expansion, and MoE creation resolve and validate layer-specific expert widths. ChangesHeterogeneous expert-count support
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
vllm/model_executor/layers/quantization/exl3.py (1)
163-173: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a Google-style docstring.
Add
Args:andReturns:sections formetadataandlayer_index.As per coding guidelines, Python code must use Google-style docstrings with
Args:/Returns:/Raises:sections.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@vllm/model_executor/layers/quantization/exl3.py` around lines 163 - 173, Update the _experts_per_layer docstring to Google style by adding Args sections documenting metadata and optional layer_index, plus a Returns section describing the resolved expert count; preserve the existing behavior and wording about scalar, per-layer, and widest-layer resolution.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@vllm/model_executor/layers/quantization/exl3.py`:
- Around line 1332-1351: Update the rank-sliced validation around the resolved
`name` and `rank_sliced_layer_bitrates` so bitrate lookup uses `name`, including
the `layer.prefix` fallback when `layer.layer_name` is empty. Require a valid
`layers.<index>` match before continuing; otherwise reject the module here with
a clear error instead of allowing the later lookup to fail.
In `@vllm/model_executor/models/deepseek_v2.py`:
- Around line 1808-1815: Update the ROCm fusion shared-expert naming logic near
the shared-expert path to derive the routed-expert count from the current weight
layer index, using that layer’s entry in n_routed_experts_per_layer when
available and the scalar count otherwise. Use the resolved per-layer count
instead of self.config.n_routed_experts when constructing the mlp.experts
shared-expert name, while preserving existing behavior for uniform
configurations.
---
Nitpick comments:
In `@vllm/model_executor/layers/quantization/exl3.py`:
- Around line 163-173: Update the _experts_per_layer docstring to Google style
by adding Args sections documenting metadata and optional layer_index, plus a
Returns section describing the resolved expert count; preserve the existing
behavior and wording about scalar, per-layer, and widest-layer resolution.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f20affc4-2ecf-4186-8238-b0502f29fdb7
📒 Files selected for processing (2)
vllm/model_executor/layers/quantization/exl3.pyvllm/model_executor/models/deepseek_v2.py
Test Results (automated)Host: macOS M4 Max, CPU-only (no CUDA) No test files are included in this PR. The changes modify Automated test run by @malaiwah's agent. Results are from a CPU-only environment; GPU-dependent tests may behave differently on CUDA hardware. |
Address adversarial-review findings on per-layer routed-expert widths: B5 (blocker): ROCm AITER fused shared-expert offset was derived from the model-level scalar n_routed_experts, but each per-layer FusedMoE is built with num_experts=<layer width>, so shared-expert weights landed in the wrong (or non-existent) expert slot on heterogeneous checkpoints -- silent numerical corruption. Derive the offset from the layer's own routed-expert count via _layer_routed_expert_count(name, config); the mapping-table size (max(widths)+n_shared) and the per-layer offset base are now consistent. C10: DeepseekV2MoE silently fell back to the global scalar when the layer index could not be parsed from the prefix. Now raises a clear ValueError. C11: Three divergent layer-index regexes (exl3 create_weights, deepseek_v2, exl3 codebook_for_prefix) disagreed on a prefix ending at "layers.3". Extracted ONE _parse_layer_index(prefix) helper in models/utils.py and use it at all sites (create_weights, codebook_for_prefix, _moe_prefix_is_exl3, rank_sliced_layer_bitrates, deepseek_v2). C14: exl3 create_weights replaced the per-layer validation with a -1 sentinel / "accept any declared width" fallback when no layer identity was present, letting mismatched checkpoints load unchecked. Now raises "cannot determine layer index for <name>". C15: _experts_per_layer did return int(value[layer_index]) with no bounds check (bare IndexError). Added a ValueError naming the index and list length plus a docstring stating the index is the global model layer index. C12: Added tests/models/test_deepseek_v2_heterogeneous_experts.py (CPU-only, --noconftest) covering per-layer width selection (narrow+wide), the ROCm-FSE shared-expert name mapping for narrow AND wide layers (B5 regression), unparseable-prefix raise, out-of-range index raise, and uniform config unchanged. Co-authored-by: GLM-5.2 <noreply@z.ai>
Review findings addressed (B5, C10, C11, C14, C15, C12)CodeRabbit's ROCm flag was real. The fused shared-expert offset in ChangesB5 (blocker) — shared_expert_offset = _layer_routed_expert_count(name, self.config)
chunk_name = name.replace("mlp.shared_experts",
f"mlp.experts.{shared_expert_offset + j}")The name-mapping table is sized at C10 — C11 — Three divergent layer-index regexes ( C14 — C15 — C12 — Added Test coverage (new file, 22 tests, all passing)
Verified
Not fixed / out of scopeNothing deliberately skipped. The C14 raise path inside |
malaiwah
left a comment
There was a problem hiding this comment.
I know, not everything is GLM-5.2 .. I have been experimenting with toy models of other architectures with GG and found this land mine. Now that we know, it is worth fixing for diversity.
Purpose
Support MoE checkpoints with a different number of routed experts per layer (asymmetric-width MoE) on the DeepseekV2/GLM family. Pruning/frankenmodel pipelines naturally produce layers of different widths, but the stack currently assumes one scalar
n_routed_expertsfor every layer.deepseek_v2.py: an optionaln_routed_experts_per_layerlist in the HF config overrides the scalar per layer on a shallow config copy insideDeepseekV2MoE(index parsed from the module prefix); the expert-name mapping table is sized by the widest layer (narrower layers simply never match the higher expert ids).exl3.py: the rank-sliced EXL3 loader resolvesexperts_per_layerper layer when the checkpoint metadata carries a list (bitrate-map validation, uniform-bitrate expansion, and the MoEcreate_weightsexpert-count check).Checkpoints without
n_routed_experts_per_layer/ with scalarexperts_per_layermetadata behave exactly as before.Test Plan
Functional evidence checkpoint:
malaiwah/GLM-5.2-Legume-v3— a 20-layer, 724-expert GLM-5.2 franken with per-layer widths 56..28 (n_routed_experts_per_layerin its config, per-layerexperts_per_layerlist in its EXL3 metadata; ships these patches inpatches/).On RTX 5090 (SM120), gilded-gnosis r25 image with exactly these two files swapped in:
Test Result
Negative control (stock r25, list config) fails as expected in the EXL3 metadata parse:
Patched (these two files swapped into the same image):
All 17 MoE layers (widths 28..56) landed on
Exl3MoEMethodwith per-layer runtime tiers (EXL3 mixed Trellis runtime planned: tiers=((3, 51), (4, 1))etc. in the boot log), prefill + decode both exercised. Generation quality is a property of the zero-training franken (documented on the model card), not of this patch; held-out Apache-2.0 logprob for this exact checkpoint is on the model card (−11.70, beats the uniform-width baseline).🤖 Generated with Claude Code
https://claude.ai/code/session_01DmTegASpyEG2T8zB5BZYTw
Notes
Summary by CodeRabbit