-
Notifications
You must be signed in to change notification settings - Fork 419
feat: top-p/top-k train sampling with sampling replay #2979
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
Closed
Closed
Changes from all commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
7f3173e
feat: top-p kept-set sampling-mask replay (inference capture + traine…
faresoPrime bed890b
fix: harden kept-set replay from adversarial review
faresoPrime 1f06a33
refactor: simplify kept-set replay from cleanup review
faresoPrime 17b636d
feat: auto-enable sampling-mask replay when train sampling truncates
faresoPrime 822e583
feat: first-class train sampling top_k, cap auto-raised to keep repla…
faresoPrime a17915e
feat: replay forces a top-k bound; kept_tokens_max becomes derived
faresoPrime ac6b874
refactor: simplify replay config wiring from cleanup review
faresoPrime 2f5510b
docs: show top_k in the sampling-mask replay example
faresoPrime 6c09dda
feat: hard guards replace warnings for replay-incompatible consumers
faresoPrime a5fb85b
refactor: flagless replay — truncation policy owned by OrchestratorCo…
faresoPrime 2a152e0
docs: tighten the sampling-mask replay section
faresoPrime 6da72e0
chore: trim comments to load-bearing constraints
faresoPrime e89b6c1
fix: resolved configs re-validate — ban truncating extra_body values,…
faresoPrime 5e9fd2f
chore: bump verifiers for kept-token ruff formatting
faresoPrime 181f4c4
address review: rename to sampling replay, drop min_p, mark kept_toke…
faresoPrime 52c78ff
chore: drop box-local ablation configs from branch
faresoPrime 3fdbe83
address review: KeptTokens dataclass in verifiers, drop renderers spl…
faresoPrime 1f1d80c
chore: keep dependency pins current after rebase
samsja 06147dd
Merge remote-tracking branch 'origin/main' into feat/top-p-mask-replay
mikasenghaas c818923
fix: restore warnings import lost in merge
mikasenghaas File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| import warnings | ||
| from pathlib import Path | ||
| from typing import Annotated, Any, Literal, TypeAlias | ||
|
|
||
|
|
@@ -53,6 +54,16 @@ class TrainSamplingConfig(BaseConfig): | |
| temperature: float = Field(1.0, ge=0, le=2.0) | ||
| """Sampling temperature.""" | ||
|
|
||
| top_p: float = Field(1.0, gt=0, le=1.0) | ||
|
mikasenghaas marked this conversation as resolved.
|
||
| """Nucleus (top-p) sampling for train rollouts. Values below 1.0 truncate the sampling | ||
| distribution; the ``rl`` entrypoint auto-enables sampling replay so trainer and | ||
| rollout distributions stay consistent — see docs/inference.md (Sampling Replay).""" | ||
|
|
||
| top_k: int | None = Field(None, ge=1) | ||
|
Member
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. should we have a |
||
| """Top-k sampling for train rollouts. Truncation triggers sampling replay, and | ||
| a default top-k is injected when only top-p truncates so kept sets stay | ||
| bounded — see docs/inference.md (Sampling Replay).""" | ||
|
|
||
| max_completion_tokens: int | None = None | ||
| """Maximum output tokens per turn. If None, generates until max context length or EOS.""" | ||
|
|
||
|
|
@@ -61,18 +72,46 @@ class TrainSamplingConfig(BaseConfig): | |
| extra_body: dict[str, Any] = {} | ||
| """Extra body forwarded with each request to the inference server.""" | ||
|
|
||
| def truncates_distribution(self) -> bool: | ||
| return self.top_p < 1.0 or self.top_k is not None | ||
|
|
||
| @model_validator(mode="after") | ||
| def validate_no_extra_body_truncation(self): | ||
| """Truncating values must come from the typed fields — the replay policy reads | ||
| them. Disabled values pass so resolved configs (where ``resolve_env_config`` | ||
| stamped the ``top_k = -1`` / ``min_p = 0.0`` sentinels) re-validate cleanly.""" | ||
| smuggled = [ | ||
|
mikasenghaas marked this conversation as resolved.
|
||
| key | ||
| for key, truncates in ( | ||
| ("top_p", self.extra_body.get("top_p", 1.0) < 1.0), | ||
| ("top_k", self.extra_body.get("top_k") not in (None, -1, 0)), | ||
| ("min_p", self.extra_body.get("min_p", 0.0) > 0.0), | ||
| ) | ||
| if truncates | ||
| ] | ||
| if smuggled: | ||
| raise ValueError( | ||
| f"extra_body carries truncating {smuggled}; set them as fields on the train " | ||
| "sampling config instead (they drive sampling replay)." | ||
| ) | ||
| return self | ||
|
|
||
| def to_sampling_args(self) -> dict[str, Any]: | ||
| """Convert to OAI-compatible sampling args dict, omitting None values.""" | ||
| args: dict[str, Any] = { | ||
| "temperature": self.temperature, | ||
| "top_p": 1.0, | ||
| "top_p": self.top_p, | ||
| "logprobs": True, | ||
| } | ||
| if self.max_completion_tokens is not None: | ||
| args["max_completion_tokens"] = self.max_completion_tokens | ||
|
|
||
| if self.extra_body: | ||
| args["extra_body"] = dict(self.extra_body) | ||
| # top_k rides extra_body (like EvalSamplingConfig), overriding the sentinel. | ||
| extra_body = dict(self.extra_body) | ||
| if self.top_k is not None: | ||
| extra_body["top_k"] = self.top_k | ||
| if extra_body: | ||
| args["extra_body"] = extra_body | ||
|
|
||
| return args | ||
|
|
||
|
|
@@ -444,6 +483,12 @@ class NIXLWeightBroadcastConfig(InMemoryWeightBroadcastConfig): | |
| ] | ||
|
|
||
|
|
||
| # Top-k injected on truncated policy sampling that has none: large enough that a | ||
| # 0.95-0.99 nucleus rarely reaches it (the sampling policy is essentially unchanged), | ||
| # small enough to bound the kept-set capture width and trainer mask tensors. | ||
| DEFAULT_TRAIN_TOP_K = 512 | ||
|
|
||
|
|
||
| class OrchestratorConfig(BaseConfig): | ||
| algo: AlgoConfig = GRPOAlgoConfig() | ||
| """Training algorithm: sampling plus the per-token training signal (credit | ||
|
|
@@ -614,6 +659,56 @@ def validate_env_algorithms(self): | |
| env_cfg.algo.validate_env(env_cfg.env) | ||
| return self | ||
|
|
||
| @model_validator(mode="after") | ||
| def setup_truncated_sampling(self): | ||
| """Truncated policy sampling trains with sampling replay (rollout | ||
| logprobs are renormalized — see docs/inference.md, Sampling Replay). | ||
| Owned here: every truncating config gets a top-k bound (bounds the kept | ||
| sets); opd/opsd is rejected (full-vocab prefill refs would mix | ||
| normalizations); the gibberish/repetition filters are pruned or rejected | ||
| (their full-softmax thresholds misfire on renormalized logprobs). | ||
| Frozen-source envs sample externally and are exempt.""" | ||
| policy_samplings = [ | ||
| env.sampling for env in self.train.source if env.algo is not None and env.algo.sampling.source == "policy" | ||
| ] or ([self.train.sampling] if not self.train.source else []) | ||
| truncating = [sampling for sampling in policy_samplings if sampling.truncates_distribution()] | ||
| if not truncating: | ||
| return self | ||
|
|
||
| unbounded = [sampling for sampling in truncating if sampling.top_k is None] | ||
| if unbounded: | ||
| warnings.warn( | ||
| f"Truncated train sampling: defaulting top_k = {DEFAULT_TRAIN_TOP_K} so every kept set is " | ||
| "bounded and sampling replay stays exact. Set top_k explicitly to override.", | ||
| stacklevel=2, | ||
| ) | ||
| for sampling in unbounded: | ||
| sampling.top_k = DEFAULT_TRAIN_TOP_K | ||
|
|
||
| algos = [env.algo for env in self.train.source if env.algo is not None] or [self.algo] | ||
| if any(algo.type in ("opd", "opsd") for algo in algos): | ||
| raise ValueError( | ||
| "opd/opsd is not supported with truncated train sampling: reference logprobs are full-vocab " | ||
| "prefill scores while trainer logprobs are renormalized over the kept set, biasing the " | ||
| "ref_kl term. Remove the truncation (top_p/top_k) or the opd/opsd algo." | ||
| ) | ||
|
cursor[bot] marked this conversation as resolved.
|
||
|
|
||
| logprob_filter_types = ("gibberish", "repetition") | ||
| for slot_name in ("pre_batch_filters", "post_batch_filters"): | ||
| filters = getattr(self, slot_name) | ||
| if not any(f.type in logprob_filter_types for f in filters): | ||
| continue | ||
| if slot_name in self.model_fields_set: | ||
| raise ValueError( | ||
| f"{slot_name} contains logprob-based filters " | ||
| f"({[f.type for f in filters if f.type in logprob_filter_types]}) which misfire under " | ||
| "truncated sampling: rollout logprobs are renormalized over the kept set, so " | ||
| "full-softmax thresholds over-detect repetition and under-detect gibberish. Remove them " | ||
| "from the list (zero_advantage is unaffected)." | ||
| ) | ||
| setattr(self, slot_name, [f for f in filters if f.type not in logprob_filter_types]) | ||
| return self | ||
|
|
||
| @property | ||
| def any_policy_sourced(self) -> bool: | ||
| """True when at least one train env samples rollouts from the live policy.""" | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.