[Hardware] [Feat] Setup platform dependent package installation#1046
[Hardware] [Feat] Setup platform dependent package installation#1046Isotr0py merged 24 commits intovllm-project:mainfrom
Conversation
Co-authored-by: gcanlin <[email protected]> Co-authored-by: PopSoda2002 <[email protected]> Signed-off-by: tjtanaa <[email protected]>
Signed-off-by: tjtanaa <[email protected]>
Signed-off-by: tjtanaa <[email protected]>
Signed-off-by: tjtanaa <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7eb9c59675
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| device = detect_target_device() | ||
| requirements_dir = Path(__file__).parent / "requirements" | ||
| requirements_file = requirements_dir / f"{device}.txt" | ||
|
|
||
| print(f"Loading requirements from: {requirements_file}") |
There was a problem hiding this comment.
Ensure requirements/*.txt are packaged in sdist/wheel
This setup logic reads requirements/{device}.txt at build time, but the new requirements/ directory isn’t included in package data (only vllm_omni.model_executor.stage_configs is listed) and there’s no MANIFEST.in, so source distributions will omit those files. In that case load_requirements returns an empty list and the built wheel installs with no runtime dependencies, which breaks pip install from sdist (e.g., when no wheel is available for a platform/Python). Consider adding the requirements files to the sdist/wheel inputs (MANIFEST.in or tool.setuptools package-data) or moving the lists back into pyproject.toml so they’re always available.
Useful? React with 👍 / 👎.
Signed-off-by: tjtanaa <[email protected]>
Co-authored-by: gcanlin <[email protected]> Signed-off-by: tjtanaa <[email protected]>
Signed-off-by: tjtanaa <[email protected]>
|
To reviewer: please help me to ensure all three authors are there. Thanks |
Signed-off-by: tjtanaa <[email protected]>
|
@david6666666 I have updated the PR, so that the |
|
CI failed. It looks like fa3-fwd didn't install successfully. Do we need |
|
|
||
|
|
||
| #### Installation of vLLM | ||
| Note: Pre-built wheels are currently only available for vLLM-Omni 0.11.0rc1, 0.12.0rc1, 0.14.0rc1. For the latest version, please [build from source](https://docs.vllm.ai/projects/vllm-omni/en/latest/getting_started/installation/gpu/#build-wheel-from-source). |
There was a problem hiding this comment.
This note seems should be put above Installation of vLLM.
There was a problem hiding this comment.
Done. I was following the instruction from CUDA. Since we can move this before the installation of vLLM, I have put it in the gpu.md.
Signed-off-by: tjtanaa <[email protected]>
Signed-off-by: tjtanaa <[email protected]>
I have added that to the ci dockerfile, let's see. |
My concern is that it will change the user behavior when installing. Is it acceptable? My original point is we avoid to introduce the extra flag like |
@gcanlin I think let's go for I am currently also working on helping to setup release pipeline, both docker image and wheel. I noticed that if we use If we go for Then for test-time dependencies, we can use the tag |
|
it's not optimal to require cuda users to use "pip install vllm-omni[cuda]" |
|
This PR will not be included in v0.14.0 for now; this will be explained in the documentation first, and a better method will be used later. |
Signed-off-by: tjtanaa <[email protected]>
Signed-off-by: tjtanaa <[email protected]>
pyproject.toml
Outdated
| "setuptools>=77.0.3,<81.0.0", | ||
| "wheel", | ||
| "setuptools-scm>=8.0", | ||
| "torch == 2.9.1", |
There was a problem hiding this comment.
Do we still need to include torch in building dependency? I think vLLM-Omni doesn't need to build pytorch kernels?
There was a problem hiding this comment.
We need to include torch if we want to make pip install . to install cuda dependencies as we are using if torch.version.cuda is not None: in the setup.py. Without torch in the pyproject.toml, if torch.version.cuda is not None: will be None and it will not install cuda dependencies.
I am thinking of proposing not to support CPU for now and always assume the fallback to be CUDA platform. Then we don't need to include torch into the pyproject.toml.
…on platform, add onnxruntime uninstallation in rocm path Signed-off-by: tjtanaa <[email protected]>
Signed-off-by: tjtanaa <[email protected]>
Signed-off-by: tjtanaa <[email protected]>
Signed-off-by: tjtanaa <[email protected]>
Signed-off-by: tjtanaa <[email protected]>
|
I have updated the PR description to reflect the current state of the code. Please take alook. |
Signed-off-by: tjtanaa <[email protected]>
Signed-off-by: tjtanaa <[email protected]>
Signed-off-by: tjtanaa <[email protected]>
Signed-off-by: tjtanaa <[email protected]>
Signed-off-by: tjtanaa <[email protected]>
…-project#1046) Signed-off-by: tjtanaa <[email protected]> Co-authored-by: PopSoda2002 <[email protected]> Co-authored-by: gcanlin <[email protected]> Signed-off-by: gerayking <[email protected]>
…-project#1046) Signed-off-by: tjtanaa <[email protected]> Co-authored-by: PopSoda2002 <[email protected]> Co-authored-by: gcanlin <[email protected]>
PLEASE FILL IN THE PR DESCRIPTION HERE ENSURING ALL CHECKLIST ITEMS (AT THE BOTTOM) HAVE BEEN CONSIDERED.
Purpose
This PR implements the RFC #997 .
It fixes #909 as well.
The dockerfiles and documentation has been updated to reflect this.
Current behaviour
Default behaviour (CUDA)
pip installanduv pip installdefault will install CUDA configuration.Other platforms
To install dependencies for other platform (CPU/ROCm/NPU/XPU), has to be installed using the following approach.
Test Plan
Validated locally on ROCm platform.
Test Result
Validated locally on ROCm platform that it works and we need
--no-build-isolationflag.CC @gcanlin @PopSoda2002 @david6666666
Simple bugfix
Fix the license deprecation warning
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.BEFORE SUBMITTING, PLEASE READ https://github.com/vllm-project/vllm-omni/blob/main/CONTRIBUTING.md (anything written below this line will be removed by GitHub Actions)