Fix sender device alignment in vLLM weight synchronization - #10005
Open
0KEAHA wants to merge 1 commit into
Open
Conversation
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.
PR type
PR information
Submission base:
modelscope/ms-swift@174f15e2a812c2669171c1006d8441348ecf35e5.The real GPU baseline/patch A/B was run on the earlier main snapshot
2895611fd77ded82c042724cb0bfc633ca748079; intervening upstream commits didnot modify the source or test paths in this PR.
What does this PR do?
Ensures trainer-side tensors sent to external vLLM servers satisfy the device contract of
PyNcclCommunicator.Problem
In Megatron GRPO, the sender communicator is initialized on the last trainer rank's current device, while bridge-exported model tensors may reside on another local CUDA device. vLLM requires the input of
PyNcclCommunicator.broadcast()to be oncommunicator.deviceand otherwise raises:Weight updates run in
ThreadPoolExecutorworkers, so the worker's current CUDA device/stream also cannot be assumed to match the communicator.Fix
This change adds a sender-only broadcast wrapper in
vllm_client.pythat:communicator.device;The generic broadcast helper remains unchanged because it is also used by rollout receivers that require in-place updates to their original receive buffers.
Experiment results
Tests
Added CPU-CI-compatible mock tests for:
.to();cuda:0broadcasts acuda:1tensor through the actual helper with acuda:1current stream;Local checks completed:
Real CUDA/NCCL validation on 3 x RTX 4090:
For the 64 MiB mismatch tensor test, active target allocation returned to zero
after every cycle, while allocator-reserved memory plateaued after the first
cycle.
Scope
This PR intentionally does not change the Megatron exporter, bucket sizing, rollout scheduling, GRPO logic, or receiver-side weight loading.