[BugFix][310p] Fix torch-npu cannot import error - #9249
Conversation
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 addresses an import error related to 'torch-npu' by removing the explicit installation of 'triton-ascend' during the Docker image construction. This change ensures a cleaner environment and prevents dependency conflicts that were causing runtime issues. 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. Ignored Files
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. |
There was a problem hiding this comment.
Code Review
This pull request removes the installation of triton-ascend from the Dockerfile.310p and Dockerfile.310p.openEuler files to address potential torch-npu import issues. Feedback from the reviewer indicates that the PR title and summary need to be updated to comply with the repository's style guide. Furthermore, it is recommended to reorder the packages in the pip uninstall command to maintain consistency with other Dockerfiles in the repository.
| python3 -m pip install -e /vllm-workspace/vllm-ascend/ --extra-index https://download.pytorch.org/whl/cpu/ && \ | ||
| python3 -m pip uninstall -y triton-ascend triton&& \ | ||
| python3 -m pip install triton-ascend==3.2.1 --extra-index-url https://triton-ascend.osinfra.cn/pypi/simple && \ | ||
| python3 -m pip uninstall -y triton-ascend triton && \ |
There was a problem hiding this comment.
The Pull Request title and summary do not adhere to the repository's style guide. The title should follow the [Branch][Module][Action] Title format, and the summary should include the required sections. Additionally, for consistency with other Dockerfiles in the repository (e.g., Dockerfile, Dockerfile.openEuler), the package order in the uninstall command should be updated.
Suggested PR Title:
[310p][BugFix] Fix torch-npu cannot import errorSuggested PR Summary:
### What this PR does / why we need it?
This PR removes the installation of `triton-ascend` in the Dockerfiles for Ascend 310P. This change addresses an issue where `torch-npu` fails to import, likely due to incompatibilities or conflicts introduced by the `triton-ascend` package on this specific hardware architecture.
### Does this PR introduce _any_ user-facing change?
No. This is an internal change to the Docker image build process.
### How was this patch tested?
The change is intended to resolve a known import error in the 310P environment. python3 -m pip uninstall -y triton triton-ascend && \
References
- The PR title and summary must follow the specific format defined in the repository style guide (lines 12-47). (link)
| python3 -m pip install -e /vllm-workspace/vllm-ascend/ --extra-index https://download.pytorch.org/whl/cpu/ && \ | ||
| python3 -m pip uninstall -y triton-ascend triton&& \ | ||
| python3 -m pip install triton-ascend==3.2.1 --extra-index-url https://triton-ascend.osinfra.cn/pypi/simple && \ | ||
| python3 -m pip uninstall -y triton-ascend triton && \ |
Signed-off-by: Tflowers-0129 <2906339855@qq.com>
37d4236 to
066d950
Compare
### What this PR does / why we need it? Fixed the recent CI failure on Ascend 310P where `torch_npu` could not be imported. The root cause is related to the `torch-npu` 2.10.0 upgrade. After the upgrade, if a residual `triton` directory still exists in the environment, importing `torch_npu` may indirectly depend on `triton.language`. However, Triton is not supported on Ascend 310P and should be removed. In the CI environment, `triton` had been uninstalled, but the cleanup was incomplete because of the uninstall order. We need to uninstall `triton-ascend` first and then uninstall `triton`; otherwise, some Triton-related files may remain. The correct cleanup order is: ```bash pip uninstall -y triton-ascend pip uninstall -y triton ``` ### Does this PR introduce _any_ user-facing change? NA ### How was this patch tested? CI - vLLM version: v0.20.2 - vLLM main: vllm-project/vllm@0d4d334 --------- Signed-off-by: Tflowers-0129 <2906339855@qq.com> Signed-off-by: Tian <tt553093031@gmail.com>
### What this PR does / why we need it? Fixed the recent CI failure on Ascend 310P where `torch_npu` could not be imported. The root cause is related to the `torch-npu` 2.10.0 upgrade. After the upgrade, if a residual `triton` directory still exists in the environment, importing `torch_npu` may indirectly depend on `triton.language`. However, Triton is not supported on Ascend 310P and should be removed. In the CI environment, `triton` had been uninstalled, but the cleanup was incomplete because of the uninstall order. We need to uninstall `triton-ascend` first and then uninstall `triton`; otherwise, some Triton-related files may remain. The correct cleanup order is: ```bash pip uninstall -y triton-ascend pip uninstall -y triton ``` ### Does this PR introduce _any_ user-facing change? NA ### How was this patch tested? CI - vLLM version: v0.20.2 - vLLM main: vllm-project/vllm@0d4d334 --------- Signed-off-by: Tflowers-0129 <2906339855@qq.com> Signed-off-by: Tian <tt553093031@gmail.com>
### What this PR does / why we need it? Fixed the recent CI failure on Ascend 310P where `torch_npu` could not be imported. The root cause is related to the `torch-npu` 2.10.0 upgrade. After the upgrade, if a residual `triton` directory still exists in the environment, importing `torch_npu` may indirectly depend on `triton.language`. However, Triton is not supported on Ascend 310P and should be removed. In the CI environment, `triton` had been uninstalled, but the cleanup was incomplete because of the uninstall order. We need to uninstall `triton-ascend` first and then uninstall `triton`; otherwise, some Triton-related files may remain. The correct cleanup order is: ```bash pip uninstall -y triton-ascend pip uninstall -y triton ``` ### Does this PR introduce _any_ user-facing change? NA ### How was this patch tested? CI - vLLM version: v0.20.2 - vLLM main: vllm-project/vllm@0d4d334 --------- Signed-off-by: Tflowers-0129 <2906339855@qq.com> Signed-off-by: 李少鹏 <lishaopeng21@huawei.com>
### What this PR does / why we need it? Fixed the recent CI failure on Ascend 310P where `torch_npu` could not be imported. The root cause is related to the `torch-npu` 2.10.0 upgrade. After the upgrade, if a residual `triton` directory still exists in the environment, importing `torch_npu` may indirectly depend on `triton.language`. However, Triton is not supported on Ascend 310P and should be removed. In the CI environment, `triton` had been uninstalled, but the cleanup was incomplete because of the uninstall order. We need to uninstall `triton-ascend` first and then uninstall `triton`; otherwise, some Triton-related files may remain. The correct cleanup order is: ```bash pip uninstall -y triton-ascend pip uninstall -y triton ``` ### Does this PR introduce _any_ user-facing change? NA ### How was this patch tested? CI - vLLM version: v0.20.2 - vLLM main: vllm-project/vllm@0d4d334 --------- Signed-off-by: Tflowers-0129 <2906339855@qq.com>
### What this PR does / why we need it? Fixed the recent CI failure on Ascend 310P where `torch_npu` could not be imported. The root cause is related to the `torch-npu` 2.10.0 upgrade. After the upgrade, if a residual `triton` directory still exists in the environment, importing `torch_npu` may indirectly depend on `triton.language`. However, Triton is not supported on Ascend 310P and should be removed. In the CI environment, `triton` had been uninstalled, but the cleanup was incomplete because of the uninstall order. We need to uninstall `triton-ascend` first and then uninstall `triton`; otherwise, some Triton-related files may remain. The correct cleanup order is: ```bash pip uninstall -y triton-ascend pip uninstall -y triton ``` ### Does this PR introduce _any_ user-facing change? NA ### How was this patch tested? CI - vLLM version: v0.20.2 - vLLM main: vllm-project/vllm@0d4d334 --------- Signed-off-by: Tflowers-0129 <2906339855@qq.com>
### What this PR does / why we need it? Fixed the recent CI failure on Ascend 310P where `torch_npu` could not be imported. The root cause is related to the `torch-npu` 2.10.0 upgrade. After the upgrade, if a residual `triton` directory still exists in the environment, importing `torch_npu` may indirectly depend on `triton.language`. However, Triton is not supported on Ascend 310P and should be removed. In the CI environment, `triton` had been uninstalled, but the cleanup was incomplete because of the uninstall order. We need to uninstall `triton-ascend` first and then uninstall `triton`; otherwise, some Triton-related files may remain. The correct cleanup order is: ```bash pip uninstall -y triton-ascend pip uninstall -y triton ``` ### Does this PR introduce _any_ user-facing change? NA ### How was this patch tested? CI - vLLM version: v0.20.2 - vLLM main: vllm-project/vllm@0d4d334 --------- Signed-off-by: Tflowers-0129 <2906339855@qq.com>
### What this PR does / why we need it? Fixed the recent CI failure on Ascend 310P where `torch_npu` could not be imported. The root cause is related to the `torch-npu` 2.10.0 upgrade. After the upgrade, if a residual `triton` directory still exists in the environment, importing `torch_npu` may indirectly depend on `triton.language`. However, Triton is not supported on Ascend 310P and should be removed. In the CI environment, `triton` had been uninstalled, but the cleanup was incomplete because of the uninstall order. We need to uninstall `triton-ascend` first and then uninstall `triton`; otherwise, some Triton-related files may remain. The correct cleanup order is: ```bash pip uninstall -y triton-ascend pip uninstall -y triton ``` ### Does this PR introduce _any_ user-facing change? NA ### How was this patch tested? CI - vLLM version: v0.20.2 - vLLM main: vllm-project/vllm@0d4d334 --------- Signed-off-by: Tflowers-0129 <2906339855@qq.com>
What this PR does / why we need it?
Fixed the recent CI failure on Ascend 310P where
torch_npucould not be imported.The root cause is related to the
torch-npu2.10.0 upgrade. After the upgrade, if a residualtritondirectory still exists in the environment, importingtorch_npumay indirectly depend ontriton.language. However, Triton is not supported on Ascend 310P and should be removed.In the CI environment,
tritonhad been uninstalled, but the cleanup was incomplete because of the uninstall order. We need to uninstalltriton-ascendfirst and then uninstalltriton; otherwise, some Triton-related files may remain.The correct cleanup order is:
Does this PR introduce any user-facing change?
NA
How was this patch tested?
CI