-
Notifications
You must be signed in to change notification settings - Fork 582
fix: complete deterministic rollout sampling determinism concurrent rollout #1607
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -222,6 +222,10 @@ class GenerationHyperparameters: | |
| ) | ||
| }, | ||
| ) | ||
| seed: int | None = field( | ||
| default=None, | ||
| metadata={"help": "Per-request sampling seed sent to the inference backend."}, | ||
| ) | ||
| lora_name: str = field( | ||
| default="default_lora", | ||
| metadata={"help": "Lora name to be used for this generation."}, | ||
|
|
@@ -2065,6 +2069,11 @@ def build_cmd( | |
| return vLLMConfig.build_cmd_from_args(args) | ||
|
|
||
|
|
||
| # Keep this list aligned with SGLang's deterministic inference documentation: | ||
| # https://docs.sglang.ai/advanced_features/deterministic_inference.html | ||
| _SGLANG_DETERMINISTIC_ATTENTION_BACKENDS = frozenset({"flashinfer", "fa3", "triton"}) | ||
|
|
||
|
|
||
| @dataclass | ||
| class SGLangConfig: | ||
| """Configuration for SGLang runtime. Refer to: | ||
|
|
@@ -2098,6 +2107,7 @@ class SGLangConfig: | |
| enable_memory_saver: bool = False | ||
| allow_auto_truncate: bool = False | ||
| attention_backend: str | None = "fa3" | ||
| enable_deterministic_inference: bool = False | ||
| enable_multimodal: bool = False | ||
| sampling_backend: str | None = None | ||
| context_length: int | None = 32768 | ||
|
|
@@ -2181,6 +2191,19 @@ def build_args( | |
| node_rank: int = 0, | ||
| pp_size: int = 1, | ||
| ): | ||
| attention_backend = sglang_config.attention_backend | ||
| if ( | ||
| sglang_config.enable_deterministic_inference | ||
| and attention_backend is not None | ||
| and attention_backend.lower() | ||
| not in _SGLANG_DETERMINISTIC_ATTENTION_BACKENDS | ||
| ): | ||
| logger.warning( | ||
| "SGLang deterministic inference is only documented for attention " | ||
| "backends %s; configured attention_backend=%r may be non-deterministic.", | ||
| sorted(_SGLANG_DETERMINISTIC_ATTENTION_BACKENDS), | ||
| attention_backend, | ||
| ) | ||
| # Map "all-linear" to "all" | ||
| args: dict = conf_as_dict(sglang_config) | ||
| if sglang_config.enable_multithread_load: | ||
|
|
@@ -2415,6 +2438,25 @@ class InferenceEngineConfig: | |
| "help": "Whether to output verbose tracing messages for each generation request." | ||
| }, | ||
| ) | ||
| deterministic_sampling: bool = field( | ||
| default=False, | ||
| metadata={ | ||
|
Comment on lines
+2441
to
+2443
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you clarify the determinism contract for If asynchronous staleness is supported, how does this implementation guarantee If end-to-end determinism is only supported with
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You’re right — this implementation does not guarantee a stable task-to-weight-version mapping when max_head_offpolicyness > 0. End-to-end determinism is currently supported only with max_head_offpolicyness=0. Addressed in efef650:
Supporting deterministic asynchronous rollout would require a separate contract, likely binding each task to a model version at submission time and defining retry/rejection behavior around that binding. That is outside the scope of this PR. |
||
| "help": "Use stable request seeds for internal OpenAI-proxy/data-proxy " | ||
| "sessions, canonical group ordering, and task-ID ordering of completed " | ||
| "rollout results. Concurrent SGLang generation also requires " | ||
| "sglang.enable_deterministic_inference. End-to-end determinism is only " | ||
| "supported with max_head_offpolicyness=0." | ||
| }, | ||
| ) | ||
| serialize_group_samples: bool = field( | ||
| default=False, | ||
| metadata={ | ||
| "help": "Run RolloutControllerV2 samples within each group sequentially " | ||
| "instead of concurrently. This provides stable within-group member " | ||
| "submission order at the cost of rollout throughput; it does not " | ||
| "serialize requests across groups." | ||
| }, | ||
| ) | ||
| check_trajectory_format: bool = field( | ||
| default=False, | ||
| metadata={ | ||
|
|
@@ -2559,6 +2601,13 @@ def __post_init__(self): | |
| ) | ||
| if not self.admin_api_key or not self.admin_api_key.strip(): | ||
| raise ValueError("admin_api_key must not be empty or whitespace-only") | ||
| if self.deterministic_sampling and self.max_head_offpolicyness > 0: | ||
| logger.warning( | ||
| "deterministic_sampling=True with max_head_offpolicyness=%d does " | ||
| "not guarantee deterministic task-to-weight-version mapping; " | ||
| "set max_head_offpolicyness=0 for end-to-end determinism.", | ||
| self.max_head_offpolicyness, | ||
| ) | ||
| if ( | ||
| self._version == "v2" | ||
| and self.agent is not None | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point — addressed in efef650.
SGLangConfig.build_args() now emits a warning when enable_deterministic_inference=True and an explicitly configured attention_backend is outside {flashinfer, fa3, triton}. attention_backend=None remains allowed so SGLang can select its default backend.
I also added tests covering all documented backends, None, and an explicitly unsupported backend.