[BugFix]A2G3 adapted for the CH version of HDK - #11504
Conversation
Signed-off-by: ZT-AIA <1028681969@qq.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request updates the chip detection logic in setup.py to support the A2G3 variant of the HDK. This ensures that VLLM ASCEND can be correctly configured and installed in environments utilizing the overseas version of the HDK. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
👋 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. Tip 💡 Consider Linking a Related Issue or RFCYour PR title contains the [BugFix] tag, indicating a bug fix or new feature. Linking a related issue or RFC in the PR description is strongly encouraged — it gives reviewers helpful context and speeds up the review. You can use any of these keywords:
🙏 Thanks for helping us keep the project well-organized! |
There was a problem hiding this comment.
Code Review
This pull request adds support for identifying the 'a2g3' chip name in setup.py and mapping it to 'ascend910b1'. The reviewer pointed out a contradiction in the code comment, which refers to the 'CH version of the HDK' instead of the 'overseas version of HDK A2G3' as described in the PR, and suggested a correction.
| elif "a2g3" in chip_name.lower(): | ||
| # A2 case: CH version of the HDK | ||
| return "ascend910b1" |
There was a problem hiding this comment.
The comment on line 122 refers to the "CH version of the HDK", but the PR description states that this change is specifically to adapt for the "overseas version of HDK A2G3". This contradiction is misleading and could cause confusion for future maintainers. Please update the comment to refer to the "overseas version" to align with the actual intent of this PR.
Following the Repository Style Guide (Pull Request Summary Style Guide), here are the suggested PR Title and PR Summary:
Suggested PR Title:
[Ops][BugFix] Adapt A2G3 for the overseas version of HDKSuggested PR Summary:
### What this PR does / why we need it?
This PR adds support for automatically identifying the chip type on environments using the overseas version of HDK A2G3. When the chip name contains "a2g3", it correctly identifies and returns "ascend910b1" as the SOC version, ensuring that vLLM Ascend can be properly installed and run.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
The installation and operation of vLLM Ascend in the environment of the overseas version of HDK must be carried out. After self-verification confirms no issues, the results still need to be provided by the testers.| elif "a2g3" in chip_name.lower(): | |
| # A2 case: CH version of the HDK | |
| return "ascend910b1" | |
| elif "a2g3" in chip_name.lower(): | |
| # A2 case: overseas version of the HDK | |
| return "ascend910b1" |
References
- The PR Title and PR Summary must follow the specific format defined in the Repository Style Guide. (link)
What this PR does / why we need it?
VLLM ASCEND is adapted for the overseas version of HDK A2G3. To ensure that VLLM ASCEND can be properly installed and used in scenarios where the overseas version of HDK is being used.
Does this PR introduce any user-facing change?
No
How was this patch tested?
the installation and operation of VLLM ASCEND in the environment of the overseas version of HDK must be carried out. After self-verification confirms no issues, the results still need to be provided by the testers.