fix: make num_experts BC fallback not override explicit n_routed_experts#47409
Closed
webtecnica wants to merge 1 commit into
Closed
fix: make num_experts BC fallback not override explicit n_routed_experts#47409webtecnica wants to merge 1 commit into
webtecnica wants to merge 1 commit into
Conversation
In GlmMoeDsaConfig and DeepseekV32Config __post_init__, the legacy num_experts key unconditionally overwrites the modern n_routed_experts even when the latter was explicitly provided in the config. Fix: only apply the BC fallback when n_routed_experts is still at the defining class's default value (meaning it was NOT explicitly set). Fixes huggingface#47355 Signed-off-by: webtecnica <marcos@webtecnica.com.br>
Contributor
|
[For maintainers] Suggested jobs to run (before merge) run-slow: deepseek_v32, glm_moe_dsa |
Contributor
CI recapDashboard: View test results in Grafana
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #47355
Root cause: In both
GlmMoeDsaConfig.__post_init__andDeepseekV32Config.__post_init__, the backward-compatibility re-route fromnum_experts(legacy key) ton_routed_expertswas unconditional — it could overwrite an explicitn_routed_expertsvalue from the sameconfig.json.Example: A config with
{"n_routed_experts": 168, "num_experts": 256}would silently use 256 experts, causing an OOM downstream (vLLM allocates for 256 experts instead of 168).Fix: Only apply the BC fallback when
n_routed_expertsis still at its defining class's default value, which means it was NOT explicitly provided. Sincen_routed_expertsis a dataclass field consumed by__init__before__post_init__runs, we compareself.n_routed_expertsagainsttype(self).n_routed_experts.Files changed:
modular_deepseek_v32.pyconfiguration_deepseek_v32.pyconfiguration_glm_moe_dsa.pytests/models/glm_moe_dsa/test_config_glm_moe_dsa.pyVerification (6 test cases, all pass):
n_routed_experts=168winsnum_expertsn_routed_experts