Skip to content

fix: make rollout sampling deterministic - #1625

Merged
sitabulaixizawaluduo merged 1 commit into
mainfrom
pyq/deterministic-rollout-review-fixes
Aug 21, 2026
Merged

fix: make rollout sampling deterministic#1625
sitabulaixizawaluduo merged 1 commit into
mainfrom
pyq/deterministic-rollout-review-fixes

Conversation

@pyq623

@pyq623 pyq623 commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

Summary

Supersedes #1607 with the deterministic-rollout changes rebased onto current main and separated from the independent dispatcher-correctness work requested during review.

This PR propagates stable per-request sampling seeds through the V1 and V2 inference paths, preserves stable grouped-sample identity and result ordering, and documents the limits of the determinism contract. All behavior remains opt-in.

Determinism contract

  • rollout.deterministic_sampling defaults to false.
  • When deterministic sampling is enabled and no explicit request seed is supplied, V1 and V2 derive stable, distinct seeds from the logical rollout identity and request index.
  • An explicit request-level seed takes precedence over automatic derivation.
  • Grouped deterministic rollouts reject a shared gconfig.seed or eval_gconfig.seed, because that would assign the same seed to every sample. Leave it unset to derive stable per-sample seeds, or use n_samples=1.
  • End-to-end deterministic task-to-weight-version mapping is supported only with max_head_offpolicyness=0; enabling deterministic sampling with asynchronous staleness emits a warning.
  • Concurrent SGLang generation relies on SGLang batch-invariant deterministic inference. An explicit attention backend outside {flashinfer, fa3, triton} emits a warning when that mode is enabled; None remains allowed so SGLang can choose its default.
  • serialize_group_samples guarantees only stable member submission order within each V2 group. It does not serialize requests across groups or claim to stabilize global dynamic-batch composition.

Changes

  • Add GenerationHyperparameters.seed and forward it through OpenAI-compatible requests to vLLM and SGLang, including V2 sampling_seed.
  • Add shared stable seed derivation and thread-safe request-index allocation for V1 proxy and V2 Data Proxy sessions.
  • Pass deterministic_sampling explicitly through DataProxyConfig and its startup command.
  • Preserve logical sampling identity when physical session IDs receive collision suffixes.
  • Keep grouped rollout members associated with stable sample indices while retaining concurrent execution by default.
  • Return completed rollout results in task-ID order without shuffling when deterministic ordering is enabled. This PR intentionally does not retain the submission-order membership frontier from fix: complete deterministic rollout sampling determinism concurrent rollout #1607.
  • Add the opt-in V2 within-group serialization mode and scope its help text, logs, and tests to that narrower guarantee. Per-member start/finish logs are DEBUG-level.
  • Regenerate the English and Chinese CLI references.

The callback registration, task-ID reservation/rollback, dynamic-batch accounting, shutdown, and concurrent-waiter changes discussed on #1607 are intentionally excluded and have moved to the separate dispatcher-correctness PR #1626.

Testing

Executed in the project Slurm development image:

pre-commit run --all-files (uv-lock skipped because dependency inputs are unchanged): passed
focused pytest suite: 196 passed, 5 skipped
git diff --check: passed
Python compilation: passed
CLI documentation generation: passed

The focused suite covers deterministic seed derivation and forwarding, request-index concurrency, explicit-seed precedence, shared grouped-seed validation, SGLang backend warnings, off-policy contract warnings, deterministic completed-result ordering, V2 Data Proxy propagation, and serial/concurrent group behavior.

@sitabulaixizawaluduo sitabulaixizawaluduo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@sitabulaixizawaluduo
sitabulaixizawaluduo merged commit 57a3d83 into main Aug 21, 2026
13 checks passed
@sitabulaixizawaluduo
sitabulaixizawaluduo deleted the pyq/deterministic-rollout-review-fixes branch August 21, 2026 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe-to-test Ready to run unit-tests in a PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants