Repository navigation
[Core] Drop the Ray driver-node GPU pin left over from the legacy executor - #61713
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
| peft>=0.20.0 # required for phi-4-mm test | ||
| pqdm | ||
| ray[cgraph,default]>=2.56.0,<2.57 # Ray Compiled Graph, required by pipeline parallelism tests; sharded-RDT weight transfer needs Ray Direct Transport (>=2.56) | ||
| ray[cgraph,default]>=2.56.0,<2.57 # cgraph pulls cupy, used by test_weight_transfer_nccl_uid; sharded-RDT weight transfer needs Ray Direct Transport (>=2.56) |
There was a problem hiding this comment.
we can remove the cgraph extra as the compiled-graph based executor is entirely removed
There was a problem hiding this comment.
Agreed, dropped it in 8c2ff81 (also from rocm.in and nightly-torch.txt). I'd kept it only because it's what pulls in cupy for test_weight_transfer_nccl_uid, but that test importorskips cupy and isn't run by any Buildkite job, so nothing in CI loses coverage.
|
Documentation preview: https://vllm--61713.org.readthedocs.build/en/61713/ |
|
This pull request has merge conflicts that must be resolved before it can be |
…cutor With the compiled-graph executor gone, nothing runs a worker inside the Ray driver process, so initialize_ray_cluster no longer needs require_gpu_on_driver (and its driver-node bundle pin). Remove it, test_multi_node_assignment (which only asserted the pin), and the Ray 2.9 available_resources_per_node shim. Also un-skip prompt embeds on Ray in test_models_distributed (the skip blamed the compiled DAG) and fix the stale ray[cgraph] requirement comment: the extra now only matters for cupy in test_weight_transfer_nccl_uid. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Woosuk Kwon <woosuk@inferact.ai>
Only the removed compiled-graph executor needed it. Its sole other effect was pulling in cupy, which test_weight_transfer_nccl_uid importorskips and no Buildkite job runs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Woosuk Kwon <woosuk@inferact.ai>
ca6cc68 to
8c2ff81
Compare
|
/ci run |
|
✅ Triggered Buildkite CI #95015 for commit |
…cutor (vllm-project#61713) Signed-off-by: Woosuk Kwon <woosuk@inferact.ai> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Overview
Follow-up to #61712 (merged). Removes Ray code that only the legacy compiled-graph executor needed.
Claims
initialize_ray_clusterlosesrequire_gpu_on_driverand the driver-node bundle pin. The legacy executor ran rank 0 inside the driver process, so it needed a GPU on the driver node.RayExecutorV2runs every worker as a remote actor and already passedFalse.tests/distributed/test_multi_node_assignment.pyand its step in the multi-node CI job; it only asserted that pin.available_resources_per_nodeimport shim inray_utils.py(test requirements pin Ray 2.55/2.56).enable_prompt_embeds+ Ray case oftest_models_distributed; the skip reason blamed the Ray compiled DAG.ray[cgraph]extra from the CUDA, ROCm and nightly test requirements. Its only other effect was pulling in cupy (removed fromcuda.txt/cpu.txton recompile).test_weight_transfer_nccl_uidalreadyimportorskips cupy and no Buildkite job runs it.−160 / +7 lines.
Validation
pre-commit run(ruff, mypy, Buildkite checks, pip-compile): passed.tests/basic_correctness/models/test_basic_correctness.py -k "test_models_distributed and ray"(TARGET_TEST_SUITE=L4), including the un-skipped prompt-embeds case: 4 passed.tests/distributed/test_ray_v2_executor.py, 2-GPU cases: 13 passed. The 4-GPU cases couldn't run because another user's processes hold GPUs 0–1.tests/distributed/test_pipeline_parallel.py -k ray: the only 2-GPU case (PP=2, TP=1) brought up the Ray server and served completions, then failed with a 400 from the Rust frontend's JSON parser on a list-of-stringsprompt. Thempbackend fails the same way, so this is my stale localvllm-rsbinary and unrelated to this PR; CI covers this case.Details
Compatibility:
initialize_ray_clusteris exported fromvllm, so a caller that passesrequire_gpu_on_drivernow gets aTypeError. A search of the vllm-project org found no caller passing it: tpu-inference and vllm-torchtpu use their own_initialize_ray_cluster, and vllm-omni defines its own function of the same name. Callers that relied on the default pin now get a plainPACKplacement group with no driver-node constraint.Not a duplicate: no open PR touches this; it is the follow-up list from #61712.
AI assistance (Claude Code) was used to prepare this change; I reviewed every changed line.
Pull Request Checklist
I used vLLM's
/pr-checklistskill. (Mandatory for agents, optional for humans).AI assistance was used during the creation of this PR.
Design Fit: Minimizes impact on core components, reuses existing functionality, and justifies added complexity.
Testing and Validation: Validates the change and ensures any added tests are meaningful and reliable, with CI coverage or documented CI resource constraints and validation performed outside CI.
Code Quality and Style: Keeps code and comments clear and concise, and updates relevant documentation and examples.
Pull Request Contents: Includes a brief summary and relevant links, supports claims with evidence, explains root causes and implementation trade-offs, and follows the contributing guide.
🤖 Generated with Claude Code