bugfix for glm #2872
bugfix for glm #2872
Conversation
Signed-off-by: Shanshan Shen <87969357+shen-shanshan@users.noreply.github.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. |
![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 appears to be a significant refactoring of the Fused MoE implementation for Ascend, removing a large amount of custom logic. However, the refactoring seems incomplete and introduces critical issues. The new fused_experts function is imported but not defined in the module it's imported from, which will cause an ImportError. Additionally, a call to select_experts has an incorrect number of variables for unpacking its return values, which will lead to a ValueError. I've provided comments on these critical issues with suggestions for fixes.
| AlltoAllCommImpl, MC2CommImpl) | ||
| from vllm_ascend.ops.moe.token_dispatcher import setup_token_dispatchers | ||
| from vllm_ascend.utils import ACL_FORMAT_FRACTAL_NZ, is_310p | ||
| from vllm_ascend.ops.fused_moe import fused_experts, select_experts |
There was a problem hiding this comment.
The function fused_experts is imported from vllm_ascend.ops.fused_moe, but it is not defined in the provided vllm_ascend/ops/fused_moe.py file. This will cause an ImportError at runtime. Please ensure that fused_experts is correctly defined and exposed in vllm_ascend/ops/fused_moe.py or that the import path is corrected.
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
1 participant
What this PR does / why we need it?
Does this PR introduce any user-facing change?
How was this patch tested?