[BugFix][Ops][310p]:fix the accuracy issue caused by MoEGatingTopkSoftmax - #11391
Conversation
…tmax Signed-off-by: Tflowers-0129 <2906339855@qq.com>
|
👋 Hi! Thank you for contributing to the vLLM Ascend project. The following points will speed up your PR merge:
If CI fails, you can run linting and testing checks locally according Contributing and Testing. Tip 💡 Consider Linking a Related Issue or RFCYour PR title contains the [BugFix] tag, indicating a bug fix or new feature. Linking a related issue or RFC in the PR description is strongly encouraged — it gives reviewers helpful context and speeds up the review. You can use any of these keywords:
🙏 Thanks for helping us keep the project well-organized! |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a hardware-specific limitation on 310P devices where the MoE gating softmax operation produces incorrect results for input batches exceeding 1024 tokens. By introducing a chunking mechanism, the logic now safely processes large inputs in smaller segments, ensuring numerical stability and correctness. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
Suggested PR Title:
[Ops][BugFix] Chunk router logits for npu_moe_gating_top_k_softmax on 310P when tokens exceed 1024Suggested PR Summary:
### What this PR does / why we need it?
On Ascend 310P, the `torch_npu.npu_moe_gating_top_k_softmax` operator returns invalid routing results when it receives more than 1024 tokens. This PR fixes this issue by chunking the `router_logits` along the token dimension (dim 0) into chunks of size 1024, processing each chunk individually, and then concatenating the results.
Additionally, the review feedback suggests ensuring that `router_logits` is contiguous before processing to prevent potential runtime errors or silent correctness issues with custom NPU operators.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
A new unit test `test_select_experts_chunks_large_token_batch` has been added in `tests/ut/_310p/fused_moe/test_experts_selector_310.py` to verify the chunking behavior and output shapes for a batch of 2050 tokens.|
算子精度问题测试脚本: 表现: |
|
Fix this issue: #10892 |
…tmax (vllm-project#11391) ### What this PR does / why we need it? Based on previous community issues, we had already noticed that Qwen3.5-MoE might have a hidden accuracy issue. The root cause has now been identified: when the input token dimension of the `MoEGatingTopkSoftmax` operator is 2048, UB overflow may occur, which can eventually produce INF values and lead to accuracy degradation. Since the operator-level fix may not be externally available, this PR first introduces a workaround at the vllm-ascend level. ### Does this PR introduce _any_ user-facing change? NA ### How was this patch tested? Local test - vLLM version: v0.23.0 - vLLM main: vllm-project/vllm@b9a7cd4 Signed-off-by: Tflowers-0129 <2906339855@qq.com>
…tmax (vllm-project#11391) ### What this PR does / why we need it? Based on previous community issues, we had already noticed that Qwen3.5-MoE might have a hidden accuracy issue. The root cause has now been identified: when the input token dimension of the `MoEGatingTopkSoftmax` operator is 2048, UB overflow may occur, which can eventually produce INF values and lead to accuracy degradation. Since the operator-level fix may not be externally available, this PR first introduces a workaround at the vllm-ascend level. ### Does this PR introduce _any_ user-facing change? NA ### How was this patch tested? Local test - vLLM version: v0.23.0 - vLLM main: vllm-project/vllm@b9a7cd4 Signed-off-by: Tflowers-0129 <2906339855@qq.com>
…tmax (vllm-project#11391) ### What this PR does / why we need it? Based on previous community issues, we had already noticed that Qwen3.5-MoE might have a hidden accuracy issue. The root cause has now been identified: when the input token dimension of the `MoEGatingTopkSoftmax` operator is 2048, UB overflow may occur, which can eventually produce INF values and lead to accuracy degradation. Since the operator-level fix may not be externally available, this PR first introduces a workaround at the vllm-ascend level. ### Does this PR introduce _any_ user-facing change? NA ### How was this patch tested? Local test - vLLM version: v0.23.0 - vLLM main: vllm-project/vllm@b9a7cd4 Signed-off-by: Tflowers-0129 <2906339855@qq.com>
…tmax (vllm-project#11391) ### What this PR does / why we need it? Based on previous community issues, we had already noticed that Qwen3.5-MoE might have a hidden accuracy issue. The root cause has now been identified: when the input token dimension of the `MoEGatingTopkSoftmax` operator is 2048, UB overflow may occur, which can eventually produce INF values and lead to accuracy degradation. Since the operator-level fix may not be externally available, this PR first introduces a workaround at the vllm-ascend level. ### Does this PR introduce _any_ user-facing change? NA ### How was this patch tested? Local test - vLLM version: v0.23.0 - vLLM main: vllm-project/vllm@b9a7cd4 Signed-off-by: Tflowers-0129 <2906339855@qq.com>
…tmax (vllm-project#11391) ### What this PR does / why we need it? Based on previous community issues, we had already noticed that Qwen3.5-MoE might have a hidden accuracy issue. The root cause has now been identified: when the input token dimension of the `MoEGatingTopkSoftmax` operator is 2048, UB overflow may occur, which can eventually produce INF values and lead to accuracy degradation. Since the operator-level fix may not be externally available, this PR first introduces a workaround at the vllm-ascend level. ### Does this PR introduce _any_ user-facing change? NA ### How was this patch tested? Local test - vLLM version: v0.23.0 - vLLM main: vllm-project/vllm@b9a7cd4 Signed-off-by: Tflowers-0129 <2906339855@qq.com>



What this PR does / why we need it?
Based on previous community issues, we had already noticed that Qwen3.5-MoE might have a hidden accuracy issue. The root cause has now been identified: when the input token dimension of the
MoEGatingTopkSoftmaxoperator is 2048, UB overflow may occur, which can eventually produce INF values and lead to accuracy degradation.Since the operator-level fix may not be externally available, this PR first introduces a workaround at the vllm-ascend level.
Does this PR introduce any user-facing change?
NA
How was this patch tested?
Local test