NVIDIA-NeMo / NVIDIA-NeMo/RL

docs: add sampling-params reference covering per-backend top_k/top_p/temperature semantics

Open
#3,776 0 comments 0 reactions 0 assignees View on GitHub
Documentation
Dominant language
Python
Stars
2k
Forks
561
Avg merge
4d 5h
Merged PRs (30d)
145

Description

## Summary

There is no single place that explains how sampling parameters (`temperature` / `top_p` / `top_k`) are configured and applied across NeMo-RL's generation backends. `docs/design-docs/generation.md` lists them as config fields but never says how each backend interprets them — and the backends genuinely disagree, including on what "disabled" means.

Proposal: add `docs/design-docs/sampling-params.md` (and link it from `docs/index.md`), covering the two questions below.

## 1. How do you set up sampling params across all the backends?

The config surface is one thing (`policy.generation.{temperature,top_p,top_k}`), but the sentinel for "no top-k restriction" is **not** the same everywhere. Verified against `main`:

| path | "disabled" value | honors `cfg["top_k"]`? | source |
|---|---|---|---|
| vLLM `generate()` | `-1` | yes | `nemo_rl/models/generation/vllm/vllm_worker.py` |
| **vLLM HTTP** (NeMo-Gym rollouts) | forced `-1` | **no — config ignored** | `nemo_rl/models/generation/vllm/vllm_worker_async.py` |
| SGLang | `-1` (key omitted entirely when `-1`) | yes | `nemo_rl/models/generation/sglang/sglang_generation.py` |
| TRT-LLM `generate()` | `0` | yes | `nemo_rl/models/generation/trtllm/trtllm_worker_async.py` |
| TRT-LLM HTTP | `0` | yes (since #3537) | `nemo_rl/models/generation/trtllm/trtllm_http_server.py` |
| Megatron | `0` | yes | `nemo_rl/models/generation/megatron/megatron_worker.py` |

Three things a doc should make explicit:

1. **Two different sentinels.** vLLM/SGLang spell "disabled" as `-1`; TRT-LLM/Megatron spell it `0`. TRT-LLM additionally **rejects negatives** (`require top_k >= 0`), so a config written with `top_k: -1` — which is what `examples/configs/evals/eval.yaml` uses, commented `# -1 means disable` — raises at request time on a TRT-LLM run. The YAML-level neutral value is `null`; that is the portable way to say "unrestricted".
2. **The vLLM HTTP path ignores `top_k` entirely.** It asserts `request.top_k in (None, -1)` then forces `-1`, with a comment claiming it matches `BaseVllmGenerationWorker::_build_sampling_params` — which is not true when `cfg["top_k"]` is set. So the same config yields different sampling on vLLM-HTTP vs TRT-LLM-HTTP for agentic rollouts. Either the vLLM HTTP path should thread `top_k` like TRT-LLM now does, or the doc should state that agentic rollouts are top-k-free on vLLM by design.
3. **Greedy is uniform** — every backend uses `top_k=1` for greedy. Worth stating once so nobody re-derives it.

## 2. How do you enable top-k / top-p / temperature-aware sampling?

Support was added in #2053 (`feat: support top-p top-k in grpo`). The doc should carry a worked example from that PR — which config keys to set, what they do to the rollout distribution, and the interaction with importance sampling / off-policy correction.

Also worth referencing #3537 (`fix(trtllm): Align HTTP server sampling params, use generate_async`), whose description contains a good direct-vs-HTTP drift table and is the reason TRT-LLM's two paths now agree.

## Why this is worth writing down

The drift is not hypothetical — #3537 exists because TRT-LLM's HTTP path had silently ignored `top_k` relative to its own `generate()` path. The same class of divergence still exists between vLLM-HTTP and every other path. A short reference page makes that visible instead of something each reader rediscovers by reading six `_build_sampling_params` implementations.

Suggested scope: one page, the table above, the `null` vs `-1` vs `0` rule, a worked example from #2053, and a pointer from `docs/design-docs/generation.md`.

/cc @terrykong

Contributor guide

Open the contributing guide

Research direction

Start with docs/design-docs/generation.md and compare the backend sources named in the issue: the vLLM, SGLang, TRT-LLM, and Megatron generation workers and HTTP paths. Review #2053 and #3537 for the worked example and HTTP behavior. Done means adding docs/design-docs/sampling-params.md with the sentinel table, sampling example, greedy behavior, and a link from docs/index.md.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
documentation
Issue type
Documentation
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.