[Chore] Separate out vllm.utils.platform_utils.py
#27374
Conversation
Signed-off-by: Jonathan <chenleejonathan@gmail.com>
jonathanc-n
requested review from
22quinn,
NickLucche,
WoosukKwon,
benchislett,
houseroad,
jeejeelee,
luccafong,
mgoin,
njhill,
pavanimajety,
sighingnow,
tlrmchlsmth and
yewentao256
as code owners
mergify
bot
added
qwen
![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 effectively refactors hardware-related utility functions from vllm.utils into a new, dedicated module vllm.utils.hardware_utils. The changes are clean and improve code organization by grouping related functionalities. I've identified one area for improvement regarding multiprocessing context creation to enhance portability and consistency, which I've detailed in a specific comment. Overall, this is a good step towards better code structure.
| return tuple(getattr(props, name) for name in names) | ||
|
|
||
| # Run in subprocess to avoid initializing CUDA as a side effect. | ||
| mp_ctx = multiprocessing.get_context("fork") |
There was a problem hiding this comment.
Hardcoding the multiprocessing start method to "fork" is not portable and can cause issues on platforms like macOS or Windows where fork is unavailable or unsafe. It's better to use the centralized get_mp_context utility for consistency and safety.
To avoid a circular dependency (since vllm.utils imports this file), I recommend moving get_mp_context and its helper _maybe_force_spawn from vllm/utils/__init__.py to this file (vllm/utils/hardware_utils.py). This change aligns with the goal of this PR to centralize hardware-related utilities.
After moving the functions, you can change this line to use get_mp_context(). You'll also need to:
- Add the necessary imports (
os,vllm.envs,vllm.logger,vllm.ray.lazy_utils) tovllm/utils/hardware_utils.py. - Update
vllm/utils/__init__.pyto importget_mp_contextfrom its new location.
| mp_ctx = multiprocessing.get_context("fork") | |
| mp_ctx = get_mp_context() |
There was a problem hiding this comment.
I don't think get_mp_context and maybe_force_spawn are hardware utility functions. I think I can do a follow up to move these into their own multiprocessor.py util file and have that import the hardware_util functions
Signed-off-by: Jonathan <chenleejonathan@gmail.com>
DarkLight1337
changed the title
vllm.utils.hardware_utils.pyvllm.utils.platform_utils.py
|
Please fix pre-commit though |
Signed-off-by: Jonathan <chenleejonathan@gmail.com>
github-actions
bot
added
the
ready
Signed-off-by: Jonathan <chenleejonathan@gmail.com> Signed-off-by: Alberto Perdomo <aperdomo@redhat.com>
Signed-off-by: Jonathan <chenleejonathan@gmail.com>
Signed-off-by: Jonathan <chenleejonathan@gmail.com> Signed-off-by: 0xrushi <6279035+0xrushi@users.noreply.github.com>
Signed-off-by: Jonathan <chenleejonathan@gmail.com> Signed-off-by: 0xrushi <6279035+0xrushi@users.noreply.github.com>
### What this PR does / why we need it? vllm-project/vllm@c9461e0 Fix ```spec decode rejection sampler```, caused by vllm-project/vllm#26060 Fix some ```import```, caused by vllm-project/vllm#27374 Fix ```scheduler_config.send_delta_data```, caused by #3719 Fix ```init_with_cudagraph_sizes```, caused by vllm-project/vllm#26016 Fix ```vl model```of replacing PatchEmbed's conv3d to linear layer, caused by vllm-project/vllm#27418 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? CI passed with new added/existing test. - vLLM version: v0.11.0rc3 - vLLM main: vllm-project/vllm@c9461e0 --------- Signed-off-by: Icey <1790571317@qq.com>
2 participants
Purpose
Part of #26900
vllm.utils.cuda_is_initialized ⇒ vllm.utils.hardware_utils.cuda_is_initializedvllm.utils.xpu_is_initialized ⇒ vllm.utils.hardware_utils.xpu_is_initializedvllm.utils.cuda_get_device_properties ⇒ vllm.utils.hardware_utils.cuda_get_device_propertiesvllm.utils.is_pin_memory_available ⇒ vllm.utils.hardware_utils.is_pin_memory_availablevllm.utils.is_uva_available ⇒ vllm.utils.hardware_utils.is_uva_availableTest Plan
Test Result
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.