Skip to content

[Bugfix][PCP] Align MoE routing padding with gathered tokens - #60946

Open
wangyicong52 wants to merge 1 commit into
vllm-project:mainfrom
wangyicong52:codex/fix-pcp-moe-padding
Open

wangyicong52 wants to merge 1 commit into
vllm-project:mainfrom
wangyicong52:codex/fix-pcp-moe-padding

Conversation

@wangyicong52

@wangyicong52 wangyicong52 commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

The non-all-to-all PCP path gathers hidden states and router logits across PCP ranks, but the router still reads the rank-local padding mask. With N local rows and P PCP ranks, routing processes P*N rows while the mask still has N entries. CUDA top-k checks that the mask length matches the token count and can raise an is_padding size mismatch.

This PR aligns the MoE padding mask with the PCP-gathered token rows and restores the original rank-local mask in a finally block. The dispatch/combine interfaces and PCP1/all-to-all paths remain unchanged. Regression tests cover PCP1/2/4, the all-to-all bypass, per-forward caching, and exception cleanup.

A search of open PRs covering PCP padding, PCP MoE, and the relevant padding-cache key found no equivalent fix. Nearby PRs #53639, #60049, and #56258 address different paths.

Implementation

  • Non-all-to-all PCP routing receives a padding mask with the same rank-major layout and row count as the gathered hidden states.
  • The original local padding mask is restored on normal and exceptional exits.
  • The gathered mask is cached per forward context and reused across MoE calls.

Validation

Environment

  • Model: official mistralai/Mistral-Small-4-119B-2603 native FP8 checkpoint.
  • Image: vllm/vllm-openai:nightly-81198e97ba7eee2a22540caaa756b7fdddcb4d93.
  • Hardware: two nodes, each with 4x NVIDIA H20 GPUs.

vLLM startup arguments

# Set MASTER_ADDR to the reachable IP address of the master node.
vllm serve /models/Mistral-Small-4-119B-2603 \
  --host 0.0.0.0 \
  --port 19180 \
  --served-model-name mistral-small4-pcp-ab \
  --config-format mistral \
  --load-format mistral \
  --tokenizer-mode mistral \
  --dtype bfloat16 \
  --tensor-parallel-size 2 \
  --prefill-context-parallel-size 2 \
  --pipeline-parallel-size 1 \
  --data-parallel-size 1 \
  --decode-context-parallel-size 1 \
  --distributed-executor-backend mp \
  --master-addr "${MASTER_ADDR}" \
  --master-port 29511 \
  --language-model-only \
  --enforce-eager \
  --attention-backend TRITON_MLA \
  --moe-backend triton \
  --max-model-len 4096 \
  --max-num-batched-tokens 2048 \
  --max-num-seqs 4 \
  --block-size 64 \
  --kv-cache-dtype auto \
  --gpu-memory-utilization 0.80 \
  --no-enable-prefix-caching \
  --no-async-scheduling

Result

Without this fix (baseline):
With the unmodified image and model, all seven native weight shards loaded (29.34 GiB per rank). The unpatched run then failed during vLLM's standard startup memory profiling, before the API server became ready:

RuntimeError: .../topk_softmax_kernels.cu:785, is_padding size mismatch, expected: 4096

With this PR:
Applying the _sequence_parallel_context change from this PR at startup allowed the TP2/PCP2 deployment to start and generate successfully. It passed all five chat smoke requests, including a 2,235-token input and concurrent requests of different lengths.

Using the official tests/evals/gsm8k/gsm8k_eval.py scoring logic, we evaluated all 1,319 test examples with 5-shot prompts, temperature=0, seed=42, max_tokens=256, and concurrency=4. Results:

Run GSM8K Invalid answers Runtime result
Baseline + TP4/PCP1 1191/1319 (90.2957%) 0 Full evaluation completed; 0 API errors and 0 Pod restarts
Baseline + TP2/PCP2 Could not run — Failed during startup profiling with the padding-size mismatch above
This PR applied + TP2/PCP2 1197/1319 (90.7506%) 0 Full evaluation completed; 0 API errors and 0 Pod restarts

