Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions tensorrt_llm/sampling_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,12 @@ def __call__(
pass # noqa


# Smallest non-zero temperature forwarded to the sampling backend. Positive
# values below this overflow logits / temperature to inf/nan in fp16/bf16, so
# they are clamped up to this floor. temperature == 0 (greedy) is left as-is.
MIN_SAMPLING_TEMPERATURE = 1e-2


@dataclass(slots=True, kw_only=True)
class SamplingParams:
"""Sampling parameters for text generation.
Expand Down Expand Up @@ -320,6 +326,10 @@ def _validate(self):
raise ValueError(f"require top_k >= 0, got top_k={self.top_k}")
if self.temperature is not None and self.temperature < 0:
raise ValueError(f"require temperature >= 0, got temperature={self.temperature}")
# Clamp very small non-zero temperatures up to a numerically safe floor;
# keep 0.0 as-is so greedy decoding is unaffected.
if self.temperature is not None and 0 < self.temperature < MIN_SAMPLING_TEMPERATURE:
self.temperature = MIN_SAMPLING_TEMPERATURE
Comment on lines 330 to +335

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Reject non-finite temperatures before applying the floor.

float("nan") makes both comparisons false, so it is forwarded to the backend; positive infinity also passes through. Validate finiteness and add NaN/Infinity tests before the sign/clamp checks.

Proposed fix
+import math
+
...
+        if self.temperature is not None and not math.isfinite(self.temperature):
+            raise ValueError(f"temperature must be finite, got temperature={self.temperature}")
         if self.temperature is not None and self.temperature < 0:
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if self.temperature is not None and self.temperature < 0:
raise ValueError(f"require temperature >= 0, got temperature={self.temperature}")
# Clamp very small non-zero temperatures up to a numerically safe floor;
# keep 0.0 as-is so greedy decoding is unaffected.
if self.temperature is not None and 0 < self.temperature < MIN_SAMPLING_TEMPERATURE:
self.temperature = MIN_SAMPLING_TEMPERATURE
if self.temperature is not None and not math.isfinite(self.temperature):
raise ValueError(f"temperature must be finite, got temperature={self.temperature}")
if self.temperature is not None and self.temperature < 0:
raise ValueError(f"require temperature >= 0, got temperature={self.temperature}")
# Clamp very small non-zero temperatures up to a numerically safe floor;
# keep 0.0 as-is so greedy decoding is unaffected.
if self.temperature is not None and 0 < self.temperature < MIN_SAMPLING_TEMPERATURE:
self.temperature = MIN_SAMPLING_TEMPERATURE
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tensorrt_llm/sampling_params.py` around lines 327 - 332, Validate that
temperature is finite in the SamplingParams temperature validation logic before
the negative-value and minimum-floor checks. Reject NaN and positive or negative
infinity with a ValueError, and add tests covering these non-finite values
alongside the existing sign and clamping tests.


if self.best_of is not None and self.best_of < self.n:
raise ValueError(f"best_of ({self.best_of}) cannot be less than n ({self.n})")
Expand Down
41 changes: 41 additions & 0 deletions tests/unittest/llmapi/test_sampling_params.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
# SPDX-License-Identifier: Apache-2.0
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import pytest

from tensorrt_llm.sampling_params import MIN_SAMPLING_TEMPERATURE, SamplingParams


def test_temperature_none_unchanged():
assert SamplingParams().temperature is None


def test_temperature_zero_kept_for_greedy():
# 0.0 means greedy decoding and must not be clamped.
assert SamplingParams(temperature=0.0).temperature == 0.0


@pytest.mark.parametrize("tiny", [1e-12, 1e-6, MIN_SAMPLING_TEMPERATURE / 2])
def test_tiny_positive_temperature_clamped(tiny):
assert SamplingParams(temperature=tiny).temperature == MIN_SAMPLING_TEMPERATURE

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert the contractual floor independently of the implementation constant.

Using MIN_SAMPLING_TEMPERATURE as the expected value means an incorrect constant could still pass the tests. The requirement is specifically 1e-2.

Proposed fix
-    assert SamplingParams(temperature=tiny).temperature == MIN_SAMPLING_TEMPERATURE
+    assert SamplingParams(temperature=tiny).temperature == 1e-2
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@pytest.mark.parametrize("tiny", [1e-12, 1e-6, MIN_SAMPLING_TEMPERATURE / 2])
def test_tiny_positive_temperature_clamped(tiny):
assert SamplingParams(temperature=tiny).temperature == MIN_SAMPLING_TEMPERATURE
`@pytest.mark.parametrize`("tiny", [1e-12, 1e-6, MIN_SAMPLING_TEMPERATURE / 2])
def test_tiny_positive_temperature_clamped(tiny):
assert SamplingParams(temperature=tiny).temperature == 1e-2
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unittest/llmapi/test_sampling_params.py` around lines 29 - 31, Update
test_tiny_positive_temperature_clamped to assert the contractual floor directly
as 1e-2 instead of using MIN_SAMPLING_TEMPERATURE, while retaining the existing
tiny positive temperature parameterization.



@pytest.mark.parametrize("temp", [MIN_SAMPLING_TEMPERATURE, 0.5, 1.0, 2.0])
def test_normal_temperature_unchanged(temp):
assert SamplingParams(temperature=temp).temperature == temp


def test_negative_temperature_rejected():
with pytest.raises(ValueError, match="temperature"):
SamplingParams(temperature=-1.0)