[Bugfix] use_existing_torch.py: filter out comments #12255

tdoublep · 2025-01-21T09:01:16Z

The current use_existing_torch.py filters out any line that contains the word torch, but right now in requirements-commont.txt we have these lines:

outlines == 0.1.11 # Requires pytorch
compressed-tensors == 0.8.1 # required for compressed-tensors, requires pytorch

and this is causing the lines to get removed:

>>> cleaning requirements-common.txt
removed:
outlines == 0.1.11 # Requires pytorch
compressed-tensors == 0.8.1 # required for compressed-tensors, requires pytorch
<<< done cleaning requirements-common.txt

This is breaking some of our images that use custom pytorch version, because these dependencies end up not getting installed.

Signed-off-by: Thomas Parnell <[email protected]>

github-actions · 2025-01-21T09:01:27Z

👋 Hi! Thank you for contributing to the vLLM project.
Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run fastcheck CI which starts running only a small and essential subset of CI tests to quickly catch errors. You can run other CI tests on top of those by going to your fastcheck build on Buildkite UI (linked in the PR checks section) and unblock them. If you do not have permission to unblock, ping simon-mo or khluu to add you in our Buildkite org.

Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging.

To run CI, PR reviewers can do one of these:

Add ready label to the PR
Enable auto-merge.

🚀

tdoublep · 2025-01-21T09:04:21Z

cc @youkaichao

Signed-off-by: Thomas Parnell <[email protected]>

See upstream PR: vllm-project/vllm#12255 Would like to merge the fix here asap though because it is breaking our images.

youkaichao · 2025-01-21T09:25:52Z

requires pytorch

it usually means it needs to build against pytorch. and if you are using a custom pytorch build, the public version will not be applicable.

tdoublep · 2025-01-21T09:36:10Z

The package compressed-tensors seems to depend on torch >= 1.7:
https://github.com/neuralmagic/compressed-tensors/blob/main/setup.py#L75

and outlines seems to have unconstrained torch dependency:
https://github.com/dottxt-ai/outlines/blob/main/pyproject.toml#L42

Unless I miss something here, it seems like the packages should be flexible enough to work with custom torch version (within reason).

These are not optional dependencies and if they are not installed we get errors at import time like:

ERROR 01-20 22:47:08 registry.py:295] Traceback (most recent call last):
ERROR 01-20 22:47:08 registry.py:295]   File "<frozen runpy>", line 198, in _run_module_as_main
ERROR 01-20 22:47:08 registry.py:295]   File "<frozen runpy>", line 88, in _run_code
ERROR 01-20 22:47:08 registry.py:295]   File "/opt/vllm/lib64/python3.11/site-packages/vllm/model_executor/models/registry.py", line 514, in <module>
ERROR 01-20 22:47:08 registry.py:295]     _run()
ERROR 01-20 22:47:08 registry.py:295]   File "/opt/vllm/lib64/python3.11/site-packages/vllm/model_executor/models/registry.py", line 507, in _run
ERROR 01-20 22:47:08 registry.py:295]     result = fn()
ERROR 01-20 22:47:08 registry.py:295]              ^^^^
ERROR 01-20 22:47:08 registry.py:295]   File "/opt/vllm/lib64/python3.11/site-packages/vllm/model_executor/models/registry.py", line 265, in <lambda>
ERROR 01-20 22:47:08 registry.py:295]     lambda: _ModelInfo.from_model_cls(self.load_model_cls()))
ERROR 01-20 22:47:08 registry.py:295]                                       ^^^^^^^^^^^^^^^^^^^^^
ERROR 01-20 22:47:08 registry.py:295]   File "/opt/vllm/lib64/python3.11/site-packages/vllm/model_executor/models/registry.py", line 268, in load_model_cls
ERROR 01-20 22:47:08 registry.py:295]     mod = importlib.import_module(self.module_name)
ERROR 01-20 22:47:08 registry.py:295]           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ERROR 01-20 22:47:08 registry.py:295]   File "/usr/lib64/python3.11/importlib/__init__.py", line 126, in import_module
ERROR 01-20 22:47:08 registry.py:295]     return _bootstrap._gcd_import(name[level:], package, level)
ERROR 01-20 22:47:08 registry.py:295]            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ERROR 01-20 22:47:08 registry.py:295]   File "<frozen importlib._bootstrap>", line 1204, in _gcd_import
ERROR 01-20 22:47:08 registry.py:295]   File "<frozen importlib._bootstrap>", line 1176, in _find_and_load
ERROR 01-20 22:47:08 registry.py:295]   File "<frozen importlib._bootstrap>", line 1147, in _find_and_load_unlocked
ERROR 01-20 22:47:08 registry.py:295]   File "<frozen importlib._bootstrap>", line 690, in _load_unlocked
ERROR 01-20 22:47:08 registry.py:295]   File "<frozen importlib._bootstrap_external>", line 940, in exec_module
ERROR 01-20 22:47:08 registry.py:295]   File "<frozen importlib._bootstrap>", line 241, in _call_with_frames_removed
ERROR 01-20 22:47:08 registry.py:295]   File "/opt/vllm/lib64/python3.11/site-packages/vllm/model_executor/models/llama.py", line 41, in <module>
ERROR 01-20 22:47:08 registry.py:295]     from vllm.model_executor.layers.quantization.compressed_tensors.utils import (
ERROR 01-20 22:47:08 registry.py:295]   File "/opt/vllm/lib64/python3.11/site-packages/vllm/model_executor/layers/quantization/compressed_tensors/utils.py", line 4, in <module>
ERROR 01-20 22:47:08 registry.py:295]     from compressed_tensors import CompressionFormat
ERROR 01-20 22:47:08 registry.py:295] ModuleNotFoundError: No module named 'compressed_tensors'

