Repository navigation
Revert "GMM custom operator optimization in small batch scenarios (vllm-project#7100)" - #7557
Conversation
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 performs a full revert of a prior commit that aimed to optimize Grouped Matrix Multiplication (GMM) custom operators for small batch sizes. The revert removes the specialized logic and operator calls that were conditionally applied based on batch size, restoring the system to its state before the optimization was introduced. This change ensures consistent behavior across all batch sizes by removing the small-batch specific implementations. Highlights
🧠 New Feature in Public Preview: You can now enable Memory 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 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 counter productive. 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
|
SunnyLee151064
changed the title
9ae0a1c to
eaa3cff
Compare
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request reverts commit 42bcad7e9b68cf92d9b98a9072eff10edc8d6f33, which introduced an optimization for GMM custom operators in small batch scenarios. The changes remove the specialized code paths for small batches across C++ and Python files.
While the code changes correctly perform the revert, the pull request description is currently empty. This violates the repository's style guide (lines 12-40), which requires a clear explanation of the changes. For a revert, it is critical to document why the original changes are being undone (e.g., correctness issues, performance regressions). Additionally, the pull request title does not follow the prescribed format (lines 41-61). Please update the description and title to provide this crucial context for reviewers and future reference.
Suggested PR Title:
[Ops][BugFix] Revert GMM custom operator optimization in small batch scenariosSuggested PR Summary:
### What this PR does / why we need it?
This PR reverts commit 42bcad7e9b68cf92d9b98a9072eff10edc8d6f33.
The reverted commit introduced an optimization for the GMM custom operator in small batch scenarios. This revert is necessary because **[PLEASE FILL IN THE REASON FOR THE REVERT, e.g., it caused correctness issues under certain conditions or led to performance regressions in other scenarios.]**
This change removes the specialized code paths for small batches and restores the previous, stable implementation.
### Does this PR introduce _any_ user-facing change?
No. This change fixes an issue with a previous optimization and is not expected to introduce any user-facing changes, other than restoring correct behavior and/or performance.
### How was this patch tested?
CI passed. As this is a revert to a previously validated state, existing tests are sufficient to ensure correctness.
…lm-project#7100)" This reverts commit 42bcad7. Signed-off-by: Your Name <you@example.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. |
MengqingCao
changed the title
…lm-project#7100)" (vllm-project#7557) ### What this PR does / why we need it? This reverts commit 42bcad7. The commit cause accuracy decrease of qwen3Next, 150 items of gsm8k, 98 -> 91. - vLLM version: v0.18.0 - vLLM main: vllm-project/vllm@6a9cceb Signed-off-by: Your Name <you@example.com> Co-authored-by: Your Name <you@example.com>
…lm-project#7100)" (vllm-project#7557) ### What this PR does / why we need it? This reverts commit 42bcad7. The commit cause accuracy decrease of qwen3Next, 150 items of gsm8k, 98 -> 91. - vLLM version: v0.18.0 - vLLM main: vllm-project/vllm@6a9cceb Signed-off-by: Your Name <you@example.com> Co-authored-by: Your Name <you@example.com>
…lm-project#7100)" (vllm-project#7557) ### What this PR does / why we need it? This reverts commit 42bcad7. The commit cause accuracy decrease of qwen3Next, 150 items of gsm8k, 98 -> 91. - vLLM version: v0.18.0 - vLLM main: vllm-project/vllm@6a9cceb Signed-off-by: Your Name <you@example.com> Co-authored-by: Your Name <you@example.com>
…lm-project#7100)" (vllm-project#7557) ### What this PR does / why we need it? This reverts commit 42bcad7. The commit cause accuracy decrease of qwen3Next, 150 items of gsm8k, 98 -> 91. - vLLM version: v0.18.0 - vLLM main: vllm-project/vllm@6a9cceb Signed-off-by: Your Name <you@example.com> Co-authored-by: Your Name <you@example.com>
…lm-project#7100)" (vllm-project#7557) ### What this PR does / why we need it? This reverts commit 42bcad7. The commit cause accuracy decrease of qwen3Next, 150 items of gsm8k, 98 -> 91. - vLLM version: v0.18.0 - vLLM main: vllm-project/vllm@6a9cceb Signed-off-by: Your Name <you@example.com> Co-authored-by: Your Name <you@example.com> Signed-off-by: nanxing <1014662416@qq.com>
…lm-project#7100)" (vllm-project#7557) ### What this PR does / why we need it? This reverts commit 42bcad7. The commit cause accuracy decrease of qwen3Next, 150 items of gsm8k, 98 -> 91. - vLLM version: v0.18.0 - vLLM main: vllm-project/vllm@6a9cceb Signed-off-by: Your Name <you@example.com> Co-authored-by: Your Name <you@example.com>
…lm-project#7100)" (vllm-project#7557) ### What this PR does / why we need it? This reverts commit fa4f8f6. The commit cause accuracy decrease of qwen3Next, 150 items of gsm8k, 98 -> 91. - vLLM version: v0.18.0 - vLLM main: vllm-project/vllm@6a9cceb Signed-off-by: Your Name <you@example.com> Co-authored-by: Your Name <you@example.com>
### What this PR does / why we need it? The op has had no callers since #7557 reverted the small-batch GMM optimization introduced in #7100 (qwen3-next gsm8k 98 -> 91). All grouped-matmul paths run on torch_npu.npu_grouped_matmul. - delete csrc/moe/moe_grouped_matmul/ (op_host, op_kernel, op_api) - drop schema/impl registration from csrc/torch_binding.cpp - drop meta registration from csrc/torch_binding_meta.cpp - drop the op from csrc/build_aclnn.sh build lists (ascend910b/910_93) ### Does this PR introduce _any_ user-facing change? Yes, the `moe_grouped_matmul` operator is no longer available in the PyTorch bindings. ### How was this patch tested? No new tests were added as this is a code removal PR. Existing CI tests should pass. - vLLM main: vllm-project/vllm@b2f6858 Signed-off-by: TangPeng <85704592@qq.com>
### What this PR does / why we need it? The op has had no callers since vllm-project#7557 reverted the small-batch GMM optimization introduced in vllm-project#7100 (qwen3-next gsm8k 98 -> 91). All grouped-matmul paths run on torch_npu.npu_grouped_matmul. - delete csrc/moe/moe_grouped_matmul/ (op_host, op_kernel, op_api) - drop schema/impl registration from csrc/torch_binding.cpp - drop meta registration from csrc/torch_binding_meta.cpp - drop the op from csrc/build_aclnn.sh build lists (ascend910b/910_93) ### Does this PR introduce _any_ user-facing change? Yes, the `moe_grouped_matmul` operator is no longer available in the PyTorch bindings. ### How was this patch tested? No new tests were added as this is a code removal PR. Existing CI tests should pass. - vLLM main: vllm-project/vllm@b2f6858 Signed-off-by: TangPeng <85704592@qq.com>
### What this PR does / why we need it? The op has had no callers since vllm-project#7557 reverted the small-batch GMM optimization introduced in vllm-project#7100 (qwen3-next gsm8k 98 -> 91). All grouped-matmul paths run on torch_npu.npu_grouped_matmul. - delete csrc/moe/moe_grouped_matmul/ (op_host, op_kernel, op_api) - drop schema/impl registration from csrc/torch_binding.cpp - drop meta registration from csrc/torch_binding_meta.cpp - drop the op from csrc/build_aclnn.sh build lists (ascend910b/910_93) ### Does this PR introduce _any_ user-facing change? Yes, the `moe_grouped_matmul` operator is no longer available in the PyTorch bindings. ### How was this patch tested? No new tests were added as this is a code removal PR. Existing CI tests should pass. - vLLM main: vllm-project/vllm@b2f6858 Signed-off-by: TangPeng <85704592@qq.com> Signed-off-by: tianming2009 <13246728590@163.com>
### What this PR does / why we need it? The op has had no callers since vllm-project#7557 reverted the small-batch GMM optimization introduced in vllm-project#7100 (qwen3-next gsm8k 98 -> 91). All grouped-matmul paths run on torch_npu.npu_grouped_matmul. - delete csrc/moe/moe_grouped_matmul/ (op_host, op_kernel, op_api) - drop schema/impl registration from csrc/torch_binding.cpp - drop meta registration from csrc/torch_binding_meta.cpp - drop the op from csrc/build_aclnn.sh build lists (ascend910b/910_93) ### Does this PR introduce _any_ user-facing change? Yes, the `moe_grouped_matmul` operator is no longer available in the PyTorch bindings. ### How was this patch tested? No new tests were added as this is a code removal PR. Existing CI tests should pass. - vLLM main: vllm-project/vllm@b2f6858 Signed-off-by: TangPeng <85704592@qq.com> Signed-off-by: like-0517 <ithwlike@126.com>
2 participants
What this PR does / why we need it?
This reverts commit 42bcad7. The commit cause accuracy decrease of qwen3Next, 150 items of gsm8k, 98 -> 91.