Skip to content

[DeepSeek V4] Fix fused MoE expert distribution - #57465

Merged
tlrmchlsmth merged 4 commits into
vllm-project:mainfrom
itayalroy:fix_dsv4_fused_moe_expert_distribution
Sep 18, 2026
Merged

tlrmchlsmth merged 4 commits into
vllm-project:mainfrom
itayalroy:fix_dsv4_fused_moe_expert_distribution

Conversation

@itayalroy

@itayalroy itayalroy commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

DeepSeek V4's fused MoE path incorrectly used the TP size and rank to calculate expert distribution and the number of physical experts on each GPU. These values should be determined by the EP group.

Use the EP size and rank instead, matching the MegaMoE path and other MoE models.

cc @zyongye @tlrmchlsmth

DeepSeek V4's fused MoE path incorrectly used the TP size and rank to calculate expert distribution and the number of physical experts on each GPU. These values are determined by the EP group.

Use the EP size and rank instead, matching the MegaMoE path and other MoE models.

Signed-off-by: Itay Alroy <ialroy@nvidia.com>
@itayalroy
itayalroy requested a review from zyongye as a code owner September 18, 2026 01:01

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@mergify mergify Bot added deepseek Related to DeepSeek models DSv4 labels Sep 18, 2026
@tlrmchlsmth

Copy link
Copy Markdown
Member

/ci run

@tlrmchlsmth
tlrmchlsmth enabled auto-merge (squash) September 18, 2026 02:08
@tlrmchlsmth tlrmchlsmth added the ready ONLY add when PR is ready to merge/full CI is needed label Sep 18, 2026
@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #89757 for commit f22f779acd42.

@github-actions

Copy link
Copy Markdown

✅ @itayalroy, CI is now available for this PR.

  • /ci run starts upstream CI; /amd-ci run starts AMD CI only.
  • Your branch must contain every commit currently on its upstream target branch. Merge or rebase onto the latest target branch, then rerun the command. Append --allow-stale to a run command to test an outdated branch at your own risk.
  • /ci retry retries failed jobs in the CI build for the current PR head. If the current head has no CI build, it starts a new CI build for the current head containing only jobs that failed in the latest earlier CI build for this PR.
  • /amd-ci retry retries failed jobs in AMD CI for the current PR head. Use /amd-ci run when the current head has no AMD CI build.
  • /ci cancel cancels scheduled or running CI builds for this PR branch; /amd-ci cancel does the same for AMD CI only.

Comment thread vllm/models/deepseek_v4/nvidia/model.py Outdated
Comment on lines +1005 to +1007
assert self.n_physical_experts % ep_size == 0, (
f"n_physical_experts={self.n_physical_experts} must be divisible by "
f"tp_size={self.tp_size}. Adjust num_redundant_experts."
f"ep_size={ep_size}. Adjust num_redundant_experts."

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this is actually too strict - we don't require divisibility for ep_size... at least not in all cases

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I checked a few other FusedMoE models (including DSV2/V3). They also calculate the number of local physical experts as num_physical_experts // ep_size, they just do not assert divisibility. I aligned DeepSeek V4 with them. Currently this is only read by EPLB, but I wonder if we should make a broader change to account for uneven distribution across all models.

Signed-off-by: Itay Alroy <ialroy@nvidia.com>
auto-merge was automatically disabled September 18, 2026 10:58

Head branch was pushed to by a user without write access

@tlrmchlsmth

Copy link
Copy Markdown
Member

/ci run

@tlrmchlsmth
tlrmchlsmth enabled auto-merge (squash) September 18, 2026 13:58
@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #89852 for commit 7ca83ab06420.

@itayalroy

Copy link
Copy Markdown
Contributor Author

/ci retry

@github-actions

Copy link
Copy Markdown

✅ Queued 1 failed job(s) for retry in Buildkite CI #89852.

@tlrmchlsmth
tlrmchlsmth merged commit 017dced into vllm-project:main Sep 18, 2026
82 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deepseek Related to DeepSeek models DSv4 ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants