NVIDIA-NeMo / NVIDIA-NeMo/RL

hf_config_overrides propagation silently discards policy.generation.vllm_kwargs.hf_overrides

Open
#3,728 0 comments 0 reactions 0 assignees View on GitHub
accuracy bug community-request
Dominant language
Python
Stars
2k
Forks
561
Avg merge
4d 5h
Merged PRs (30d)
145

Description

**Describe the bug**

Five call sites propagate `policy.hf_config_overrides` to the generation engine by
wholesale assignment:

```python
## make vllm hf overrides match the training policy
vllm_kwargs["hf_overrides"] = policy_config.get("hf_config_overrides", {})
```

| file | line | backend |
|---|---|---|
| `nemo_rl/algorithms/grpo.py` | 1345 | vllm |
| `nemo_rl/algorithms/grpo.py` | 496 | dynamo |
| `nemo_rl/algorithms/ppo.py` | 817 | vllm |
| `nemo_rl/algorithms/distillation.py` | 514 | vllm |
| `nemo_rl/algorithms/single_controller_utils/setup.py` | 227 | vllm |

`vllm_kwargs` is a user-facing config field (`VllmConfig.vllm_kwargs`, and
`DynamoConfig.vllm_kwargs` on the dynamo path), so whatever the user set under
`policy.generation.vllm_kwargs.hf_overrides` is thrown away here — and since the default
is `{}`, it is thrown away even when the policy declares no overrides of its own.
Nothing is logged, so the config still reads as if the override were in effect while the
engine is initialized with `hf_overrides={}`. That silence is the expensive part: there
is no signal at launch, and the mismatch only shows up later in downstream metrics.

This is the same overwrite-vs-merge bug that was already fixed one layer down in the
worker: `_merge_fp8_kwargs` exists precisely to keep a wholesale assignment from
clobbering user-supplied `hf_overrides` (#1413, silently reverted in #2188, re-fixed in
#2904). The algorithm entry points were never updated.

**Steps/Code to reproduce bug**

```yaml
policy:
# no hf_config_overrides
generation:
backend: vllm
vllm_kwargs:
hf_overrides:
max_position_embeddings: 8192
```

The engine is initialized with `hf_overrides={}`, with no warning. Reduced to the line
itself:

```python
policy_config = {} # policy declares no hf_config_overrides
vllm_kwargs = {"hf_overrides": {"max_position_embeddings": 8192}}

vllm_kwargs["hf_overrides"] = policy_config.get("hf_config_overrides", {})

assert vllm_kwargs["hf_overrides"] == {} # the user's setting is gone
```

**Expected behavior**

Merge rather than replace, as `_merge_fp8_kwargs` does. The training policy should still
win on conflicting keys — keeping the generation engine in sync with the trainer is the
whole point of these lines — but keys the policy does not declare should survive, and a
conflict should warn rather than being resolved silently.

**Why generation-only overrides are needed**

When the generation engine's view of a model's HF config differs from the trainer's, the
correction has to be applied on the generation side only; putting it in
`policy.hf_config_overrides` would also (wrongly) change the trainer. We hit this with a
RoPE-scaling setting that a `trust_remote_code` model exposes in a form the installed
vLLM/transformers pair does not pick up: with the override silently dropped, the rollout
engine ran a different positional encoding than the trainer, so generation logprobs and
the trainer's recomputed logprobs disagreed systematically — with nothing in the config
or the logs to indicate it.

This does not weaken the trainer/generation RoPE invariant that
`nemo_rl/models/megatron/setup.py` validates: anything the policy declares still wins, so
a generation-side override only takes effect for keys the policy leaves unset.

Contributor guide

Open the contributing guide

Research direction

Start with the five listed propagation sites in nemo_rl/algorithms/grpo.py, ppo.py, distillation.py, and single_controller_utils/setup.py, then read the existing _merge_fp8_kwargs implementation referenced in the issue. Verify the vllm and dynamo configuration paths preserve generation-side hf_overrides, give policy overrides precedence, and warn on conflicts without changing the trainer-side invariant.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend, machine-learning
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.