If the packages are not compatible with the custom torch version, that should be encoded in their package dependencies and the install should fail? IMO that doesn't mean we shouldn't try to install them?

youkaichao · 2025-01-21T09:43:15Z

the script was originally developed for arm platform (GH200), and these packages do not have arm wheels.

It seems we only run opt-125m model on GH200, so we didn't get this error :(

tdoublep · 2025-01-21T09:48:36Z

I see. Would it make sense to filter out the dependencies in a platform-specific way?

outlines == 0.1.11; sys_platform != 'arm'
compressed-tensors == 0.8.1; sys_platform != 'arm'

(not sure what the actual string is there).

Probably if one wants to use vllm on that platform though, these packages will need to be compiled from source.

youkaichao · 2025-01-21T09:57:05Z

outlines == 0.1.11 # Requires pytorch
compressed-tensors == 0.8.1 # required for compressed-tensors, requires pytorch

can you try to remove these two comments and see if they build successfully?

we intentionally consider comments in use_existing_torch.py , because some packages do not have torch in their names, but we still want to remove them when running use_existing_torch.py

tdoublep · 2025-01-21T10:03:51Z

Sure, this change works fine in our setup (we are just using slightly older version of pytorch than vllm):

diff --git a/requirements-common.txt b/requirements-common.txt
index 6c390bcf..777b2bb1 100644
--- a/requirements-common.txt
+++ b/requirements-common.txt
@@ -19,7 +19,7 @@ pillow  # Required for image processing
 prometheus-fastapi-instrumentator >= 7.0.0
 tiktoken >= 0.6.0  # Required for DBRX tokenizer
 lm-format-enforcer >= 0.10.9, < 0.11
-outlines == 0.1.11 # Requires pytorch
+outlines == 0.1.11
 lark == 1.2.2 
 xgrammar >= 0.1.6; platform_machine == "x86_64"
 typing_extensions >= 4.10
@@ -34,6 +34,6 @@ pyyaml
 six>=1.16.0; python_version > '3.11' # transitive dependency of pandas that needs to be the latest version for python 3.12
 setuptools>=74.1.1; python_version > '3.11' # Setuptools is used by triton, we need to ensure a modern version is installed for 3.12+ so that it does not try to import distutils, which was removed in 3.12
 einops # Required for Qwen2-VL.
-compressed-tensors == 0.8.1 # required for compressed-tensors, requires pytorch
+compressed-tensors == 0.8.1 # required for compressed-tensors
 depyf==0.18.0 # required for profiling and debugging with compilation config
 cloudpickle # allows pickling lambda functions in model_executor/models/registry.py