AI assistance was used to extract the change, prepare regression tests and run the validation described above. The human submitter retains responsibility for line-by-line review and final sign-off.

Extract the generic PCP MoE padding-mask fix from vllm-project#59857 as requested by LucasWilkinson. Match the routing mask to gathered token rows and restore the local ForwardContext mask after the call.

Cache the gathered mask per forward and cover normal and exceptional exits for PCP1/2/4 and the all-to-all bypass.

Co-authored-by: OpenAI Codex <noreply@openai.com>
Signed-off-by: wangyicong <wangyicong@bytedance.com>
@mergify mergify Bot added the bug Something isn't working label Oct 10, 2026
wangyicong52 added a commit to wangyicong52/vllm that referenced this pull request Oct 10, 2026
Resolve LucasWilkinson's review comments by extracting the generic PCP MoE padding-mask fix into draft vllm-project#60946 and reverting the corresponding shared MoERunner changes from this PR.

Move PCP token-ID gathering and per-forward caching into the DeepSeek-V4.1 MoE model, covering both the normal and deferred-finalization entry points while retaining local IDs for embedding and Engram.

Review discussions: vllm-project#59857 (comment) and vllm-project#59857 (comment).

Co-authored-by: OpenAI Codex <noreply@openai.com>
Signed-off-by: wangyicong <wangyicong@bytedance.com>
@rebecca-bell-2001

Copy link
Copy Markdown

This is a Bytebell Pull request verifier bot.
Here is our verification report for this PR

Full report here

Findings — 9 in 2 files

vllm/model_executor/layers/fused_moe/runner/moe_runner.py

major · bug · lines 661–665

Guard omits do_naive_dispatch_combine, so when naive EP dispatch rewrites the token axis before the PCP gather, the mask is only P×local and can still mismatch.

Evidence:

minor · bug · lines 667–673

Cached gather is never invalidated: additional_kwargs lives for the whole context, so a later MoE call whose local mask changed reuses stale rows.

Evidence:

minor · performance · lines 668–672

Gather runs whenever a mask is set, yet every mask consumer gates on VLLM_MOE_SKIP_PADDING, so with that flag off it adds a per-pass PCP collective nobody reads.

Evidence:

tests/kernels/moe/test_routing.py

minor · tests · line 80

Every case supplies a tensor mask, so the new local_is_padding is not None guard is never exercised; deleting it leaves this test green.

Evidence:

minor · tests · line 82

dp_metadata is left None, so the sp_local_sizes branch of the wrapped context — entered in real PCP forwards — is never combined with the gathered mask.

Evidence:

minor · tests · line 83

The test re-declares the gather predicate and hard-codes do_naive_dispatch_combine=False, so it cannot catch drift between the mask-gather condition and the real dispatch condition.

Evidence:

minor · tests · lines 94–104

New test only asserts ctx.is_padding and the gather count; nothing exercises the router/top-k path the fix targets, so the reported mismatch stays untested.

Evidence:

minor · tests · line 98

Mask order is pinned only against the mock's own concatenation; a real gather that is not rank-major in the hidden-state order still passes.

Evidence:

minor · tests · line 100

This assertion cannot fail: the mock gathers hidden+4 with the same rank offsets as hidden, so logits always equals states+4 by construction.

Evidence:

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

@wangyicong52

Copy link
Copy Markdown
Contributor Author

@LucasWilkinson Could you have a look when you have time? Thanks!

@LucasWilkinson

LucasWilkinson commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Apologies for the delay; it's a holiday here in Canada, I will try to look tomorrow or the day after 👍

@wangyicong52

wangyicong52 commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor Author

Apologies for the delay; it's a holiday here in Canada, I will try to look tomorrow or the day after 👍

No worries at all—thanks for letting me know! Just enjoy the Thanksgiving holiday!

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants