vLLM resolves unset sampling params from the model's generation_config.json, not neutral defaults
- Dominant language
- Python
- Stars
- 2k
- Forks
- 561
- Avg merge
- 4d 5h
- Merged PRs (30d)
- 145
Description
## Summary
vLLM resolves any **unset** sampling parameter on an incoming OpenAI request from the *model's* `generation_config.json`, not from neutral defaults. NeMo-RL never overrides this, so an omitted `top_p` / `temperature` / `top_k` / `min_p` / `repetition_penalty` silently picks up a model-specific value that differs from `policy.generation.*`.
For RL this is worse than ordinary drift: top-p/top-k truncate and renormalize the distribution, which changes the logprobs. NeMo-RL mirrors that into the training forward pass — [`dtensor_policy_worker` builds `TrainingSamplingParams` from `generation_cfg["top_p"/"top_k"]`](https://github.com/NVIDIA-NeMo/RL/blob/414b9bb6b69539011e835ce369be8e53aad709bd/nemo_rl/models/policy/workers/dtensor_policy_worker.py#L244) and [`_apply_top_k_top_p_filtering` rescales the logits](https://github.com/NVIDIA-NeMo/RL/blob/414b9bb6b69539011e835ce369be8e53aad709bd/nemo_rl/models/policy/workers/dtensor_policy_worker.py#L571) only when [`_need_top_p_filtering` sees `top_p != 1.0`](https://github.com/NVIDIA-NeMo/RL/blob/414b9bb6b69539011e835ce369be8e53aad709bd/nemo_rl/algorithms/logits_sampling_utils.py#L52). If the engine actually samples at a different top_p than the config records, the train-side rescaling replicates the wrong distribution and the mismatch is invisible.
## Mechanism
Against the pinned `vllm==0.25.1`:
1. [`top_p: float | None = None`](https://github.com/vllm-project/vllm/blob/v0.25.1/vllm/entrypoints/openai/chat_completion/protocol.py#L219) — an omitted field stays `None`.
2. [`if (top_p := self.top_p) is None: top_p = default_sampling_params.get("top_p", ...)`](https://github.com/vllm-project/vllm/blob/v0.25.1/vllm/entrypoints/openai/chat_completion/protocol.py#L603-L605) — the neutral `1.0` is only the *last* fallback. Same pattern for `temperature`, `top_k`, `min_p`, `repetition_penalty`.
3. [`default_sampling_params = self.model_config.get_diff_sampling_param()`](https://github.com/vllm-project/vllm/blob/v0.25.1/vllm/entrypoints/openai/chat_completion/serving.py#L168).
4. [`config = {} if src == "vllm" else self.try_get_generation_config()`](https://github.com/vllm-project/vllm/blob/v0.25.1/vllm/config/model.py#L1499-L1501) — docstring: *"vLLM's neutral defaults if `generation_config="vllm"`, the model's defaults if `generation_config="auto"`"*.
5. [`generation_config: str = "auto"`](https://github.com/vllm-project/vllm/blob/v0.25.1/vllm/config/model.py#L290) is the default, and NeMo-RL passes neither `generation_config` nor `override_generation_config` into [`AsyncEngineArgs`](https://github.com/NVIDIA-NeMo/RL/blob/414b9bb6b69539011e835ce369be8e53aad709bd/nemo_rl/models/generation/vllm/vllm_worker_async.py#L191) (grep of `nemo_rl/` → 0 hits).
Concretely, `Qwen/Qwen3-30B-A3B-Instruct-2507` (used by `examples/nemo_gym/grpo_qwen3_30ba3b_instruct.yaml`) ships `top_p: 0.8`, `top_k: 20`, `temperature: 0.7`, while the gym recipes set `top_p: 1.0` / `top_k: null` / `temperature: 1.0`.
## Not always the case — introduced by vLLM v0.8.0
| vLLM | `ModelConfig.generation_config` | effect |
|---|---|---|
| ≤ v0.7.3 | [`Optional[str] = None`](https://github.com/vllm-project/vllm/blob/v0.7.3/vllm/config.py#L244) | [`get_diff_sampling_param` returns `{}`](https://github.com/vllm-project/vllm/blob/v0.7.3/vllm/config.py#L948-L950) — the model's file is ignored |
| ≥ v0.8.0 | `str = "auto"` | the model's `generation_config.json` is read |
## Current mitigation is partial
The async OpenAI server guard is what stands between this and a silent off-policy rollout, and it covers the parameters unevenly:
- **`top_k`** — safe: [`request.top_k = -1`](https://github.com/NVIDIA-NeMo/RL/blob/414b9bb6b69539011e835ce369be8e53aad709bd/nemo_rl/models/generation/vllm/vllm_worker_async.py#L678) explicitly pins it, so vLLM never consults the model default.
- **`temperature`** — safe: [compared strictly](https://github.com/NVIDIA-NeMo/RL/blob/414b9bb6b69539011e835ce369be8e53aad709bd/nemo_rl/models/generation/vllm/vllm_worker_async.py#L682), so `None` trips the assert.
- **`top_p`** — [compared strictly on `main`](https://github.com/NVIDIA-NeMo/RL/blob/414b9bb6b69539011e835ce369be8e53aad709bd/nemo_rl/models/generation/vllm/vllm_worker_async.py#L683), but #3401 coalesces an unset value to a literal `1.0` before comparing, which makes the omitted case pass. Being handled in that PR's review.
- **`min_p` / `repetition_penalty`** — not covered by the guard at all.
Note the guard only protects the **async OpenAI-server** path; it is not a general property of the engine.
## Proposed fix
Pass `generation_config="vllm"` when constructing [`AsyncEngineArgs`](https://github.com/NVIDIA-NeMo/RL/blob/414b9bb6b69539011e835ce369be8e53aad709bd/nemo_rl/models/generation/vllm/vllm_worker_async.py#L191) (and the sync engine equivalent), so unset parameters resolve to vLLM's neutral defaults instead of per-model values. That removes the whole class rather than patching one parameter at the guard, and makes `policy.generation.*` the single source of truth — which is what the train-side logprob rescaling already assumes.
Worth deciding explicitly whether any model-default behavior is wanted for **validation** rollouts before changing it globally.
_(Surfaced during the review of #3401.)_
Contributor guide
Research direction
Start in nemo_rl/models/generation/vllm/vllm_worker_async.py at the AsyncEngineArgs construction, then locate the sync engine equivalent. Compare how unset sampling parameters reach vLLM and how policy.generation values are used for training logprobs. Decide whether validation rollouts should retain model defaults; done means both engine paths consistently use the intended neutral or model-specific behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- machine-learning
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 55/100