youkaichao · 2025-01-21T10:11:30Z

this change works fine in our setup (we are just using slightly older version of pytorch than vllm)

then why do we need this PR?

tdoublep · 2025-01-21T10:19:17Z

If removing dependencies that have torch in the comment is the desired behaviour, we should not change the use_existing_torch script, fine. This PR can be closed.

However, as it stands, if someone wants to use vLLM with slightly older pytorch version, running this script will strip out required dependencies (outlines, compressed-tensors) and the package will not work (see import errors above).

If the comments for outlines and compressed-tensors were added to prevent them being installed on one specific platform (Arm), then I would argue that should be encoded in a different way (using platform specifiers), or the comments should simply be removed (since the package won't work on Arm without them either).

If we agree on the solution, I can either change this PR or create a new one.

youkaichao · 2025-01-21T10:21:48Z

the comments should simply be removed (since the package won't work on Arm without them either)

let's try this approach, thanks.

tdoublep · 2025-01-21T10:25:28Z

Closing (will resolve in a different way as-per discussion).

…ncies (#68) I already tried to fix this using #66 but upstream didn't like that change (the behaviour to filter out comments containing torch was intentional). After [some discussion](vllm-project/vllm#12255), we agreed on a different solution implemented in this PR. Note that I reverted the changes from #66 by force pushing main. Note this has already been merged upstream by vllm-project/vllm#12260 but I'm cherry-picking the fix here since it is blocking the CI builds.

use_existing_torch.py: filter out comments

3105f0c

Signed-off-by: Thomas Parnell <[email protected]>

tdoublep mentioned this pull request Jan 21, 2025

use_existing_torch.py: filter out comments IBM/vllm#66

Merged

fmt

8e18cc8

Signed-off-by: Thomas Parnell <[email protected]>

tdoublep added a commit to IBM/vllm that referenced this pull request Jan 21, 2025

use_existing_torch.py: filter out comments (#66)

30177f1

See upstream PR: vllm-project/vllm#12255 Would like to merge the fix here asap though because it is breaking our images.

tdoublep closed this Jan 21, 2025

This was referenced Jan 21, 2025

[Bugfix] Remove comments re: pytorch for outlines + compressed-tensors dependencies #12260

Merged

Remove comments re: pytorch for outlines + compressed-tensors dependencies IBM/vllm#68

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Bugfix] use_existing_torch.py: filter out comments #12255

[Bugfix] use_existing_torch.py: filter out comments #12255

tdoublep commented Jan 21, 2025 •

edited by github-actions bot

Loading

github-actions bot commented Jan 21, 2025

tdoublep commented Jan 21, 2025

youkaichao commented Jan 21, 2025

tdoublep commented Jan 21, 2025 •

edited

Loading

youkaichao commented Jan 21, 2025

tdoublep commented Jan 21, 2025

youkaichao commented Jan 21, 2025

tdoublep commented Jan 21, 2025

youkaichao commented Jan 21, 2025

tdoublep commented Jan 21, 2025

youkaichao commented Jan 21, 2025

tdoublep commented Jan 21, 2025 •

edited

Loading

[Bugfix] use_existing_torch.py: filter out comments #12255

[Bugfix] use_existing_torch.py: filter out comments #12255

Conversation

tdoublep commented Jan 21, 2025 • edited by github-actions bot Loading

github-actions bot commented Jan 21, 2025

tdoublep commented Jan 21, 2025

youkaichao commented Jan 21, 2025

tdoublep commented Jan 21, 2025 • edited Loading

youkaichao commented Jan 21, 2025

tdoublep commented Jan 21, 2025

youkaichao commented Jan 21, 2025

tdoublep commented Jan 21, 2025

youkaichao commented Jan 21, 2025

tdoublep commented Jan 21, 2025

youkaichao commented Jan 21, 2025

tdoublep commented Jan 21, 2025 • edited Loading

tdoublep commented Jan 21, 2025 •

edited by github-actions bot

Loading

tdoublep commented Jan 21, 2025 •

edited

Loading

tdoublep commented Jan 21, 2025 •

edited

Loading