Skip to content

feat: Support for dtensor ppo - #2837

Merged
terrykong merged 28 commits into
mainfrom
ppo-dtensor-path-a
Jun 26, 2026
Merged

terrykong merged 28 commits into
mainfrom
ppo-dtensor-path-a

Conversation

@fujial-code

@fujial-code fujial-code commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

What does this PR do ?

Adds DTensor/FSDP2 support for the PPO value model path and aligns the DTensor PPO 1.5B GSM8K recipe with the existing Megatron-Core PPO baseline.
The main correctness fix is a value temporal alignment issue: DTensor value inference/training was not using the same V(s_t) convention as the Megatron PPO path. After right-shifting token values to match Megatron's temporal semantics, the DTensor reward curve aligns with the Megatron baseline on the Qwen2.5-1.5B GSM8K 1-node recipe.

Issues

close [#2047]

Summary of Changes

DTensor PPO Value Model

  • Adds DTensorValueWorkerV2 for PPO value model training and inference on the DTensor/FSDP2 backend.
  • Wires DTensor value worker selection through LMValue.
  • Supports PPO value inference, value training, and checkpoint save/load paths for the DTensor backend.
  • Adds lifecycle compatibility with the PPO loop, including inference/training finish hooks where needed.

Value Temporal Alignment

  • Fixes the DTensor value convention to match the Megatron PPO path.
  • Applies right-shifted values so token-level value predictions correspond to V(s_t).
  • This was the key fix for the DTensor vs Megatron reward mismatch observed during GSM8K validation.

Recipe and Config Alignment

  • Adds the DTensor 1-node 8-GPU Qwen2.5-1.5B GSM8K PPO recipe:
    • examples/configs/recipes/llm/ppo-qwen2.5-1.5b-gsm8k-1n8g-dtensor.yaml
  • Aligns the DTensor recipe with the Megatron recipe for A/B comparison:
    • ppo_epochs: 1
    • math_verify_impl: hf_math_verify
    • matching value loss config structure
  • Removes the unused GSM8K verifier-specific path and uses the existing HF math verifier for the GSM8K recipe.

Runtime Compatibility Fixes

  • Updates DTensor worker call sites to match current Automodel API signatures.
  • Removes stale kwargs that no longer exist in the current forward/checkpoint APIs.
  • Passes checkpointing config when saving the PPO value model checkpoint.
  • Adds missing imports and no-op lifecycle methods needed by the PPO loop.

Tests

  • Unit tests: Added DTensor PPO value temporal alignment tests for _right_shift_values and _RightShiftLossWrapper.
  • Functional test: Added a 2-GPU DTensor PPO smoke test using Qwen2.5-0.5B on GSM8K for 2 steps with metric sanity checks.
  • Nightly test: Added a 1-node 8-GPU Qwen2.5-1.5B GSM8K DTensor PPO test, checking train/reward > 0.75 and validation/accuracy > 0.65 at step 40.
  • Local checks: Ran Python compile, shell syntax checks, nightly dry-run, and git diff --check.

Validation

Validated the DTensor PPO path against the Megatron-Core PPO baseline on the Qwen2.5-1.5B GSM8K 1-node 8-GPU setup.

Ran 100-step PPO training for both backends with matched recipe settings:

  • Model: Qwen2.5-1.5B-Instruct
  • Dataset: GSM8K
  • Cluster: 1 node, 8 GPUs
  • Backends:
    • DTensor/FSDP2 Automodel backend
    • Megatron-Core backend
  • Matched settings:
    • ppo_epochs: 1
    • math_verify_impl: hf_math_verify
    • matched value loss config
    • DTensor train/logprob micro batch size aligned to 4 for the final run

Results:

  • train/reward: DTensor and Megatron follow the same learning curve and both converge to around 0.88-0.90 by step 100.
  • validation/accuracy: DTensor and Megatron track closely across training and reach around 0.78-0.80 by step 100.
  • The reward and validation accuracy curves are aligned after fixing the value temporal shift and aligning the recipe settings.
    These results indicate that the DTensor PPO value path now matches the Megatron-Core PPO baseline behavior on the 1.5B GSM8K recipe.

@copy-pr-bot

copy-pr-bot Bot commented Jun 16, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@fujial-code fujial-code added the CI:Lfast Runs a fast test suite and re-use nightly `main` container (but sync dependencies to PRs version) label Jun 16, 2026
@fujial-code

Copy link
Copy Markdown
Contributor Author

/ok to test 980ef04

@fujial-code

Copy link
Copy Markdown
Contributor Author

/ok to test 980ef04

@copy-pr-bot

copy-pr-bot Bot commented Jun 16, 2026

Copy link
Copy Markdown

/ok to test 980ef04

@fujial-code, there was an error processing your request: E2

See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/2/

@fujial-code

Copy link
Copy Markdown
Contributor Author

/ok to test 980ef04

@copy-pr-bot

copy-pr-bot Bot commented Jun 16, 2026

Copy link
Copy Markdown

/ok to test 980ef04

@fujial-code, there was an error processing your request: E2

See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/2/

@fujial-code

Copy link
Copy Markdown
Contributor Author

/ok to test 8681202

@fujial-code

Copy link
Copy Markdown
Contributor Author

/ok to test 138ae84

@fujial-code

Copy link
Copy Markdown
Contributor Author

/ok to test a9d9ea8

@fujial-code

Copy link
Copy Markdown
Contributor Author

/ok to test aa7a690

@yuki-97
yuki-97 force-pushed the ppo-dtensor-path-a branch 5 times, most recently from 473ddaf to 6f4ddc6 Compare June 26, 2026 07:02
yuki-97 added 2 commits June 26, 2026 00:21
Signed-off-by: Yuki Huang <yukih@nvidia.com>
Signed-off-by: Yuki Huang <yukih@nvidia.com>
yuki-97 added 5 commits June 26, 2026 00:59
Signed-off-by: Yuki Huang <yukih@nvidia.com>
Signed-off-by: Yuki Huang <yukih@nvidia.com>
Signed-off-by: Yuki Huang <yukih@nvidia.com>
Signed-off-by: Yuki Huang <yukih@nvidia.com>
Signed-off-by: Yuki Huang <yukih@nvidia.com>
@yuki-97 yuki-97 added CI:L1 Run doctests, unit tests, and functional tests and removed CI:Lfast Runs a fast test suite and re-use nightly `main` container (but sync dependencies to PRs version) labels Jun 26, 2026
@yuki-97

yuki-97 commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

/ok to test 51cad68

Signed-off-by: Yuki Huang <yukih@nvidia.com>
@yuki-97

yuki-97 commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

/ok to test c569d70

@yuki-97 yuki-97 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two cleanup nits: dead _v2=False branch in Value.save_checkpoint and an unused-arg wrapper in the new DTensor value worker. Neither is blocking.

Comment thread nemo_rl/models/value/lm_value.py Outdated
Comment thread nemo_rl/models/value/workers/dtensor_value_worker_v2.py Outdated
… actor, tighten value-backend assert

Signed-off-by: Yuki Huang <yukih@nvidia.com>
@yuki-97

yuki-97 commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

/ok to test 8403d74

yuki-97 pushed a commit that referenced this pull request Jun 26, 2026
The PPO value worker (MegatronValueWorkerImpl) landed in main after this branch
was first written and runs a Megatron model on GPU like the policy worker, but
did not call bind_to_gpu_numa(). Add the bind early in __init__ for parity with
the Megatron/DTensor policy workers and vLLM workers.

Note: the DTensor PPO value worker (PR #2837, feat: Support for dtensor ppo)
will need the same call; whichever of the two PRs merges second must add it.

Signed-off-by: Terry Kong <terryk@nvidia.com>
@terrykong
terrykong merged commit 4c0c268 into main Jun 26, 2026
164 of 167 checks passed
@terrykong
terrykong deleted the ppo-dtensor-path-a branch June 26, 2026 18:08
@qiaochuz-nv

qiaochuz-nv commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

POR Test Summary — NVIDIA-NeMo/RL PR #2837 (DTensor PPO support)

Result: PASS

What PR #2837 adds

DTensor (FSDP2) backend support for the value model in PPO. Prior to this,
the value/critic path was Megatron-only (cf. PR #2530). The test drives the real
shipped recipe on the DTensor path with megatron_cfg.enabled: false.

It now states the test covers:
- PPO algorithm value/critic path: nemo_rl/algorithms/ppo.py
- DTensor value worker: nemo_rl/models/value/workers/dtensor_value_worker_v2.py
- DTensor policy worker: nemo_rl/models/policy/workers/dtensor_policy_worker_v2.py
- Automodel DTensor setup/checkpoint path: nemo_rl/models/automodel/{setup.py,checkpoint.py}
- Ray worker registry path: nemo_rl/distributed/ray_actor_environment_registry.py
- Shipped DTensor PPO recipe: ppo-qwen2.5-1.5b-gsm8k-1n8g-automodel-valuetp2sp.yaml

Test

  • ID: test_ppo_dtensor_qwen1.5b_1n8g
  • Tier: function (1 node × 8 GPU, submit_mode: srun)
  • Wrapper: nemo_llm/test_suite/rl/testcases/algorithms/test_ppo_dtensor_qwen1.5b_1n8g.sh
  • Entrypoint: examples/run_ppo.py (real product recipe — NOT a re-wrap of PR unit tests)
  • Config: examples/configs/recipes/llm/ppo-qwen2.5-1.5b-gsm8k-1n8g-automodel-valuetp2sp.yaml
  • Backend: policy.dtensor_cfg.enabled: true, value.dtensor_cfg.enabled: true, megatron_cfg.enabled: false
  • Model: Qwen/Qwen2.5-1.5B-Instruct; value TP2/SP
  • Shrink: step counts only (ppo.max_num_steps=2, max_num_epochs=1, val_period=999999); model/parallelism/batch untouched
  • Tags: [codecoverage, rc_p0, por]

Run

  • Cluster: EOS (login-eos02), account coreai_dlalgo_ci, partition batch
  • JOBID: 5528566 (scoring srun); driver run-id 20260629T062002Z-2cbd7c20
  • State: COMPLETED, duration 268s
  • Container: nemo-rl-nightly-20260629.sqsh
  • Workdir (isolated): /lustre/fsw/coreai_dlalgo_ci/qiaochuz/nemo_llm_por_2837

Metric gate (all PASS — from real stdout.log)

Assertion Value
max train/critic/loss < 6.0 3.090
min train/critic/loss >= 0 1.982
max train/critic/explained_var <= 1.0001 -13.370
max train/critic/grad_norm < 350 105.634
max train/token_mult_prob_error < 1.05 1.0158
train/probs_ratio_clamped_min in (0.79, 1.21) 1.0
train/probs_ratio_clamped_max in (0.79, 1.29) 1.0

DTensor backend confirmed exercised in logs: DTensorValueWorkerV2 +
DTensorPolicyWorkerV2 workers; value model weights loaded (338 shards) and
critic loop emitted train/critic/{loss,grad_norm,explained_var}.

Status flip

testcase_list.jsonfunction.test_ppo_dtensor_qwen1.5b_1n8g.status: untestedpass

Artifacts

~/nemo/nmfw_tests_por_2837/nemo_llm/test_suite/rl/logs/20260629T062002Z-2cbd7c20/

  • tests/test_ppo_dtensor_qwen1.5b_1n8g/summary.json (status: passed, job_id 5528566)
  • tests/test_ppo_dtensor_qwen1.5b_1n8g/stdout.log

Branch / worktree

  • Worktree: ~/nemo/nmfw_tests_por_2837
  • Branch: qiaochuz/por_2837_ppo_dtensor (off main)
  • slurm_config.py host/identity/workdir edits are local launch-env overrides — keep out of the committed test contract.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI:L1 Run doctests, unit tests, and functional tests Documentation Improvements or additions to documentation QA:Verified r0.7.0 Ready for QA

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[dtensor] PPO

6 participants