feat: add delta-compressed collective refit - #2444
Conversation
|
Auto-sync is disabled for ready for review pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
There was a problem hiding this comment.
Pull request overview
Adds an optional delta-compressed weight transfer protocol for non-colocated vLLM collective refit, enabling the trainer source rank to send full weights or additive deltas (dense / sparse_indices / sparse_bitmask) and apply deltas additively through existing vLLM weight loaders.
Changes:
- Introduces a delta-aware packed weight transfer protocol (
full/delta/done) with sparse delta encodings and a trainer-sideDeltaCompressionTrackerbaseline. - Integrates the new transfer path into DTensor v1/v2 and Megatron policy workers via a shared
dispatch_packed_weight_transfer(...)helper. - Updates vLLM collective refit to optionally consume the new full/delta protocol and adds unit tests + example configs.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
tests/unit/utils/test_weight_transfer.py |
Adds unit coverage for delta tracker behavior, sparse transports, additive load context, and producer/consumer roundtrips. |
nemo_rl/utils/weight_transfer.py |
Implements delta tracking, sparse encodings, packed full/delta broadcast protocol, and additive load context. |
nemo_rl/utils/weight_transfer_types.py |
Defines shared literal types/constants for delta compression and transfer kinds. |
nemo_rl/utils/torch_dtypes.py |
Centralizes dtype string→torch.dtype mappings (canonical + aliases). |
nemo_rl/models/policy/workers/megatron_policy_worker.py |
Switches collective weight broadcast to the delta-aware dispatcher when enabled. |
nemo_rl/models/policy/workers/dtensor_policy_worker.py |
Switches collective weight broadcast to the delta-aware dispatcher when enabled. |
nemo_rl/models/policy/workers/dtensor_policy_worker_v2.py |
Switches collective weight broadcast to the delta-aware dispatcher when enabled. |
nemo_rl/models/generation/vllm/vllm_worker.py |
Determines whether to use delta transfer and forwards that flag to the vLLM worker extension. |
nemo_rl/models/generation/vllm/vllm_worker_async.py |
Forwards the delta-transfer enablement flag in the async prepare_refit_info path. |
nemo_rl/models/generation/vllm/vllm_backend.py |
Adds delta-aware collective consumer path and additive-delta loading through existing loaders. |
nemo_rl/models/generation/vllm/config.py |
Extends vLLM generation config typing with delta_compression settings. |
nemo_rl/models/automodel/setup.py |
Reuses canonical dtype mapping from torch_dtypes instead of duplicating it. |
examples/configs/grpo_math_1B.yaml |
Documents/introduces the new delta_compression config block (disabled by default). |
examples/configs/distillation_math.yaml |
Documents/introduces the new delta_compression config block (disabled by default). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Awesome @HollowMan6 I found delta weight transfer has its own weight transfer function, which seems a duplicated one compared with the full weight transfer. It is out of the scope of this PR, but is there any block to have delt and full weight transfer shared the same communication function while have their independent protocol to pack, unpack the model weights? |
|
Thank you @ZhiyuLi-Nvidia for pointing this out, I just did some refactoring according to your suggestion, and it looks fine. |
fa0cb08 to
a3b3c70
Compare
4022b81 to
dd37c84
Compare
2bab874 to
6071cd5
Compare
9c50ebf to
43719b7
Compare
0b16704 to
3218f2a
Compare
|
/ok to test 40812ea |
terrykong
left a comment
There was a problem hiding this comment.
Re-review re-anchored to 40812eae (was 191977d3, now rebased/orphaned)
The PR was force-pushed/rebased onto newer main (plus nightly-test tweaks), so the prior pending review's anchor commit 191977d3 is now orphaned. Re-anchored here; findings are unchanged — every refit source file and config.py is byte-identical between the two heads (verified), and grpo.py only shifted line numbers (re-located: refit block L1106, attach L1301, getattr L2091).
Findings remain all low-severity (PR approved by @ZhiyuLi-Nvidia): two security-hardening asks, a latent failure-path invariant, a verified uint32 codec test, a fail-loud guard for the GRPO-only refit_transport, env→config centralization (code + docs), plus doc/readability nits. Nothing blocking.
Generated by Claude Code (re-anchored incremental re-review)
|
/ok to test a59f208 |
Signed-off-by: Hollow Man <hollowman@opensuse.org>
Signed-off-by: Hollow Man <hollowman@opensuse.org>
Signed-off-by: Hollow Man <hollowman@opensuse.org>
Signed-off-by: Hollow Man <hollowman@opensuse.org>
Signed-off-by: Hollow Man <hollowman@opensuse.org>
Signed-off-by: Hollow Man <hollowman@opensuse.org>
Signed-off-by: Hollow Man <hollowman@opensuse.org>
Signed-off-by: Hollow Man <hollowman@opensuse.org>
Signed-off-by: Hollow Man <hollowman@opensuse.org>
Signed-off-by: Hollow Man <hollowman@opensuse.org>
Signed-off-by: Hollow Man <hollowman@opensuse.org>
Signed-off-by: Hollow Man <hollowman@opensuse.org>
Signed-off-by: Hollow Man <hollowman@opensuse.org>
Signed-off-by: Hollow Man <hollowman@opensuse.org>
Signed-off-by: Hollow Man <hollowman@opensuse.org>
|
/ok to test cf6f490 |
What does this PR do ?
Adds optional delta-compressed weight transfer with s3 or zeromq backend for non-colocated vLLM collective refit.
Please see docs/design-docs/sparse-delta-refit.md for docs.
Issues
N/A
Usage
Before your PR is "Ready for review"
Pre checks:
Additional Information