Skip to content

feat(sc): setup + entrypoint#3266

Open
yuki-97 wants to merge 31 commits into
mainfrom
yukih/sc-split-3
Open

feat(sc): setup + entrypoint#3266
yuki-97 wants to merge 31 commits into
mainfrom
yukih/sc-split-3

Conversation

@yuki-97

@yuki-97 yuki-97 commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Part 3/4 of the #2819 split. See #2819 (reference) or #3267 (4/4) for the full code and context. Stack on #3220.

#3219 (1/4) -> #3220 (2/4) -> #3266 (3/4) -> #3267 (4/4)

Note: NeMo-Gym support goes in PR#4; this PR wires up the non-gym SC path.

Summary

Driver-side setup and CLI entrypoint that make the SC (Single Controller) async-GRPO path end-to-end runnable.

  • single_controller_utils package (setup.py / config.py / utils.py): setup_single_controller builds the worker groups, virtual cluster, dp client, dataloader, TQReplayBuffer, RolloutManager, and weight synchronizer on the driver and returns a SingleControllerActorArgs. config.py centralizes AsyncRLConfig / AdvantageConfig / WeightSyncConfig / MasterConfig. utils.py hosts metric-reduction rules and KVBatchMeta helpers.
  • SingleControllerActor slimdown: takes a prebuilt SingleControllerActorArgs and does no construction work — heavy objects are cloudpickled in. Running setup inside a nested Ray actor breaks runtime_env resolution (see feat(policy): split-API train-step state machine on DTensor v1/v2 #2692). Pump crashes propagate and in-flight rollouts drain on exit.
  • AsyncRLConfig: dropped batch_selection_strategy in favor of explicit max_staleness_versions + over_sampling; renamed min_prompt_groups_per_batchmin_groups_for_streaming_train; defaults aligned with the exemplar and each field has a doc comment.
  • examples/run_grpo_single_controller.py: SC CLI entrypoint, mirrors run_grpo.py for config loading. Requires data_plane.enabled=true. Drains env actors on the error path before vLLM shutdown.
  • Recipes: grpo_math_1B_megatron_single_controller.yaml exemplar plus streaming2 and megatron-sync variants under examples/configs/recipes/llm/.
  • CI: adds L1_Functional_Tests_SingleController.sh to cicd-main.yml; adds SC recipes to nightly.txt and paired *-single-controller-{streaming2,sync}.sh launchers.
  • Tests: test_single_controller_setup.py covers the new setup module (with an init_communicator() assertion). New tests/functional/grpo_dp_single_controller.sh gives the entrypoint a 2-GPU e2e smoke. Rollout/train pump tests migrated to the split configs.

Test Results

  1. sync mode: grpo-qwen2.5-math-1.5b-instruct-1n8g-megatron-single-controller-sync
    baseline (DTensor + colocated), SC (Megatron + non-colocated), so only compare convergence.
    image
  2. async mode: grpo-llama3.1-8b-instruct-2n8g-async-1off-single-controller-streaming2
    SC run uses min_groups_for_streaming_train=num_prompts_per_step, over_sampling=false, force_in_order=true (different from the script in this PR) to keep using the same setting as legacy code for compare. image image

Known Issues (From the above tests results)

  1. grad_norm is ~1/2x smaller: fixed in fix: fix single controller grad_norm #3282.
  2. invalidate_kv_cache only enabled in sync mode, which will cause SC async mode never reset vLLM's prefix cache, tracked in "Known Missing Features" below and [Single Controller / Async RL] cleanup tracking issue #2625.
  3. total_step_time is longer (since policy_and_reference_logprobs is longer), it's because SC's prev_logprobs_required / reference_logprobs_required gating is provisional and didn't skip calculating prev_logprobs in this run, already tracked in [Single Controller / Async RL] cleanup tracking issue #2625.

Known Missing Features (Tracked in #2625 or have PRs)

  1. Checkpointing not wired: feat(sc): add checkpoint save/restore to SingleController #3138, currently landed in 2819, need to merge into main after this PR (3266) go in.
  2. Validation not wired: feat(sc): add native and NeMo-Gym validation #3203, currently landed in 2819, need to merge into main after this PR (3266) go in.
  3. Abort stale in-flight rollouts in refit instead of evicting in train: feat(sc): abort stale in-flight rollouts early under over_sampling #3263, need this PR (3266) get into main first.
  4. Drain gate in refit is not supported yet.
  5. invalidate_kv_cache only enabled in sync mode (max_staleness_versions=1), need to respect to recompute_kv_cache_after_weight_updates.

@yuki-97
yuki-97 requested review from a team as code owners July 17, 2026 13:03
@copy-pr-bot

copy-pr-bot Bot commented Jul 17, 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 CI Relating to CI label Jul 17, 2026
@yuki-97
yuki-97 marked this pull request as draft July 17, 2026 13:07
@yuki-97
yuki-97 force-pushed the yukih/sc-split-3 branch 2 times, most recently from c99f349 to 51924be Compare July 18, 2026 09:21
@yuki-97
yuki-97 changed the base branch from main to yukih/sc-split-2 July 18, 2026 09:22
@yuki-97
yuki-97 force-pushed the yukih/sc-split-3 branch 2 times, most recently from 8d8dbfb to 0ff1e12 Compare July 18, 2026 10:06
@yuki-97
yuki-97 force-pushed the yukih/sc-split-2 branch from a1d41bf to 6580a91 Compare July 18, 2026 13:19
@yuki-97
yuki-97 force-pushed the yukih/sc-split-3 branch 3 times, most recently from e63c189 to 9083b7e Compare July 19, 2026 02:56
@terrykong

Copy link
Copy Markdown
Collaborator

The async_rl knob set this PR exposes (over_sampling, force_in_order, max_staleness_versions, min_groups_for_streaming_train, max_buffered_rollouts) is powerful, but figuring out which combination yields which async-RL behavior currently requires piecing together comments scattered across the three exemplar YAMLs plus the usage table in #3219's description — and some combinations are invalid (force_in_order=true + over_sampling=true) or silently ignored (force_in_order in sync mode).

Could this PR (or an immediate follow-up in the stack) add a user-facing markdown that maps configurations to knob settings? A natural home is a "Single-controller async GRPO" section in docs/guides/async-grpo.md (or a new guide wired into docs/index.md). Suggested skeleton, seeded from this PR's own exemplars:

Mode max_staleness_versions over_sampling force_in_order min_groups_for_streaming_train Exemplar config
Sync / on-policy 0 false true (ignored in sync mode) num_prompts_per_step ...-single-controller-sync.yaml
Async, exact batch→step matching (legacy target_weight semantics) ≥1 false true num_prompts_per_step grpo_math_1B_megatron_single_controller.yaml
Streaming, gated dispatch ("streaming 1" in #3219) ≥1 false false x ≤ num_prompts_per_step
Streaming, over-sampled ("streaming 2") ≥1 true false x ≤ num_prompts_per_step ...-async-1off-single-controller-streaming2.yaml

…plus the invariants that today live only in YAML comments and setup-time raises: over_sampling=false requires max_buffered_rollouts == num_prompts_per_step × (max_staleness_versions + 1); force_in_order=true requires over_sampling=false; sync mode (max_staleness_versions: 0) rejects over_sampling=true and ignores force_in_order; over-sampled data aged past the staleness window is evicted (wasted compute — see the over_sampling_ratio follow-up tracked in #2625).

Asking here rather than on #3219/#3220 because this is the PR where the knobs get their final names and become user-settable (the async_rl block + entrypoint), and the three exemplar YAMLs the table should reference land here.

Generated by Claude Code

@RayenTian
RayenTian force-pushed the yukih/sc-split-2 branch 2 times, most recently from 283ba7f to 09a88ff Compare July 22, 2026 13:34
yuki-97 and others added 25 commits July 24, 2026 02:43
Signed-off-by: Yuki Huang <yukih@nvidia.com>
… each async_rl field

Signed-off-by: Yuki Huang <yukih@nvidia.com>
…sync/streaming variants

Signed-off-by: Yuki Huang <yukih@nvidia.com>
…red yet

Signed-off-by: Yuki Huang <yukih@nvidia.com>
…ights docs

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>
Signed-off-by: Yuki Huang <yukih@nvidia.com>
Signed-off-by: Yuki Huang <yukih@nvidia.com>
…ersions

Signed-off-by: Yuki Huang <yukih@nvidia.com>
Signed-off-by: Yuki Huang <yukih@nvidia.com>
Adapt the split-3 controller, configs, recipes, and tests to the sampler interface introduced by split-2.

Signed-off-by: Terry Kong <terryk@nvidia.com>
Signed-off-by: ruit <ruit@nvidia.com>
Signed-off-by: ruit <ruit@nvidia.com>
Signed-off-by: ruit <ruit@nvidia.com>
Signed-off-by: ruit <ruit@nvidia.com>
Signed-off-by: ruit <ruit@nvidia.com>
Signed-off-by: ruit <ruit@nvidia.com>
Signed-off-by: ruit <ruit@nvidia.com>
Signed-off-by: ruit <ruit@nvidia.com>
Signed-off-by: ruit <ruit@nvidia.com>
Signed-off-by: ruit <ruit@nvidia.com>
@RayenTian

Copy link
Copy Markdown
Contributor

/ok to test 0629fed

Signed-off-by: Yuki Huang <yukih@nvidia.com>
Signed-off-by: ruit <ruit@nvidia.com>
@RayenTian

Copy link
Copy Markdown
Contributor

/ok to test 7c100ad

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

Labels

CI:Lfast Runs a fast test suite and re-use nightly `main` container (but sync dependencies to PRs version) CI Relating to CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants