[1/N] Add optional NCCL m2n weight sync backend#335
Conversation
Add the non-colocate backend switch and an explicit NCCL Xfer fallback wrapper so the experimental path can be enabled without changing the stable broadcast default. Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: princepride <wangzhipeng628@gmail.com>
Keep the MVP PR focused by dropping standalone argument parser tests for the opt-in backend flag. Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: princepride <wangzhipeng628@gmail.com>
Avoid repeated model scans during weight updates and handle object-style quantization configs when deciding broadcast fallback. Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: princepride <wangzhipeng628@gmail.com>
Add the explicit zip strictness required by Ruff and include formatter-only cleanup so the branch passes local hooks. Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: princepride <wangzhipeng628@gmail.com>
There was a problem hiding this comment.
Code Review
This pull request introduces an experimental, opt-in weight synchronization backend using NCCL Xfer (nccl-xfer) for non-colocated rollout engines, featuring a fallback mechanism to the existing broadcast backend. It includes layout analysis to classify weight tensors, a placeholder shim for the native NCCL Xfer Python bridge, and corresponding unit tests. Feedback on the changes highlights two important issues: first, named_params_and_buffers needs to be called with convert_to_global_name=True to ensure Hugging Face parameter names are passed to the layout analysis; second, torch.uint32 and torch.uint64 should be accessed safely via getattr to avoid compatibility issues on PyTorch versions older than 2.3.
| if not availability.available: | ||
| return availability.reason or "native NCCL Xfer bridge is unavailable" | ||
|
|
||
| for name, tensor in named_params_and_buffers(self.args, self.model): |
There was a problem hiding this comment.
By default, named_params_and_buffers returns Megatron-style parameter names (e.g., containing query_key_value or dense_h_to_4h). However, analyze_nccl_xfer_layout expects Hugging Face parameter names (e.g., q_proj.weight, o_proj.weight) to correctly identify parallel layouts. Without converting to global (Hugging Face) names, the layout analysis will fail to match any parallel suffixes and incorrectly classify all sharded parameters as replicated, leading to incorrect synchronization behavior. Pass convert_to_global_name=True to retrieve the correct Hugging Face names.
| for name, tensor in named_params_and_buffers(self.args, self.model): | |
| for name, tensor in named_params_and_buffers(self.args, self.model, convert_to_global_name=True): |
| _SUPPORTED_DTYPES = { | ||
| dtype | ||
| for dtype in ( | ||
| torch.int8, | ||
| torch.uint8, | ||
| getattr(torch, "float8_e4m3fn", None), | ||
| getattr(torch, "float8_e5m2", None), | ||
| torch.float16, | ||
| torch.bfloat16, | ||
| torch.int32, | ||
| torch.uint32, | ||
| torch.float32, | ||
| torch.int64, | ||
| torch.uint64, | ||
| torch.float64, | ||
| ) | ||
| if dtype is not None | ||
| } |
There was a problem hiding this comment.
Directly referencing torch.uint32 and torch.uint64 will raise an AttributeError on PyTorch versions older than 2.3, as these dtypes were only introduced recently. To ensure compatibility with older PyTorch environments, these should be accessed safely using getattr(torch, 'uint32', None) and getattr(torch, 'uint64', None).
| _SUPPORTED_DTYPES = { | |
| dtype | |
| for dtype in ( | |
| torch.int8, | |
| torch.uint8, | |
| getattr(torch, "float8_e4m3fn", None), | |
| getattr(torch, "float8_e5m2", None), | |
| torch.float16, | |
| torch.bfloat16, | |
| torch.int32, | |
| torch.uint32, | |
| torch.float32, | |
| torch.int64, | |
| torch.uint64, | |
| torch.float64, | |
| ) | |
| if dtype is not None | |
| } | |
| _SUPPORTED_DTYPES = { | |
| dtype | |
| for dtype in ( | |
| torch.int8, | |
| torch.uint8, | |
| getattr(torch, "float8_e4m3fn", None), | |
| getattr(torch, "float8_e5m2", None), | |
| torch.float16, | |
| torch.bfloat16, | |
| torch.int32, | |
| getattr(torch, "uint32", None), | |
| torch.float32, | |
| torch.int64, | |
| getattr(torch, "uint64", None), | |
| torch.float64, | |
| ) | |
| if dtype is not None | |
| } |
Resolve conflicts: - actor.py: keep both the nccl-xfer backend selection and the new delta-mode update_weight branch (independent elif clauses). - tests: relocate nccl-xfer tests into tests/utils/ following the test-directory rename on main. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Summary
This PR is the first step toward the NCCL m2n rollout weight sync proposal in #52.
It adds an opt-in non-colocated weight sync backend:
The existing gather-broadcast path remains the default. Colocated rollout engines continue using the existing CUDA IPC path.
Changes
UpdateWeightFromNcclm2nas an optional non-colocated updater.Stack
This is
[1/N].Follow-up PRs will cover the native
ncclm2nReshardWithWindowruntime path, vLLM worker receive integration, staging/window management, and performance comparison against the legacy gather-broadcast path.Test Plan
Supersedes #159 (re-opened from fork branch
princepride:feature/nccl-xfer-weight-sync-mvp).