Revert "[cherry-pick][refactor]support gatingtopk operator generaliza… #4353
Conversation
|
👋 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. |
…tion (vllm-project#4050)" This reverts commit c87a77e. Signed-off-by: 1092626063 <1092626063@qq.com>
1092626063
force-pushed
the
revertgatingtopk
branch
from
c16f78f to
59b309a
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 a previous commit that aimed to generalize the gatingtopk operator. The revert appears to be a necessary step, likely due to limitations discovered in the generalized operator. The changes correctly restore the logic to use more specialized operators for different expert selection scenarios, and the corresponding tests have been updated to reflect this. However, I've identified a potential critical issue in the restored code where two conditional blocks are not mutually exclusive, which could lead to incorrect behavior under certain conditions. My review includes a suggestion to fix this.
| # y2_flag=False, # old api; should the third output be output | ||
| routed_scaling_factor=1, | ||
| eps=float(1e-20)) | ||
| if not use_grouped_topk and custom_routing_function is None and scoring_func == "softmax": |
There was a problem hiding this comment.
The if block at line 183 for is_deepseek_v3_r1 and this if block for scoring_func == "softmax" are not mutually exclusive. If is_deepseek_v3_r1 is true and scoring_func also happens to be "softmax", this block will execute and overwrite the topk_weights and topk_ids calculated in the first block. This could lead to incorrect expert selection. To prevent this, this if should be converted to an elif to ensure only one of the specialized operator paths is taken.
| if not use_grouped_topk and custom_routing_function is None and scoring_func == "softmax": | |
| elif not use_grouped_topk and custom_routing_function is None and scoring_func == "softmax": |
|
see #4352 |
2 participants
…tion (#4050)"
This reverts commit c87a77e.
What this PR does / why we need it?
Does this PR introduce any user-facing change?
How was this patch tested?