Skip to content

Fix moe scratch buffer sizing for the TP>1 and DP>1 case - #55105

Closed
elvircrn wants to merge 1 commit into
vllm-project:mainfrom
elvircrn:humming-buffer-sizing
Closed

elvircrn wants to merge 1 commit into
vllm-project:mainfrom
elvircrn:humming-buffer-sizing

Conversation

@elvircrn

@elvircrn elvircrn commented Sep 3, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

Fix the Humming MoE scratch-buffer assertion with DeepEP v2, expert parallelism, and tensor sequence parallelism.

FusedMoEConfig.max_num_tokens is the scheduler capacity for one DP rank. DeepEP v2 previously treated that unsharded value as the capacity of every physical EP input rank. With TP sequence parallelism, each input rank instead owns at most ceil(max_num_tokens / sp_size) rows. The oversized DeepEP CUDA-graph receive tensor then exceeded Humming's DP-sized scratch buffer.

This change establishes one CUDA-graph-stable capacity for both sides:

per_ep_rank_capacity = next_power_of_2(ceil(max_num_tokens / sp_size))
post_dispatch_capacity = per_ep_rank_capacity * ep_size

DeepEP v2 uses the per-rank value for handle initialization and runtime decode buckets. Humming uses the corresponding post-dispatch value. For DP=4, TP/SP=8, EP=32, and max_num_tokens=128, both now agree on 512 rows instead of allocating 4096 rows in DeepEP and 512 in Humming.

This is separate from #56079: that PR invalidates sequence-parallel padding during routing, but does not change DeepEP's receive allocation or Humming's scratch-capacity validation.

Duplicate-work check

This updates the existing PR for this assertion. A search for open DeepEP v2/Humming scratch-buffer fixes found no duplicate PR.

Test plan and results

.venv/bin/python -m pytest tests/kernels/moe/test_moe.py -k 'humming_permute_scratch or humming_deepep_v2_scratch or deepep_v2_rank_capacity' -v
5 passed, 1119 deselected

.venv/bin/pre-commit run --files <six changed Python files>
All applicable hooks passed, including ruff, mypy, SPDX, and configuration validation.

No model evaluation was run because this changes buffer capacity, not routing results or model numerics.

AI assistance was used to analyze the capacity mismatch, implement the change, and add tests. The submitter must review every changed line and the test output before merge.

Signed-off-by: Elvir Crncevic elvircrn@gmail.com

@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.

max_expanded_rows = (
self.moe_config.max_num_tokens
* self.moe_config.dp_size
* self.moe_config.ep_size

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.

what about for TP with DP? maybe it should be max(DP, EP)?

@elvircrn elvircrn Sep 11, 2026 •

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.

ep_size should still be correct, we just care about the total number of ranks participating in MoE.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm ok with this change but I'm still wondering why it only applies to DeepEPV2 and not other dispatchers?

@elvircrn
elvircrn force-pushed the humming-buffer-sizing branch 2 times, most recently from d8582e0 to bced643 Compare September 13, 2026 20:22
@bnellnm

bnellnm commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

Why is this only needed for DeepEPV2?

@mergify

mergify Bot commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @elvircrn.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

Size DeepEP v2 buffers from sequence-parallel token capacity and use the
same per-rank capacity for Humming permute scratch. Preserve the current
shared scratch manager and allocate activation scratch for its runtime dtype.

Upstream already handles grouped SITU activation through process_input, so
keep that path while rebasing the capacity fix.

Co-authored-by: OpenAI Codex <codex@openai.com>
Signed-off-by: Elvir Crncevic <elvircrn@gmail.com>
@mergify

mergify Bot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @elvircrn.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added the needs-rebase label Oct 8, 2026
@elvircrn elvircrn closed this Oct 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants