NVIDIA-NeMo / NVIDIA-NeMo/RL

vLLM resolves unset sampling params from the model's generation_config.json, not neutral defaults

Open
#3,497 1 comment 0 reactions 0 assignees View on GitHub
accuracy bug
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.