Skip to content

fix: expose DDP model view to online rollout - #55

Open
0z5a wants to merge 1 commit into
ViperEkura:mainfrom
0z5a:codex/ddp-online-rollout-main-0z5a
Open

fix: expose DDP model view to online rollout#55
0z5a wants to merge 1 commit into
ViperEkura:mainfrom
0z5a:codex/ddp-online-rollout-main-0z5a

Conversation

@0z5a

@0z5a 0z5a commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add an explicit executor-owned inference-model boundary for online rollout
  • pass DDP.module to InferenceScheduler while retaining the wrapped DDP model for training and gradient synchronization
  • fail fast for distributed FSDP and torch.compile online-rollout configurations before model/scheduler construction
  • document the ownership contract and check in reproducible L20 + InfraSWE evidence

Why

TrainContextBuilder currently passes the training model wrapper directly to inference and reads context.model.config. A real DistributedDataParallel wrapper does not expose that attribute, so online GRPO/DPO fails while constructing its scheduler even though each DDP rank already owns a replicated underlying module.

The executor now declares whether an in-process rollout is supported and returns the model view inference may own. This also prevents sharded FSDP parameters from entering an unsupported generation path by accident.

Validation

Local

  • ruff format --check .: 167 files formatted
  • ruff check . --select I: passed
  • focused CPU/Gloo boundary tests: 6 passed, 1 deselected
  • complete suite: 626 passed, 104 skipped in 12.57s

NVIDIA L20 / SM89

Environment: NVIDIA L20, driver 570.86.10, CUDA 12.8, PyTorch 2.11.0+cu128, Python 3.12.12.

  • all enabled CUDA extensions rebuilt for sm_89
  • focused boundary tests: 7 passed in 1.54s
  • complete built suite: 730 passed in 52.14s
  • real CUDA/NCCL DistributedDataParallel wrapper + InferenceScheduler: the executor's inference view produced exactly the same greedy token lists as the underlying replicated model

The CUDA DDP probe uses world_size=1. A two-rank GPU run was not performed because two unowned idle GPUs were unavailable; this PR makes no multirank throughput claim. Full evidence is in docs/benchmarks/ddp_rollout_l20_sm89.json.

InfraSWE before PR

InfraSWE commit 811bc775ed5b3a6ec853219245f3469f78818020 was run before opening this PR:

  • Draft/system-path engine subsets: 53 passed
  • native ProjectComparisonCell validation: passed
  • diagnostic project-fit-system-path-v0.5.1: 92.16/100
  • diagnostic BenchmarkTrust: 89.79/100
  • official score: unresolved, as expected without a seal, five fresh-process replays, system traces, hidden probes, and verified manifest

The score is explicitly non-official and records the missing two-rank cell and absence of a performance-statistics claim. Inputs and rationale are in benchmarks/results/ddp_rollout_l20_sm89_infraswe_score.json.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant