Skip to content

Revert "[Kernel] add custom moe ops for prefill"#4806

Merged
wangxiyuan merged 2 commits intomainfrom
revert-4194-add_moe_normal
Dec 8, 2025
Merged

Revert "[Kernel] add custom moe ops for prefill"#4806
wangxiyuan merged 2 commits intomainfrom
revert-4194-add_moe_normal

Conversation

@MengqingCao
Copy link
Copy Markdown
Collaborator

@MengqingCao MengqingCao commented Dec 8, 2025

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request reverts the addition of custom MoE ops for prefill, which reportedly broke the CI. The changes primarily consist of deleting the newly added files and removing the corresponding code from existing files. The revert appears to be correct and complete. I have one suggestion regarding shell script best practices in csrc/build_aclnn.sh to improve future maintainability and prevent potential bugs.

Comment thread csrc/build_aclnn.sh
rm -rf build output
echo "building custom ops $CUSTOM_OPS for $SOC_VERSION"
bash build.sh -n "$CUSTOM_OPS" -c "$SOC_ARG"
bash build.sh -n $CUSTOM_OPS -c $SOC_ARG
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

While this change is part of a revert to fix a CI failure, removing the quotes around $CUSTOM_OPS and $SOC_ARG is generally unsafe in shell scripting as it can lead to unexpected word splitting and globbing if the variables were to contain spaces or special characters. If these variables must be unquoted for the build.sh script to work correctly, it would be beneficial to add a comment explaining why. This would prevent future contributors from re-introducing quotes and breaking the build again. An even better long-term solution would be to make build.sh robust enough to handle quoted arguments correctly.

@wangxiyuan wangxiyuan merged commit 7e70da9 into main Dec 8, 2025
18 of 20 checks passed
@wangxiyuan
Copy link
Copy Markdown
Collaborator

Thanks

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Dec 8, 2025

👋 Hi! Thank you for contributing to the vLLM Ascend project. The following points will speed up your PR merge:‌‌

  • A PR should do only one thing, smaller PRs enable faster reviews.
  • Every PR should include unit tests and end-to-end tests ‌to ensure it works and is not broken by other future PRs.
  • Write the commit message by fulfilling the PR description to help reviewer and future developers understand.

If CI fails, you can run linting and testing checks locally according Contributing and Testing.

weijinqian0 pushed a commit to weijinqian0/vllm-ascend that referenced this pull request Dec 9, 2025
Reverts vllm-project#4194 as it broke CI in
https://github.com/vllm-project/vllm-ascend/actions/runs/20030369087/job/57437687382?pr=4791

Co-authored-by: wangxiyuan <wangxiyuan1007@gmail.com>
Signed-off-by: weijinqian_v1 <weijinqian@huawei.com>
Clorist33 pushed a commit to Clorist33/vllm-ascend that referenced this pull request Dec 10, 2025
Mercykid-bash pushed a commit to Mercykid-bash/vllm-ascend that referenced this pull request Dec 10, 2025
@wangxiyuan wangxiyuan deleted the revert-4194-add_moe_normal branch December 11, 2025 01:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants