Skip to content

Commit 6cca24b

Browse files
fix(grpo): warn when truncated sampling biases the vLLM importance-sampling ratio
GRPO asks vLLM for `processed_logprobs`, which are renormalized over the support that survives top_p/top_k/min_p, while the trainer takes a full-vocab log-softmax. `grpo_trainer.py:2680` differences the two, so the result carries log(S), the log of the surviving probability mass, on top of the train/inference mismatch the correction exists to measure. Measured on one H100 with GRPOTrainer in colocate mode, `sampling/sampling_logp_difference/mean` reads |log(top_p)| exactly: top_p predicted measured 1.0 0.0000 0.0007 0.9 0.1054 0.1054 0.8 0.2231 0.2232 The sequence-level `sampling/importance_sampling_ratio/mean` confirms the same mechanism through a different quantity, reading S^16 for 16-token completions: 0.1855 against 0.185302 at top_p=0.9, and 0.02814 against 0.028147 at top_p=0.8. The correction therefore scales the policy gradient by a factor the user did not ask for, with no error and no log line. This warns at config time instead of changing the math, because a correct fix needs the kept-token set from the sampler and vLLM only returns it from a release outside the current vllm<=0.27.1 pin. The guard is in `GRPOConfig.__post_init__`, so it reaches `GRPOWithReplayBufferConfig` and the gspo_token variant through inheritance. RLOO is unaffected because it has no importance-sampling correction.
1 parent 6d484ba commit 6cca24b

2 files changed

Lines changed: 76 additions & 0 deletions

File tree

tests/test_grpo_trainer.py

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2688,6 +2688,54 @@ def dummy_reward_func(completions, **kwargs):
26882688
expected_warning = "weights each sequence by its completion length"
26892689
assert (expected_warning in caplog.text) == (loss_type != "grpo")
26902690

2691+
@pytest.mark.parametrize(
2692+
("top_p", "top_k", "min_p", "should_warn"),
2693+
[
2694+
(1.0, 0, None, False), # defaults: nothing is truncated, the two distributions agree
2695+
(0.9, 0, None, True),
2696+
(1.0, 50, None, True),
2697+
(1.0, 0, 0.05, True),
2698+
(0.9, 50, 0.05, True),
2699+
],
2700+
)
2701+
def test_warning_raised_truncated_sampling_with_importance_sampling_correction(
2702+
self, top_p, top_k, min_p, should_warn
2703+
):
2704+
"""Truncated sampling biases the vLLM importance-sampling ratio.
2705+
2706+
vLLM returns logprobs renormalized over the surviving support while the trainer takes a full-vocab log-softmax,
2707+
so their difference carries `log S`, the log of the mass that survives truncation, on top of the
2708+
train/inference mismatch the correction is meant to measure. Warn instead of correcting silently.
2709+
"""
2710+
with warnings.catch_warnings(record=True) as caught:
2711+
warnings.simplefilter("always")
2712+
GRPOConfig(
2713+
output_dir=self.tmp_dir,
2714+
use_vllm=True,
2715+
vllm_importance_sampling_correction=True,
2716+
top_p=top_p,
2717+
top_k=top_k,
2718+
min_p=min_p,
2719+
report_to="none",
2720+
)
2721+
2722+
messages = [str(w.message) for w in caught]
2723+
assert any("biases `sampling/sampling_logp_difference`" in m for m in messages) == should_warn
2724+
2725+
def test_no_truncation_warning_without_importance_sampling_correction(self):
2726+
"""The bias only exists when the correction consumes vLLM's logprobs, so truncation alone must stay silent."""
2727+
with warnings.catch_warnings(record=True) as caught:
2728+
warnings.simplefilter("always")
2729+
GRPOConfig(
2730+
output_dir=self.tmp_dir,
2731+
use_vllm=True,
2732+
vllm_importance_sampling_correction=False,
2733+
top_p=0.9,
2734+
report_to="none",
2735+
)
2736+
2737+
assert not any("biases `sampling/sampling_logp_difference`" in str(w.message) for w in caught)
2738+
26912739
def test_train_num_generations_larger_than_batch_size(self):
26922740
dataset = load_dataset("trl-internal-testing/zen", "standard_prompt_only", split="train")
26932741

trl/trainer/grpo_config.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1134,6 +1134,34 @@ def __post_init__(self):
11341134
)
11351135
self.vllm_importance_sampling_clip_max = self.vllm_importance_sampling_cap
11361136

1137+
if self.use_vllm and self.vllm_importance_sampling_correction:
1138+
# vLLM is asked for `processed_logprobs`, which are renormalized over the support that survives
1139+
# top_p/top_k/min_p, while the trainer takes a full-vocab log-softmax. Their difference therefore carries
1140+
# log(S), where S is the surviving mass, on top of the train/inference mismatch the correction exists to
1141+
# measure. Measured on one H100: `sampling/sampling_logp_difference/mean` reads |log(top_p)| exactly, i.e.
1142+
# 0.105 at top_p=0.9 and 0.223 at top_p=0.8 against 0.001 at top_p=1.0.
1143+
truncating = [
1144+
name
1145+
for name, active in (
1146+
("top_p", self.top_p < 1.0),
1147+
("top_k", self.top_k > 0),
1148+
("min_p", self.min_p is not None and self.min_p > 0.0),
1149+
)
1150+
if active
1151+
]
1152+
if truncating:
1153+
warnings.warn(
1154+
f"{' and '.join(truncating)} truncates sampling, which biases "
1155+
"`sampling/sampling_logp_difference` and the importance-sampling ratio derived from it: vLLM "
1156+
"renormalizes its logprobs over the surviving tokens while the trainer normalizes over the full "
1157+
"vocabulary, so their difference includes the log of the surviving probability mass. Set "
1158+
"`vllm_importance_sampling_correction=False`, or keep the sampling defaults (`top_p=1.0`, "
1159+
"`top_k=0`, `min_p=None`), until the correction accounts for truncation. See "
1160+
"https://github.com/huggingface/trl/issues/6789.",
1161+
UserWarning,
1162+
stacklevel=3,
1163+
)
1164+
11371165
if (
11381166
self.vllm_importance_sampling_clip_min is not None
11391167
and self.vllm_importance_sampling_clip_max is not None

0 commit comments

Comments
 (0)