-
Notifications
You must be signed in to change notification settings - Fork 458
refactor linear #2867
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor linear #2867
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. |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Warning Gemini encountered an error creating the review. You can try again by commenting |
4b069b1
to
541645c
Compare
We have discussed a refactoring plan that will encapsulate the differing parts while keeping the patch sections unchanged. |
5999110
to
5e45c5d
Compare
@weijinqian0 This PR is ready for review now |
81c6988
to
61bae75
Compare
e665f8e
to
6e872e7
Compare
Signed-off-by: realliujiaxu <realliujiaxu@163.com> Co-authored-by: weijinqian0 <weijinqian@huawei.com>
Signed-off-by: realliujiaxu <realliujiaxu@163.com>
Signed-off-by: realliujiaxu <realliujiaxu@163.com>
6e872e7
to
3ff6204
Compare
Signed-off-by: realliujiaxu <realliujiaxu@163.com>
Signed-off-by: realliujiaxu <realliujiaxu@163.com>
Signed-off-by: realliujiaxu <realliujiaxu@163.com>
approved |
3. Override the apply method according to requirements, which will replace the original linear.forward | ||
4. Add selection logic for MyColumnParallelOp in the get_column_parallel_op method, typically based on prefix and configuration judgments | ||
Row parallel op follows a similar approach - inherit from RowColumnParallelOp and register the new class in get_row_parallel_op. | ||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice description
What this PR does / why we need it?
The current linear.py has the following issues:
There is redundant conditional logic in the
comm_group
andforward
selection for classes such asAscendMergedColumnParallelLinear
.Inconsistent comm_group selection logic exists among
AscendMergedColumnParallelLinear
,AscendColumnParallelLinear
, andAscendQKVParallelLinear
.To address these two issues, this PR encapsulates
comm_group
andforward
into classes and extracts the classes selection logic into common functions. For future additions of custom communication groups or forward methods, it will only be necessary to extendCustomColumnParallelOp
orCustomRowParallelOp
and add new selection logic.Does this PR introduce any user-facing change?
No
How was this patch tested?