Repository navigation
Conversation
Signed-off-by: yuezhitangxi <3542814897@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 enhances the project's CI infrastructure by adding a dedicated CPU-only build verification path. This ensures that the package remains buildable and dependency-consistent even in environments lacking NPU hardware or the CANN toolkit. Additionally, it cleans up the dependency requirements by removing an optional plugin that was causing installation issues in restricted environments, providing clearer installation guidance for users. 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 introduces a reproducible CPU-only build and verification environment by adding Dockerfile.cpu and tools/cpu_only_build.sh. It also removes arctic-inference from the core requirements, documenting it as an optional plugin instead. Feedback suggests updating the PR title and summary to align with the repository's style guide, and improving the device verification script to check for any /dev/davinci* device rather than just /dev/davinci0 to prevent false negatives.
|
|
||
| # This image is intentionally free of CANN and NPU device support. It is a | ||
| # reproducible package build/verification environment, not an inference image. | ||
| FROM openeuler/openeuler:24.03-lts |
There was a problem hiding this comment.
Here is the suggested PR Title and Summary according to the repository style guide:
Suggested PR Title:
[CI][Feature] Add reproducible CPU-only build verificationSuggested PR Summary:
### What this PR does / why we need it?
This PR introduces a reproducible CPU-only build verification path to ensure that `vllm-ascend` can be built and verified in environments without CANN or NPU device support.
Specifically, it:
- Adds `Dockerfile.cpu` based on openEuler 24.03 LTS.
- Implements `tools/cpu_only_build.sh` to handle dependency installation, package building, and CPU-only verification.
- Removes `arctic-inference==0.1.1` from the universal requirements (`pyproject.toml` and `requirements.txt`) since its source build requires PyTorch 2.7, making it incompatible with the current CPU stack. It is now documented as an optional plugin.
- Updates documentation to reflect that `arctic-inference` is optional and should be installed separately.
### Does this PR introduce _any_ user-facing change?
No, this is a CI/build-system enhancement and documentation update. There are no user-facing API or behavioral changes to `vllm-ascend` itself, other than making `arctic-inference` an optional dependency.
### How was this patch tested?
The changes were validated via:
- CPU-only Docker image build verification.
- Dependency installation and package build with `COMPILE_CUSTOM_KERNELS=0`.
- Verification of CPU-only environment (no NPU/CANN) and successful `pip check`.References
- Adhere to the Pull Request Summary Style Guide for formatting the PR Title and Summary. (link)
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: ChenDAO1 <97371942+ChenDAO1@users.noreply.github.com>
| memfabric_hybrid | ||
| memcache_hybrid | ||
|
|
||
| arctic-inference==0.1.1 |
There was a problem hiding this comment.
Make sure the test case does not include it; otherwise, direct deletion is not recommended.
There was a problem hiding this comment.
I will Keeps arctic-inference==0.1.1 in the normal vLLM Ascend dependency contract, while excluding it only from the CPU-only build verification path because it is incompatible with the CPU-only PyTorch build environment.
| - main | ||
| - '*-dev' | ||
| - 'releases/v*' | ||
| paths: |
There was a problem hiding this comment.
Please determine the scope in which this test is triggered. At minimum, I believe the following items should not be included in the trigger scope; otherwise, this test may become a per‑pr test, while its intended purpose is to perform validation only when specific content is modified.
- 'vllm_ascend/**'
- 'tests/ut/test_dependency_documentation.py'
- '.github/workflows/scripts/test_config.yaml'
- '.github/workflows/scripts/select_tests.py'
- 'docs/source/installation.md'
- 'docs/source/tutorials/features/suffix_speculative_decoding.md'
- 'docs/source/user_guide/feature_guide/speculative_decoding.md'There was a problem hiding this comment.
Thanks for pointing this out. I agree that the current path filter is broader than necessary and may cause the CPU-only build verification to run for unrelated PRs. I'll narrow the trigger scope to dependency/build-related files. For vllm_ascend/**, I'll keep only the source files that are directly imported during package build, such as vllm_ascend/envs.py and vllm_ascend/device/hardware.py.
Signed-off-by: ChenDAO1 <alkaidb3v.chen@gmail.com>
Signed-off-by: ChenDAO1 <alkaidb3v.chen@gmail.com>
Signed-off-by: ChenDAO1 <97371942+ChenDAO1@users.noreply.github.com>
|
/rerun Failed:
|
|
/cancel https://github.com/vllm-project/vllm-ascend/actions/runs/32207228102 |
|
/rerun Failed:
|
| jobs: | ||
| build: | ||
| name: Build and smoke-test CPU image | ||
| runs-on: ubuntu-latest |
There was a problem hiding this comment.
Plz use self-host runner, don't use github runner.
There was a problem hiding this comment.
already fixed this
Signed-off-by: ChenDAO1 <alkaidb3v.chen@gmail.com>
Signed-off-by: ChenDAO1 <alkaidb3v.chen@gmail.com>
Signed-off-by: ChenDAO1 <alkaidb3v.chen@gmail.com>
Signed-off-by: ChenDAO1 <alkaidb3v.chen@gmail.com>
Signed-off-by: ChenDAO1 <alkaidb3v.chen@gmail.com>
Signed-off-by: ChenDAO1 <alkaidb3v.chen@gmail.com>
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
What this PR does / why we need it?
This PR adds a reproducible CPU-only package build and verification path for
vllm-ascend. It validates that the package can be installed and built without Ascend NPU devices or the CANN toolkit, while preserving the normal NPU/runtime dependency contract.Specifically, this PR:
Dockerfile.cpu, based on openEuler 24.03 LTS, as a clean CPU-only build environment;tools/cpu_only_build.shas the executable source of truth for dependency installation, package build, and verification;+cpuPyTorch wheels while resolving their generic dependencies from the main package index, then installs the completepyproject.tomlbuild-system requirements before building the package;COMPILE_CUSTOM_KERNELS=0, an explicitSOC_VERSION, andTORCH_DEVICE_BACKEND_AUTOLOAD=0, so the verification path does not require CANN or NPU hardware;arctic-inference==0.1.1inrequirements.txtand therefore in the normal vLLM Ascend runtime dependency contract;arctic-inference==0.1.1requirement from CPU-only verification because its source distribution is incompatible with the CPU-only build environment;setup.pygenerates editable-install package metadata, restoring the originalrequirements.txtthrough a shell trap on success or failure;pip checksucceeds;linux-amd64-cpu-4self-hosted runner;docker/build-push-action@v7, with CPU-only verification executed as a Dockerfile build layer so the job does not require a local Docker daemon ordocker run;arctic-inferencewas removed only frompyproject.tomlbuild-system requirements because it is not required to execute the package build backend. Normal installations still receive the exact runtime dependency throughrequirements.txtandsetup.py.Does this PR introduce any user-facing change?
No user-facing API or NPU runtime behavior changes are introduced.
Normal vLLM Ascend installations continue to include
arctic-inference==0.1.1, and suffix speculative decoding keeps its existing dependency. The Arctic Inference exclusion is local to the CPU-only CI verification path and does not affect normal NPU installations.How was this patch tested?
The patch was validated with:
bash -n tools/cpu_only_build.sh;git diff --check;actionlint for
.github/workflows/cpu_only_build.yaml;python -m unittest tests.ut.test_dependency_documentation -v;CPU-only dependency, build, metadata, device/CANN absence, Arctic Inference absence, and
pip checkverification encoded inDockerfile.cpuandtools/cpu_only_build.sh;the public SWR openEuler manifest and the DNF repository rewrite used by the self-hosted BuildKit path;
GitHub Actions CPU-only BuildKit verification passed on the current PR head; the repository E2E selected tests were also triggered.
vLLM version: v0.27.1
vLLM main: vllm-project/vllm@58d3918