[CI/Build][Bugfix] Fix Qwen2.5 tests in CPU CI via fallback silu_and_mul to torch native implementation #22145
Conversation
Signed-off-by: jiang1.li <jiang1.li@intel.com>
|
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels. Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging. To run CI, PR reviewers can either: Add 🚀 |
![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 addresses a crash in the SiluAndMul activation on CPU when used with hidden dimensions that are not a multiple of the CPU vector size. The fix involves falling back to the native PyTorch implementation for the CPU platform, which correctly handles arbitrary dimensions. The change is correct and effectively resolves the reported issue for SiluAndMul. I've identified that several other activation functions in the same file suffer from the identical problem on CPU and have left a comment with a recommendation to apply a similar fix to those as well for consistency and to prevent future crashes.
| if current_platform.is_cuda_alike(): | ||
| self.op = torch.ops._C.silu_and_mul | ||
| elif current_platform.is_xpu(): | ||
| from vllm._ipex_ops import ipex_ops | ||
| self.op = ipex_ops.silu_and_mul | ||
| elif current_platform.is_cpu(): | ||
| self._forward_method = self.forward_native |
There was a problem hiding this comment.
While this change correctly fixes the crash for SiluAndMul on CPU with uneven dimensions, it is an incomplete fix for the file. Other activation functions in this file, such as GeluAndMul, NewGELU, FastGELU, and QuickGELU, still use custom CPU kernels with the same dimension constraints and will crash under similar conditions.
For example, GeluAndMul at line 207 has if current_platform.is_cuda_alike() or current_platform.is_cpu(): which will lead to the same crash.
To ensure consistent behavior and prevent future bugs, it's highly recommended to apply the same fix to these other activations within this PR. This will improve code quality and maintainability.
|
CPU tests pass |
…mul to torch native implementation (vllm-project#22145) Signed-off-by: jiang1.li <jiang1.li@intel.com>
…mul to torch native implementation (vllm-project#22145) Signed-off-by: jiang1.li <jiang1.li@intel.com> Signed-off-by: Jinzhen Lin <linjinzhen@hotmail.com>
…mul to torch native implementation (vllm-project#22145) Signed-off-by: jiang1.li <jiang1.li@intel.com> Signed-off-by: Noam Gat <noamgat@gmail.com>
…mul to torch native implementation (vllm-project#22145) Signed-off-by: jiang1.li <jiang1.li@intel.com> Signed-off-by: Paul Pak <paulpak58@gmail.com>
…mul to torch native implementation (vllm-project#22145) Signed-off-by: jiang1.li <jiang1.li@intel.com> Signed-off-by: Diego-Castan <diego.castan@ibm.com>
…mul to torch native implementation (vllm-project#22145) Signed-off-by: jiang1.li <jiang1.li@intel.com>
…mul to torch native implementation (vllm-project#22145) Signed-off-by: jiang1.li <jiang1.li@intel.com>
3 participants
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.Purpose
silu_and_mul, fallback it to torch native implementation. For torch.compile, there will be no difference. For eager mode, the performance drop will be very slight.Test Plan
Test Result
(Optional) Documentation Update