fix(config): apply presence_penalty/frequency_penalty from override-generation-config (#50769)
Signed-off-by: Chenglun Hu <chenglunhu@gmail.com> Signed-off-by: hclsys <chenglunhu@gmail.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
Harry Mellor
parent
26f49e336a
commit
a7d203efe5
@@ -109,3 +109,60 @@ def test_unrecognized_env(monkeypatch):
|
||||
fail_on_environ_validation=True,
|
||||
)
|
||||
engine_args.create_engine_config()
|
||||
|
||||
|
||||
def test_get_diff_sampling_param_includes_penalties():
|
||||
"""``--override-generation-config`` must apply presence_penalty and
|
||||
frequency_penalty as server-side defaults, like the other sampling
|
||||
params. Regression test for the whitelist silently dropping them.
|
||||
|
||||
Called unbound with a lightweight stand-in so no model is loaded:
|
||||
generation_config="vllm" makes the method skip try_get_generation_config
|
||||
and read only override_generation_config.
|
||||
"""
|
||||
from types import SimpleNamespace
|
||||
|
||||
from vllm.config.model import ModelConfig
|
||||
|
||||
fake = SimpleNamespace(
|
||||
generation_config="vllm",
|
||||
override_generation_config={
|
||||
"presence_penalty": 1.5,
|
||||
"frequency_penalty": 0.5,
|
||||
"temperature": 0.6,
|
||||
},
|
||||
)
|
||||
|
||||
diff = ModelConfig.get_diff_sampling_param(fake) # type: ignore[arg-type]
|
||||
|
||||
assert diff["presence_penalty"] == 1.5
|
||||
assert diff["frequency_penalty"] == 0.5
|
||||
assert diff["temperature"] == 0.6
|
||||
|
||||
|
||||
def test_generation_config_penalties_survive_diff_sampling_filter():
|
||||
"""``generation_config.json`` penalties must survive the diff-sampling
|
||||
whitelist (regression for #50767, auto path).
|
||||
|
||||
Covers the ``generation_config="auto"`` branch, where the override
|
||||
dict is empty and the values come from ``try_get_generation_config``.
|
||||
"""
|
||||
from types import SimpleNamespace
|
||||
|
||||
from vllm.config.model import ModelConfig
|
||||
|
||||
fake = SimpleNamespace(
|
||||
generation_config="auto",
|
||||
override_generation_config={},
|
||||
try_get_generation_config=lambda: {
|
||||
"presence_penalty": 1.1,
|
||||
"frequency_penalty": 0.7,
|
||||
"temperature": 0.9,
|
||||
},
|
||||
)
|
||||
|
||||
diff = ModelConfig.get_diff_sampling_param(fake) # type: ignore[arg-type]
|
||||
|
||||
assert diff["presence_penalty"] == 1.1
|
||||
assert diff["frequency_penalty"] == 0.7
|
||||
assert diff["temperature"] == 0.9
|
||||
|
||||
@@ -0,0 +1,65 @@
|
||||
# SPDX-License-Identifier: Apache-2.0
|
||||
# SPDX-FileCopyrightText: Copyright contributors to the vLLM project
|
||||
|
||||
"""Unit tests for presence_penalty / frequency_penalty resolution from
|
||||
default_sampling_params in ChatCompletionRequest and CompletionRequest.
|
||||
|
||||
Regression test for https://github.com/vllm-project/vllm/issues/50767:
|
||||
these two penalties defaulted to 0.0 (not None) and to_sampling_params()
|
||||
forwarded them straight through, so server-side defaults coming from
|
||||
--override-generation-config / generation_config.json were silently
|
||||
discarded on the /v1/chat/completions and /v1/completions endpoints.
|
||||
"""
|
||||
|
||||
from vllm.entrypoints.openai.chat_completion.protocol import (
|
||||
ChatCompletionRequest,
|
||||
)
|
||||
from vllm.entrypoints.openai.completion.protocol import (
|
||||
CompletionRequest,
|
||||
)
|
||||
|
||||
_DEFAULTS = {"presence_penalty": 1.5, "frequency_penalty": 0.5}
|
||||
|
||||
|
||||
def _chat(**kwargs):
|
||||
return ChatCompletionRequest(
|
||||
model="test-model",
|
||||
messages=[{"role": "user", "content": "hi"}],
|
||||
**kwargs,
|
||||
)
|
||||
|
||||
|
||||
class TestChatCompletionPenaltyDefaults:
|
||||
def test_defaults_applied_when_client_omits(self):
|
||||
"""Server-default penalties are applied when the client sends none."""
|
||||
sp = _chat().to_sampling_params(100, _DEFAULTS)
|
||||
assert sp.presence_penalty == 1.5
|
||||
assert sp.frequency_penalty == 0.5
|
||||
|
||||
def test_client_value_overrides_default(self):
|
||||
"""An explicit client penalty wins over the server default."""
|
||||
sp = _chat(presence_penalty=0.2).to_sampling_params(100, _DEFAULTS)
|
||||
assert sp.presence_penalty == 0.2
|
||||
assert sp.frequency_penalty == 0.5
|
||||
|
||||
def test_falls_back_to_zero_without_default(self):
|
||||
"""Without a server default the neutral 0.0 is used (no regression)."""
|
||||
sp = _chat().to_sampling_params(100, {})
|
||||
assert sp.presence_penalty == 0.0
|
||||
assert sp.frequency_penalty == 0.0
|
||||
|
||||
|
||||
class TestCompletionPenaltyDefaults:
|
||||
def test_defaults_applied_when_client_omits(self):
|
||||
sp = CompletionRequest(model="test-model", prompt="hi").to_sampling_params(
|
||||
16, _DEFAULTS
|
||||
)
|
||||
assert sp.presence_penalty == 1.5
|
||||
assert sp.frequency_penalty == 0.5
|
||||
|
||||
def test_client_value_overrides_default(self):
|
||||
sp = CompletionRequest(
|
||||
model="test-model", prompt="hi", frequency_penalty=0.9
|
||||
).to_sampling_params(16, _DEFAULTS)
|
||||
assert sp.presence_penalty == 1.5
|
||||
assert sp.frequency_penalty == 0.9
|
||||
@@ -1822,6 +1822,8 @@ class ModelConfig:
|
||||
|
||||
available_params = [
|
||||
"repetition_penalty",
|
||||
"presence_penalty",
|
||||
"frequency_penalty",
|
||||
"temperature",
|
||||
"top_k",
|
||||
"top_p",
|
||||
|
||||
@@ -215,7 +215,7 @@ class ChatCompletionRequest(OpenAIBaseModel):
|
||||
# https://platform.openai.com/docs/api-reference/chat/create
|
||||
messages: list[ChatCompletionMessageParam]
|
||||
model: str | None = None
|
||||
frequency_penalty: float | None = 0.0
|
||||
frequency_penalty: float | None = None
|
||||
logit_bias: dict[str, float] | None = None
|
||||
logprobs: bool | None = False
|
||||
top_logprobs: int | None = 0
|
||||
@@ -226,7 +226,7 @@ class ChatCompletionRequest(OpenAIBaseModel):
|
||||
)
|
||||
max_completion_tokens: int | None = None
|
||||
n: int | None = 1
|
||||
presence_penalty: float | None = 0.0
|
||||
presence_penalty: float | None = None
|
||||
response_format: AnyResponseFormat | None = None
|
||||
seed: int | None = Field(None, ge=_INT64_MIN, le=_INT64_MAX)
|
||||
stop: StopParam = []
|
||||
@@ -624,6 +624,8 @@ class ChatCompletionRequest(OpenAIBaseModel):
|
||||
# Default sampling parameters for chat completion requests
|
||||
_DEFAULT_SAMPLING_PARAMS: dict = {
|
||||
"repetition_penalty": 1.0,
|
||||
"presence_penalty": 0.0,
|
||||
"frequency_penalty": 0.0,
|
||||
"temperature": 1.0,
|
||||
"top_p": 1.0,
|
||||
"top_k": 0,
|
||||
@@ -661,28 +663,15 @@ class ChatCompletionRequest(OpenAIBaseModel):
|
||||
max_tokens: int,
|
||||
default_sampling_params: dict,
|
||||
) -> SamplingParams:
|
||||
# Default parameters
|
||||
if (repetition_penalty := self.repetition_penalty) is None:
|
||||
repetition_penalty = default_sampling_params.get(
|
||||
"repetition_penalty",
|
||||
self._DEFAULT_SAMPLING_PARAMS["repetition_penalty"],
|
||||
)
|
||||
if (temperature := self.temperature) is None:
|
||||
temperature = default_sampling_params.get(
|
||||
"temperature", self._DEFAULT_SAMPLING_PARAMS["temperature"]
|
||||
)
|
||||
if (top_p := self.top_p) is None:
|
||||
top_p = default_sampling_params.get(
|
||||
"top_p", self._DEFAULT_SAMPLING_PARAMS["top_p"]
|
||||
)
|
||||
if (top_k := self.top_k) is None:
|
||||
top_k = default_sampling_params.get(
|
||||
"top_k", self._DEFAULT_SAMPLING_PARAMS["top_k"]
|
||||
)
|
||||
if (min_p := self.min_p) is None:
|
||||
min_p = default_sampling_params.get(
|
||||
"min_p", self._DEFAULT_SAMPLING_PARAMS["min_p"]
|
||||
# Priority: user -> server default -> OpenAI default
|
||||
sampling_params = {
|
||||
name: (
|
||||
value
|
||||
if (value := getattr(self, name)) is not None
|
||||
else default_sampling_params.get(name, default)
|
||||
)
|
||||
for name, default in self._DEFAULT_SAMPLING_PARAMS.items()
|
||||
}
|
||||
|
||||
# Merge server-default stop_token_ids (e.g., model-specific tokens
|
||||
# like </call> for gpt-oss) with any request-specified ones
|
||||
@@ -709,14 +698,8 @@ class ChatCompletionRequest(OpenAIBaseModel):
|
||||
extra_args["ec_transfer_params"] = self.ec_transfer_params
|
||||
return SamplingParams.from_optional(
|
||||
n=self.n,
|
||||
presence_penalty=self.presence_penalty,
|
||||
frequency_penalty=self.frequency_penalty,
|
||||
repetition_penalty=repetition_penalty,
|
||||
temperature=temperature,
|
||||
**sampling_params,
|
||||
watermarking=self.watermarking,
|
||||
top_p=top_p,
|
||||
top_k=top_k,
|
||||
min_p=min_p,
|
||||
seed=self.seed,
|
||||
stop=self.stop,
|
||||
stop_token_ids=stop_token_ids,
|
||||
|
||||
@@ -55,12 +55,12 @@ class CompletionRequest(OpenAIBaseModel):
|
||||
| None
|
||||
) = None
|
||||
echo: bool | None = False
|
||||
frequency_penalty: float | None = 0.0
|
||||
frequency_penalty: float | None = None
|
||||
logit_bias: dict[str, float] | None = None
|
||||
logprobs: int | None = None
|
||||
max_tokens: int | None = 16
|
||||
n: int = 1
|
||||
presence_penalty: float | None = 0.0
|
||||
presence_penalty: float | None = None
|
||||
seed: int | None = Field(None, ge=_INT64_MIN, le=_INT64_MAX)
|
||||
stop: StopParam = []
|
||||
stream: bool | None = False
|
||||
@@ -278,6 +278,8 @@ class CompletionRequest(OpenAIBaseModel):
|
||||
# Default sampling parameters for completion requests
|
||||
_DEFAULT_SAMPLING_PARAMS: dict = {
|
||||
"repetition_penalty": 1.0,
|
||||
"presence_penalty": 0.0,
|
||||
"frequency_penalty": 0.0,
|
||||
"temperature": 1.0,
|
||||
"top_p": 1.0,
|
||||
"top_k": 0,
|
||||
@@ -321,28 +323,15 @@ class CompletionRequest(OpenAIBaseModel):
|
||||
if default_sampling_params is None:
|
||||
default_sampling_params = {}
|
||||
|
||||
# Default parameters
|
||||
if (repetition_penalty := self.repetition_penalty) is None:
|
||||
repetition_penalty = default_sampling_params.get(
|
||||
"repetition_penalty",
|
||||
self._DEFAULT_SAMPLING_PARAMS["repetition_penalty"],
|
||||
)
|
||||
if (temperature := self.temperature) is None:
|
||||
temperature = default_sampling_params.get(
|
||||
"temperature", self._DEFAULT_SAMPLING_PARAMS["temperature"]
|
||||
)
|
||||
if (top_p := self.top_p) is None:
|
||||
top_p = default_sampling_params.get(
|
||||
"top_p", self._DEFAULT_SAMPLING_PARAMS["top_p"]
|
||||
)
|
||||
if (top_k := self.top_k) is None:
|
||||
top_k = default_sampling_params.get(
|
||||
"top_k", self._DEFAULT_SAMPLING_PARAMS["top_k"]
|
||||
)
|
||||
if (min_p := self.min_p) is None:
|
||||
min_p = default_sampling_params.get(
|
||||
"min_p", self._DEFAULT_SAMPLING_PARAMS["min_p"]
|
||||
# Priority: user -> server default -> OpenAI default
|
||||
sampling_params = {
|
||||
name: (
|
||||
value
|
||||
if (value := getattr(self, name)) is not None
|
||||
else default_sampling_params.get(name, default)
|
||||
)
|
||||
for name, default in self._DEFAULT_SAMPLING_PARAMS.items()
|
||||
}
|
||||
|
||||
# Merge server-default stop_token_ids (e.g., model-specific tokens
|
||||
# like </call> for gpt-oss) with any request-specified ones
|
||||
@@ -371,14 +360,8 @@ class CompletionRequest(OpenAIBaseModel):
|
||||
extra_args["ec_transfer_params"] = self.ec_transfer_params
|
||||
return SamplingParams.from_optional(
|
||||
n=self.n,
|
||||
presence_penalty=self.presence_penalty,
|
||||
frequency_penalty=self.frequency_penalty,
|
||||
repetition_penalty=repetition_penalty,
|
||||
temperature=temperature,
|
||||
**sampling_params,
|
||||
watermarking=self.watermarking,
|
||||
top_p=top_p,
|
||||
top_k=top_k,
|
||||
min_p=min_p,
|
||||
seed=self.seed,
|
||||
stop=self.stop,
|
||||
stop_token_ids=stop_token_ids,
|
||||
|
||||
Reference in New Issue
Block a user