Repository navigation
docs: add a sampling-params reference for the generation backends - #3994
Open
yupengtang wants to merge 1 commit into
Open
yupengtang wants to merge 1 commit into
yupengtang wants to merge 1 commit into
Conversation
`docs/design-docs/generation.md` lists temperature / top_p / top_k as config fields but never says how each backend reads them, and they differ. Adds `docs/design-docs/sampling-params.md`, links it from the design-docs toctree, and points at it from the generation doc. The part worth writing down is that no integer means "no top-k restriction" on every backend. vLLM, SGLang and Megatron take -1, TRT-LLM's SamplingParams rejects negatives, and SGLang rejects 0, which the other three read as unrestricted. `null` is the portable value. `examples/configs/evals/eval.yaml` currently uses -1, commented "-1 means disable". Also documents the per-path translation of `null`, what each HTTP server does with sampling values sent on a request (vLLM and TRT-LLM assert them against the config, Megatron uses the config as its default), that the vLLM HTTP path pins top_k to -1 whatever the config says, and how greedy is resolved. Checked against main and the engine versions pinned in pyproject.toml (vLLM 0.29.0, TRT-LLM 1.3.0rc28, SGLang 3003d70f). Closes NVIDIA-NeMo#3776 Signed-off-by: Yupeng Tang <85978465+yupengtang@users.noreply.github.com>
yupengtang
force-pushed
the
docs/sampling-params-reference
branch
from
October 13, 2026 00:55
ff28b61 to
8b9d194
Compare
Author
|
Rebased on main. vLLM, TRT-LLM and the Megatron HTTP server all changed after I opened this, so I went through the page again against the current code; what changed is listed in the description. Happy to adjust anything once someone has a chance to look. |
This branch has not been deployed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What does this PR do ?
Adds
docs/design-docs/sampling-params.md, a reference for how each generation backend interpretstemperature/top_p/top_k.generation.mdlists them as config fields but never says how the backends read them, and they genuinely disagree, including on what "disabled" means. The page covers the two questions in #3776: how to set the params across backends, and what actually reaches each engine.Linked from the design-docs toctree in
docs/index.mdand from theGenerationConfigblock indocs/design-docs/generation.md.Issues
Closes #3776.
Usage
The headline rule the page exists to state:
top_k: null-1, unrestricted0, unrestricted0, unrestrictedtop_k: -1require top_k >= 0top_k: 0top_k must be -1 (disable) or at least 1So
nullis the only value that means "unrestricted" everywhere, andexamples/configs/evals/eval.yaml'stop_k: -1 # -1 means disableraises at request time on a TRT-LLM run. I left that config alone since it is a behaviour change rather than a docs one; happy to fix it here or in a follow-up, whichever you prefer.Before your PR is "Ready for review"
Pre checks:
Docs-only, so no tests.
python -m sphinx -b html docs/ <out>builds, the new page renders with its three tables, and no warning mentions any of the three files.Additional Information
Verification
Rebased onto
mainon Oct 12. Several things the page describes moved after I opened this, so I went through every statement again, against the current code and against the engines at the versionspyproject.tomlpins:SamplingParamsdocumentstop_k: 0as "consider all tokens" and still accepts-1. The first version of this page said0was a literal top-k of 0 on vLLM. That is no longer true, so thenull/-1/0table now has one column per engine.top_k: 0. At the pinned rev (3003d70f),SamplingParams.verifyraisestop_k must be -1 (disable) or at least 1, and NeMo-RL forwards a configured0as it is. This is the mirror of the TRT-LLM-1case and the page now says so.require top_k >= 0in its ownSamplingParams, whose docstring documents0as "all logits". There is no such check innemo_rl/models/generation/trtllm/.-1. Megatron-LM'sSamplingParamsturns anytop_k <= 0into0. The first version put Megatron in the same column as TRT-LLM for that row, which was wrong.policy.generation. The per-path table has a row for it, and there is a short new section on what each HTTP server does with values sent on the request: vLLM and TRT-LLM assert them against the config, Megatron uses the config only as a default.Two details differ from the table in the issue:
Greedy is not uniform across all paths. Every path that takes a
greedyflag resolves it totemperature=0.0, top_k=1:BaseVllmGenerationWorker._build_sampling_params,generate_text/generate_text_async,sglang_generation.py,trtllm_worker_async.py,megatron_worker.py. The HTTP servers have nogreedyparameter at all; over HTTP, greedy is whatever the caller puts on the request.SGLang drops the key. It does not send
top_k: -1; it omitstop_kfrom the request dict when the resolved value is-1.Scope
Kept to the suggested scope: the per-path table, the
nullvs-1vs0rule, the vLLM-HTTPtop_kgap, greedy, and a pointer fromgeneration.md, plus the HTTP request section above. It references #2053 for where top-p/top-k support came from and #3537 for the TRT-LLM direct-vs-HTTP alignment.I did not change any behaviour. In particular the vLLM HTTP path still ignores
top_k, and the page says so explicitly rather than pretending the config applies. If you would rather that path threadtop_klike TRT-LLM's now does, that is a code change and I am happy to open it separately.