Repository navigation
Conversation
…built The PLE offload process builds its own VllmConfig for the isolated Gloo world and forced DP1 only after construction. ParallelConfig takes the DP variables the process inherits from its GPU worker, and VllmConfig.__post_init__ derives the participating devices from them. For a GPU worker of DP rank 1 that sees only its own device, that lookup asks for device 1 and raises IndexError before DP1 is set. Build the parallel config first, reduce it to DP1/TP1/PP1, and pass it to VllmConfig. The regression test set the DP variables as envs attributes. Because VLLM_DP_RANK_LOCAL defaults to VLLM_DP_RANK, monkeypatch recorded the patched rank as its old value and restored local DP rank 1 as a module attribute, which broke the next test on a host with one visible GPU. Set them through set_lazy_env. Co-authored-by: Claude Signed-off-by: Peuqui <43776522+Peuqui@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Fixes #739: its first test was fixed on main by 9170150, this fixes the second.
_init_offload_distributed(vllm/v1/ple_offload/worker.py) builds the VllmConfig for the PLE offload process's isolated Gloo world asVllmConfig()and forces DP1/TP1/PP1 only afterwards.ParallelConfigtakes the DP variables the process inherits from its GPU worker (VLLM_DP_SIZE,VLLM_DP_RANK,VLLM_DP_RANK_LOCAL), andVllmConfig.__post_init__derives the participating devices from them (apply_runtime_policy_defaults->_participating_cuda_device_ids). With an inherited local DP rank of 1 and one visible device, that lookup asks for device 1 and fails before DP1 is set:This PR builds the
ParallelConfigfirst, reduces it to DP1/TP1/PP1 with the same assignments as before, and passes it toVllmConfig(parallel_config=...), so__post_init__already sees DP1. From the source only, not run: outside the test this would hit the offload process of a DP rank whose GPU worker sees only its own devices (set_device_control_env_varrestricts the visible devices per DP rank), if it inherits the DP variables as the test assumes.The regression test,
test_offload_distributed_sets_config_only_for_model_parallel, set the DP variables withmonkeypatch.setattr(envs, ...).VLLM_DP_RANK_LOCALdefaults to the currentVLLM_DP_RANK, and the test patchedVLLM_DP_RANKfirst, so monkeypatch recorded 1 as the old value ofVLLM_DP_RANK_LOCALand restored it as a module attribute. Every later test in the process then ran as local DP rank 1; with one visible device,test_offload_world_drops_the_pipeline_layer_partitionfailed with the same IndexError (it passes alone). The test now sets the three variables throughset_lazy_env, which exists for exactly this shadowing.Not a duplicate: gh pr list --repo 1CatAI/1Cat-vLLM --state all --search with "_init_offload_distributed", "PLE offload DP", "VLLM_DP_RANK_LOCAL", "offload DP1", "test_offload_distributed_sets_config_only_for_model_parallel", "ple_offload worker IndexError" and "739 in:body" finds no PR that changes this construction or this test (#1005 mentions #739 only as not covered). #1214 (ours) repairs three other stale fakes in the same test file and merges with this PR without conflicts. git merge-tree against all 124 open PR heads adds one conflict: #1023 (draft) also edits the
from vllm.config import ...line of worker.py.AI assistance was used for this change. I reviewed every line and ran the tests below.
Test Plan
Path coverage
Acceleration and benchmark contract (required for performance changes)
Not a performance change. Python only; no new switches, fields or environment variables.
Test Result
test_ple_offload_uses_event_per_inflight_mrv2_batch,test_ple_offload_requires_ple_layers[ple_layer_ids0-True],test_ple_offload_runner_groups_registrations_by_dp_rank) are the stale fakes [Test] Repair stale fakes in PLE offload and SM70 decode-graph tests #1214 repairs; with the test part of [Test] Repair stale fakes in PLE offload and SM70 decode-graph tests #1214 applied as well: 55 passed, 1 skipped with one V100 and with one RTX 8000.test_offload_world_drops_the_pipeline_layer_partition, one visible V100): with only the runtime change, the second test fails after the first (1 failed, 1 passed); with only the test change, the regression test fails with the IndexError above (1 failed, 1 passed).🤖 Generated with Claude Code
https://claude.ai/code/session_01YaG8VjXn5eN3Aq7eZrA3dp