Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Misc] Use ray[adag] dependency instead of cuda #7938

Merged
merged 3 commits into from
Sep 6, 2024
Merged

[Misc] Use ray[adag] dependency instead of cuda #7938

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
27cfe45
[Misc] Use ray[adag] dependency instead of cuda
ruisearch42 Aug 28, 2024
bc27bf9
up
ruisearch42 Aug 28, 2024
fdc63f6
up
ruisearch42 Sep 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ WORKDIR /workspace

# install build and runtime dependencies
COPY requirements-common.txt requirements-common.txt
COPY requirements-adag.txt requirements-adag.txt
COPY requirements-cuda.txt requirements-cuda.txt
RUN --mount=type=cache,target=/root/.cache/pip \
python3 -m pip install -r requirements-cuda.txt
Expand Down Expand Up @@ -66,7 +65,6 @@ COPY setup.py setup.py
COPY cmake cmake
COPY CMakeLists.txt CMakeLists.txt
COPY requirements-common.txt requirements-common.txt
COPY requirements-adag.txt requirements-adag.txt
COPY requirements-cuda.txt requirements-cuda.txt
COPY pyproject.toml pyproject.toml
COPY vllm vllm
Expand Down
1 change: 0 additions & 1 deletion MANIFEST.in
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
include LICENSE
include requirements-adag.txt
include requirements-common.txt
include requirements-cuda.txt
include requirements-rocm.txt
Expand Down
3 changes: 0 additions & 3 deletions requirements-adag.txt
View file
Open in desktop

This file was deleted.

5 changes: 1 addition & 4 deletions requirements-test.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
# Needed for Ray accelerated DAG tests
-r requirements-adag.txt

# testing
pytest
tensorizer>=2.9.0
Expand All @@ -16,7 +13,7 @@ httpx
librosa # required for audio test
peft
requests
ray
ray[adag]>=2.35
sentence-transformers # required for embedding
soundfile # required for audio test
compressed-tensors==0.4.0 # required for compressed-tensors
Expand Down
2 changes: 1 addition & 1 deletion vllm/executor/ray_gpu_executor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ def _compiled_ray_dag(self, enable_asyncio: bool):
import pkg_resources
from packaging import version

required_version = version.parse("2.32")
required_version = version.parse("2.35")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is not enough? It is possible users use 2.35 but ray[adag] is not downloaded

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This path is used in CI, and CI should be able to catch it?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am talking about more of end user perspective

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is not sufficient error messgae. We should 1. detect ray[adag] is not downloaded & and raise an error message users should download pip install ray[adag]. With the current code, if users download pip install ray==2.35, it passes this part and will have weird dependency error.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I tested with checking "ray[adag]" version last week and it did not make a difference, so I kept this. Let me look further.

current_version = version.parse(
pkg_resources.get_distribution("ray").version)
if current_version < required_version:
Expand Down
Loading