feat(grpo): add M2PO for stale-rollout training - #9965
Open
primorLee wants to merge 3 commits into
Open
Conversation
1 task
primorLee
marked this pull request as ready for review
August 29, 2026 14:12
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
Implements M2PO (Second-Moment Trust Policy Optimization) for stale-rollout GRPO, following the final ICLR 2026 Algorithm 1. Related RFC: #9954.
What changed
loss_type=m2poand the paper defaultm2_threshold=0.04log(pi_current / pi_behavior)from the actual rollout log-probabilities for vLLM/Megatron pathsold_per_token_logpsfallback only for synchronous native HF generation; deployment-backed paths fail closed if behavior log-probabilities are missing on any rankgradient_accumulation_steps=1; Megatron concatenates all optimizer-step micro-batches before selectionm2_before,m2_after,masked_fraction, andtrust_region_fraction--lora_dropout 0, and non-zero model dropout/stochastic depth/BatchNorm/router jitter fail closedThe reference experiments use
beta=0, so M2PO defaults to zero KL coefficient when the user does not explicitly setbeta; an explicit non-zero value remains supported with a warning.The final paper describes TIS as complementary to M2PO. That composition needs both rollout-engine and training-engine behavior-policy log-probabilities. This implementation currently retains only the rollout behavior tensor, so it rejects an additional
rollout_importance_sampling_modeinstead of double-correcting the same ratio.Validation
Validated head
4ad0ed6a464119cc2d8fd3165320e58513ad0cae:git diff --checkpassedorigin/main(174f15e2a812c2669171c1006d8441348ecf35e5) passed the same 26 testsThe tests cover strict/fallback behavior-policy routing, active trust-region quadrants, final Algorithm 1 equivalence, one optimizer-batch selection across micro-batch splits, original-denominator gradients, masked padding non-finites, invalid inputs, variable distributed token counts, and exclusion of reconstructed CP replicas.
RTX 4090 CUDA oracle at head
2.8.0+cu128, CUDA12.80.307053 -> 0.039919at threshold0.040.2573001.16e-10Real two-node Megatron Context Parallel smoke
The topology/CP fix in parent
60da0d4048e25fa9d916e84ef666085aac737eacwas validated with one NVIDIA A800 80GB (rank 0) and one NVIDIA GeForce RTX 4090 (rank 1):2.8.0+cu128, CUDA12.8, NCCL2.27.3, Megatron-Core0.16.1, Transformer Engine2.16.0all_reduce(rank + 1)returned3.0on both GPUs; both ranks exited 0context_parallel_size=2; confirmed CP group size 2, DP+CP group size 2, and pure-DP group size 1swift.megatron.trainers.utils.reconstruct_tensor_cpon load-balanced CP shards; both ranks reconstructed[0.1, 0.2, 0.3, 0.4][[T,T,F,F], [T,T,T,F]][[T,T,F,F], [T,T,F,F]][[-0.27629274, 0], [-0.30535069, 0]]0.075 -> 0.025, masked fraction0.5, valid/trust/kept counts4/4/2The two-node run used SSH-forwarded loopback sockets plus a process-scoped bind shim because the GPUs are on different private networks. The shim changed only socket port selection; tensor operations, process groups, collectives, reconstruction, masking, loss, and backward used the production PyTorch/NCCL/Megatron/ms-swift paths.
The A800 instance was offline while validating
4ad0ed6, so the two-node smoke above is intentionally attributed to its exact parent commit rather than presented as a head rerun. Head changes were revalidated through the optimizer-batch distributed tests and the 4090 CUDA oracle above; they do not change CP reconstruction.Review status
The behavior-policy decision from #9954 is now resolved fail-closed for deployment-backed rollouts, and the optimizer-batch/parallelism boundaries are explicit. This is ready for maintainer review.