fix: complete deterministic rollout samplingPyq/complete determinism concurrent rollout - #1607
Conversation
Derive stable sampling seeds for OpenAI proxy sessions and preserve canonical rollout group order while inference requests run concurrently. Consume completed work through a submission-order frontier so rollout completion timing cannot change training batch membership. Forward request seeds and deterministic-inference configuration to SGLang, and bind callbacks after task ID allocation.
| if "top_p" not in kwargs: | ||
| kwargs["top_p"] = 1.0 | ||
|
|
||
| deterministic_sampling = session is not None and get_bool_env_var( |
There was a problem hiding this comment.
The issue is that the command to start Data Proxy did not pass this configuration or environment variable, so the V2 requests still will not automatically generate a seed, and the subsequent SGLang sampling_seed forwarding also cannot obtain the value.
Suggestion: Add deterministic_sampling to DataProxyConfig and startup parameters, or explicitly pass the environment variable when forking Data Proxy.
|
The vllm-related part has not been fixed in the related part. I understand that deterministic_sampling is a backend-independent common configuration. If convenient, can you include the vllm part update and supplement the relevant UT? |
| task_id in self._pending_results for task_id in task_frontier | ||
| ) | ||
| else: | ||
| results_ready = len(self._pending_results) >= count |
There was a problem hiding this comment.
In the synchronous, no-rejection case, the staleness manager allows only one
consumer batch to run per model version. Later task IDs cannot complete before
the current batch, so sorting completed results by task ID and disabling
shuffle appears sufficient; the frozen membership frontier seems redundant.
Is the frontier intended specifically for rejection/timeout or
submit-many/wait-few scenarios? If so, could that behavior be scoped and tested
separately?
| deterministic_sampling: bool = field( | ||
| default=False, | ||
| metadata={ |
There was a problem hiding this comment.
Could you clarify the determinism contract for
max_head_offpolicyness > 0?
If asynchronous staleness is supported, how does this implementation guarantee
a stable task-to-weight-version mapping across runs? The inference version is
read when a generation request is actually sent, so the same logical task may
use different model versions depending on scheduling timing. Stable seeds,
result ordering, and a membership frontier do not appear to fix that.
If end-to-end determinism is only supported with
max_head_offpolicyness=0, should the configuration emit a warning or document
that requirement explicitly?
| logger.info( | ||
| "V2 rollout member start: task_id=%s group_id=%s member=%d " | ||
| "session_id=%s version=%s mode=%s", | ||
| task_id, | ||
| group_id, | ||
| member_index, | ||
| session_id, | ||
| version, | ||
| execution_mode, | ||
| ) |
There was a problem hiding this comment.
Could the per-member start/finish messages be moved to DEBUG or gated by
enable_rollout_tracing? This emits two INFO lines per trajectory plus one line
per group. With batch_size=16 and n_samples=8 that is at least 272 additional
INFO lines per training step.
| @@ -2098,6 +2102,7 @@ class SGLangConfig: | |||
| enable_memory_saver: bool = False | |||
| allow_auto_truncate: bool = False | |||
| attention_backend: str | None = "fa3" | |||
There was a problem hiding this comment.
SGLang documents deterministic inference support only for the flashinfer, fa3,
and triton attention backends. At the moment enable_deterministic_inference is
forwarded for any configured backend, which can give users a false
determinism guarantee.
Could we emit a warning when deterministic inference is enabled with an
explicit attention_backend outside {flashinfer, fa3, triton}? None can remain
allowed because it delegates to the SGLang default.
| if self.serialize_group_samples: | ||
| results = [] | ||
| for member_index, (session_id, session_api_key) in enumerate(sessions): | ||
| results.append( | ||
| await _run_one(member_index, session_id, session_api_key) | ||
| ) | ||
| else: | ||
| results = await asyncio.gather( | ||
| *[ | ||
| _run_one(member_index, session_id, session_api_key) | ||
| for member_index, (session_id, session_api_key) in enumerate( | ||
| sessions | ||
| ) | ||
| ] | ||
| ) |
There was a problem hiding this comment.
I don't think serializing members within one group is sufficient to stabilize
SGLang batch composition. Multiple prompt groups still execute concurrently,
so a serialized member from group A can be co-batched with requests from group
B in timing-dependent ways.
This guarantees per-group member order, but not “strict reproducibility” of
dynamic batching. Could we remove or weaken this claim and rely on SGLang
batch-invariant inference instead? A true serialization fallback would need a
global request scheduler, not a per-group loop.
Summary
Complete deterministic rollout sampling across concurrent rollout scheduling and the V1/V2 inference paths.
This PR makes rollout identity, request seed assignment, and result ordering reproducible while preserving concurrent execution by default. It also provides an explicit opt-in serial mode for strict V2 reproducibility when SGLang dynamic batching affects numerical identity.
Motivation
Deterministic inference requires more than enabling deterministic kernels on the SGLang server.
Previously, several gaps remained:
seedcould reachArealOpenAIwithout being carried through the complete request path;GenerationHyperparameters.seedas SGLangsampling_seed;asyncio.gather, so their SGLang dynamic-batch composition could vary between runs.Together, these gaps could make repeated runs diverge even when the global training seed and SGLang deterministic inference were enabled.
Changes
Stable concurrent rollout identity and ordering
End-to-end sampling seed propagation
The request path is now:
This path is covered for:
Explicit caller-provided seeds always take precedence over automatically derived seeds.
Shared seed derivation
Add a shared derive_deterministic_seed(identity, request_index) helper so V1 and V2 use the same stable derivation logic.
The generated seed is:
Optional strict V2 group serialization
Add the explicit configuration:
InferenceEngineConfig.serialize_group_samples: bool = False
Behavior:
False:
samples within a V2 group continue to run concurrently with asyncio.gather
True:
samples run sequentially in stable member order
member 0 -> member 1 -> member 2 -> ...
This option is intentionally independent from deterministic_sampling.
deterministic_sampling stabilizes identities, seeds, scheduling, and result ordering while retaining concurrency. serialize_group_samples additionally stabilizes request arrival and SGLang batching conditions for strict reproducibility, at the expected cost of rollout throughput.
The default remains concurrent, so existing workloads are unaffected.
Observability
V2 workflow logs now include:
These fields make it possible to verify the effective rollout execution path from runtime logs.
Failure handling
Serial execution preserves the existing group-cleanup behavior:
Documentation
Regenerate the English and Chinese CLI references with the new serialize_group_samples option.
Compatibility
All deterministic behavior remains opt-in:
Testing
Focused tests were executed in the project Slurm training image:
tests/test_deterministic_sampling.py
tests/v2/inference_service/test_controller.py
tests/v2/inference_service/test_data_proxy_chat.py
118 passed, 4 skipped
The tests cover:
Additional checks:
Ruff lint/format: passed
mdformat: passed
git diff --check: passed
Python compilation: passed
CLI documentation generation: passed
The remaining warning is an existing third-party torchao SyntaxWarning.