Skip to content

feat(sc): stream PPO actor and critic minibatches - #3963

Open
tianyi-zhang-02 wants to merge 5 commits into
NVIDIA-NeMo:mainfrom
tianyi-zhang-02:codex/sc-ppo-critic-minibatches
Open

feat(sc): stream PPO actor and critic minibatches#3963
tianyi-zhang-02 wants to merge 5 commits into
NVIDIA-NeMo:mainfrom
tianyi-zhang-02:codex/sc-ppo-critic-minibatches

Conversation

@tianyi-zhang-02

@tianyi-zhang-02 tianyi-zhang-02 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Lets Single Controller PPO stream actor and Megatron critic minibatches from native TQ replay instead of materializing one merged training payload.

The critic now has an explicit begin/train-chunk/finish/abort lifecycle. Chunks accumulate gradients and correctly normalized diagnostics, then take one optimizer step at the end of each critic epoch. The controller retains replay rows across critic and actor epochs, cleans up partial failures, skips invalid chunks, and rejects drop budgets that cannot preserve data-parallel batch divisibility.

Relates to #2625.

Validation

Current head 884e238a1fdd2f84cb44a3deb2042981e91d05c8, merged with upstream main at e831622435180c88d6adcc9fce46bfd88a85f3e5.

Check Result
PPO, value, setup, and SC actor unit suites on WSL2 Linux x86_64 (Python 3.13.14, pytest 9.1.1, torch 2.11.0+cu130; CUDA hidden) 230 passed, 7 CUDA-only skipped
Ruff on all changed Python files passed
Split critic parity on 2×H100 NVL, Megatron value model (pre-refresh head) loss, grad norm, and every diagnostic metric matched
SC PPO functional + resume on 2×H100 NVL, Qwen2.5-0.5B, async vLLM, Megatron policy/value (pre-refresh head) steps 0–2 and resume 2–4 passed
Checkpoint and TensorBoard assertions (pre-refresh head) checkpoints complete at steps 1–4; 10/10 assertions passed

The main refresh composes the current token-capture finalizer cleanup for canonical and staging rows with PPO's multi-epoch row retention. The current-head CPU-capable suite covers that merged control flow, including failure cleanup.

The GPU runs were collected at pre-refresh head 2a9b108a38a0be424c9693c806cd0e41dd5985b5. The refreshed head was revalidated with the full CPU-capable suite above; GPU-only Megatron split parity remains skipped in the current environment.

Functional entry point:

uv run --locked bash tests/functional/ppo_async_single_controller.sh

Before your PR is ready for review

  • Read and followed the contributor guidelines
  • Added the necessary tests
  • Ran unit and functional tests
  • Updated the Single Controller guide and example config

Signed-off-by: Tianyi Zhang <123608656+tianyi-zhang-02@users.noreply.github.com>
Signed-off-by: Tianyi Zhang <123608656+tianyi-zhang-02@users.noreply.github.com>
Signed-off-by: Tianyi Zhang <123608656+tianyi-zhang-02@users.noreply.github.com>
@copy-pr-bot

copy-pr-bot Bot commented Sep 2, 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.

@github-actions github-actions Bot added the Documentation Improvements or additions to documentation label Sep 2, 2026
@tianyi-zhang-02
tianyi-zhang-02 marked this pull request as ready for review September 2, 2026 15:11
@tianyi-zhang-02
tianyi-zhang-02 requested review from a team as code owners September 2, 2026 15:11
…inibatches

# Conflicts:
#	docs/guides/single-controller.md
@svcnvidia-nemo-ci svcnvidia-nemo-ci added the waiting-on-maintainers Waiting on maintainers to respond label Sep 4, 2026
…inibatches

# Conflicts:
#	nemo_rl/algorithms/single_controller.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-request Documentation Improvements or additions to documentation waiting-on-maintainers Waiting on maintainers to respond

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants