-
Notifications
You must be signed in to change notification settings - Fork 451
[Fixbug] Fix soc_version for 310p #2676
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
base: main
Are you sure you want to change the base?
Conversation
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.
Code Review
This pull request correctly fixes a bug for Ascend 310p devices by adding support for its soc_version
. The changes properly identify the new soc_version
and configure the appropriate MoE communication method. I have one high-severity suggestion to improve maintainability by replacing a magic number with a named constant.
1cfe08d
to
534cce2
Compare
👋 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. |
I don't like this way, how about refactor to build_info way totally? |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2676 +/- ##
==========================================
+ Coverage 72.61% 73.84% +1.22%
==========================================
Files 154 155 +1
Lines 21319 21338 +19
==========================================
+ Hits 15480 15756 +276
+ Misses 5839 5582 -257
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
same issue! Want this branch merge! |
534cce2
to
f8c9848
Compare
It's impossible to |
d145a4d
to
a4662dc
Compare
a4662dc
to
5fc0f77
Compare
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
5fc0f77
to
554427a
Compare
bb4559a
to
c002951
Compare
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Signed-off-by: hfadzxy <[email protected]>
c002951
to
577ac6b
Compare
Signed-off-by: hfadzxy <[email protected]>
577ac6b
to
244e252
Compare
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
What this PR does / why we need it?
Does this PR introduce any user-facing change?
Users can use 310p nomarlly
How was this patch tested?