From a7d203efe549de57d8289ad15dc4fdeccf1179be Mon Sep 17 00:00:00 2001 From: hcl Date: Thu, 24 Sep 2026 23:48:18 +0800 Subject: [PATCH] fix(config): apply presence_penalty/frequency_penalty from override-generation-config (#50769) Signed-off-by: Chenglun Hu Signed-off-by: hclsys Co-authored-by: Claude Opus 4.8 Co-authored-by: Harry Mellor <19981378+hmellor@users.noreply.github.com> --- tests/config/test_config_generation.py | 57 ++++++++++++++++ .../openai/test_penalty_default_resolution.py | 65 +++++++++++++++++++ vllm/config/model.py | 2 + .../openai/chat_completion/protocol.py | 43 ++++-------- .../entrypoints/openai/completion/protocol.py | 43 ++++-------- 5 files changed, 150 insertions(+), 60 deletions(-) create mode 100644 tests/entrypoints/openai/test_penalty_default_resolution.py diff --git a/tests/config/test_config_generation.py b/tests/config/test_config_generation.py index c7edf2b971..a6908f35e5 100644 --- a/tests/config/test_config_generation.py +++ b/tests/config/test_config_generation.py @@ -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 diff --git a/tests/entrypoints/openai/test_penalty_default_resolution.py b/tests/entrypoints/openai/test_penalty_default_resolution.py new file mode 100644 index 0000000000..2c39f5bef6 --- /dev/null +++ b/tests/entrypoints/openai/test_penalty_default_resolution.py @@ -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 diff --git a/vllm/config/model.py b/vllm/config/model.py index 749916752e..82b5efc8e4 100644 --- a/vllm/config/model.py +++ b/vllm/config/model.py @@ -1822,6 +1822,8 @@ class ModelConfig: available_params = [ "repetition_penalty", + "presence_penalty", + "frequency_penalty", "temperature", "top_k", "top_p", diff --git a/vllm/entrypoints/openai/chat_completion/protocol.py b/vllm/entrypoints/openai/chat_completion/protocol.py index 225e992441..5c1bc594d7 100644 --- a/vllm/entrypoints/openai/chat_completion/protocol.py +++ b/vllm/entrypoints/openai/chat_completion/protocol.py @@ -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 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, diff --git a/vllm/entrypoints/openai/completion/protocol.py b/vllm/entrypoints/openai/completion/protocol.py index bf0a94eea7..a0974cb91c 100644 --- a/vllm/entrypoints/openai/completion/protocol.py +++ b/vllm/entrypoints/openai/completion/protocol.py @@ -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 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,