[Chore] Separate out optional dependency checks from vllm.utils #27207
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a nice cleanup that separates the optional dependency checks from vllm.utils into a new, dedicated module vllm.utils.optional_deps. This refactoring improves code organization and maintainability. The implementation is correct, and backward compatibility is maintained by re-exporting the moved functions from vllm.utils. I have no concerns with this change.
dongbo910220
force-pushed
the
feature/refactor-optional-deps
branch
from
29f5766 to
8e0fc82
Compare
dongbo910220
requested review from
DarkLight1337,
WoosukKwon,
mgoin,
robertgshaw2-redhat,
tlrmchlsmth,
yewentao256 and
ywang96
as code owners
|
Documentation preview: https://vllm--27207.org.readthedocs.build/en/27207/ |
mergify
bot
added
documentation
Move optional dependency detection functions to vllm/utils/optional_deps.py: - has_pplx: Check for pplx_kernels package - has_deep_ep: Check for deep_ep package - has_deep_gemm: Check for deep_gemm package - has_triton_kernels: Check for triton_kernels package - has_tilelang: Check for tilelang package - _has_module: Cached helper for module detection This reduces vllm/utils/__init__.py from 1310 to 1285 lines and groups related dependency checking utilities in a single module. Contributes to vllm-project#26900 Signed-off-by: dongbo910220 <[email protected]>
dongbo910220
force-pushed
the
feature/refactor-optional-deps
branch
from
8e0fc82 to
5949bb7
Compare
Move optional dependency detection functions (has_pplx, has_deep_ep, has_deep_gemm, has_triton_kernels, has_tilelang) from optional_deps.py into import_utils.py. This consolidates all import-related utilities into a single module since they all depend on importlib.util.find_spec. Changes: - Add optional dependency detection functions to import_utils.py - Update all imports from vllm.utils.optional_deps to vllm.utils.import_utils - Remove vllm/utils/optional_deps.py - Restore accidentally deleted comments in benchmark and example files Contributes to vllm-project#26900 Signed-off-by: dongbo910220 <[email protected]>
dongbo910220
force-pushed
the
feature/refactor-optional-deps
branch
from
241cc5c to
bc06984
Compare
|
@yewentao256 Thank you for advice! I've merged |
github-project-automation
bot
moved this from To Triage
to Ready
in gpt-oss Issues & Enhancements
Co-authored-by: Wentao Ye <[email protected]> Signed-off-by: dongbo910220 <[email protected]>
Add required blank lines between class definition and module-level comment as per PEP 8 style guide (two blank lines between top-level definitions). Signed-off-by: dongbo910220 <[email protected]>
Update import path from `vllm.utils` to `vllm.utils.import_utils` to align with the refactoring that merged optional dependency checks into import_utils module. This completes the migration for has_triton_kernels references across the codebase. Signed-off-by: dongbo910220 <[email protected]>
Resolve conflict in vllm/model_executor/layers/fused_moe/layer.py by combining imports from both branches: - Keep import_utils imports (has_deep_ep, has_pplx) from our branch - Add current_stream import from main branch Signed-off-by: dongbo910220 <[email protected]>
The importlib module was imported but never used in __init__.py after the optional dependency checks were moved to import_utils.py. Fixes ruff F401 error. Signed-off-by: dongbo910220 <[email protected]>
|
Hi @yewentao256 , It seems a couple of CI checks have failed. I've done an initial analysis and would appreciate your help verifying my findings, as the root causes seem potentially unrelated to this PR's changes. Distributed Test Failure: The test tests/distributed/test_same_node.py is failing with exitcode: 1. Since this PR only changes the location of Python import utilities and doesn't touch any distributed logic, my current thought is that this might be a transient CI environment issue or a flaky test. C Extension Symbol Error: An ImportError is occurring with an undefined symbol: ZNK3c106SymInt6sym_neERKS0. This symbol decodes to c10::SymInt::sym_ne, which points to a PyTorch-level issue. This looks like a potential binary compatibility problem between the project's C extension (vllm._C) and the version of PyTorch in the CI environment. Although the functions I moved are part of the traceback, their internal logic is identical to the original; only their location has changed. If there's anything I can do, just let me know. |
|
Hi @DarkLight1337 , this appears to be the same undefined symbol CI failure as in #27201. |
github-project-automation
bot
moved this from Ready
to Done
in gpt-oss Issues & Enhancements
…-project#27207) Signed-off-by: dongbo910220 <[email protected]> Signed-off-by: dongbo910220 <[email protected]> Co-authored-by: Wentao Ye <[email protected]> Co-authored-by: Cyrus Leung <[email protected]>
…-project#27207) Signed-off-by: dongbo910220 <[email protected]> Signed-off-by: dongbo910220 <[email protected]> Co-authored-by: Wentao Ye <[email protected]> Co-authored-by: Cyrus Leung <[email protected]> Signed-off-by: Alberto Perdomo <[email protected]>
…-project#27207) Signed-off-by: dongbo910220 <[email protected]> Signed-off-by: dongbo910220 <[email protected]> Co-authored-by: Wentao Ye <[email protected]> Co-authored-by: Cyrus Leung <[email protected]>
…-project#27207) Signed-off-by: dongbo910220 <[email protected]> Signed-off-by: dongbo910220 <[email protected]> Co-authored-by: Wentao Ye <[email protected]> Co-authored-by: Cyrus Leung <[email protected]> Signed-off-by: 0xrushi <[email protected]>
…-project#27207) Signed-off-by: dongbo910220 <[email protected]> Signed-off-by: dongbo910220 <[email protected]> Co-authored-by: Wentao Ye <[email protected]> Co-authored-by: Cyrus Leung <[email protected]> Signed-off-by: 0xrushi <[email protected]>
3 participants
Part of #26900
This PR continues the effort to clean up vllm.utils by consolidating the helper functions used for checking optional dependencies into vllm/utils/import_utils.py.
Following @yewentao256's suggestion, these functions have been merged into import_utils.py since they all depend on importlib.util.find_spec, making it more logical to group all import-related utilities together.
The following functions have been moved:
vllm.utils.has_pplx -> vllm.utils.import_utils.has_pplx
vllm.utils.has_deep_ep -> vllm.utils.import_utils.has_deep_ep
vllm.utils.has_deep_gemm -> vllm.utils.import_utils.has_deep_gemm
vllm.utils.has_triton_kernels -> vllm.utils.import_utils.has_triton_kernels
vllm.utils.has_tilelang -> vllm.utils.import_utils.has_tilelang
CC: @DarkLight1337
Test Plan
Test Result