Skip to content

Commit fabf48f

Browse files
mudlerclaude
andcommitted
test(engine): lock SamplingParams PostInit (__post_init__) contract; document mandatory call
Document that PostInit() is the __post_init__ equivalent and MUST be called on every SamplingParams entering the engine (Verify() alone neither normalizes fields nor runs the greedy n-check, so it accepts states upstream rejects). Cross-reference PostInit() from Verify(). Add a "PostInit contract" test case locking the split: Verify()-only accepts greedy+n>1 while PostInit() rejects it; greedy sub-param forcing at temp 0; the clamp-precedes-greedy ordering (1e-6 -> _MAX_TEMP, stays random); temperature clamp in (0, _MAX_TEMP); and seed==-1 unset. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
1 parent b888645 commit fabf48f

2 files changed

Lines changed: 72 additions & 4 deletions

File tree

include/vllm/sampling_params.h

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -114,13 +114,22 @@ struct SamplingParams {
114114
// random_seed when a seed is set, else random.
115115
SamplingType Type() const;
116116

117-
// _verify_args: pure validation. Throws std::runtime_error with the
117+
// _verify_args: pure validation only. Throws std::runtime_error with the
118118
// upstream-equivalent message on any invalid field. const (no mutation).
119+
// NOTE: this is NOT sufficient on its own — it does not normalize fields nor
120+
// run the greedy n-check, so it accepts states upstream rejects at
121+
// construction (e.g. temperature=0 with n=2). Callers that build a
122+
// SamplingParams for the engine must call PostInit(), not Verify() alone.
119123
void Verify() const;
120124

121-
// __post_init__: normalize in place (clamp near-zero temperature, drop
122-
// seed == -1, force greedy sub-params when greedy), then Verify() and the
123-
// greedy n-check. Mirrors upstream construction-time behavior.
125+
// __post_init__ equivalent: normalize in place (clamp near-zero temperature,
126+
// drop seed == -1, force greedy sub-params when greedy), then run Verify()
127+
// and the greedy n-check. Upstream ALWAYS runs __post_init__ at construction,
128+
// so this is MANDATORY: every SamplingParams that enters the engine must have
129+
// PostInit() called on it (the InputProcessor / EngineCoreRequest
130+
// construction path in M1.8 does this). Verify() alone is NOT a substitute —
131+
// it neither normalizes fields nor enforces the greedy n==1 rule, so a caller
132+
// using Verify() by itself would accept invalid states upstream rejects.
124133
void PostInit();
125134

126135
private:

tests/vllm/test_sampling_params.cpp

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,3 +244,62 @@ TEST_CASE("PostInit normalizes like __post_init__") {
244244
CHECK_THROWS_AS(p.PostInit(), std::runtime_error);
245245
}
246246
}
247+
248+
// Locks the Verify()/PostInit() split so a future refactor cannot silently
249+
// drop the mandatory __post_init__ normalization + greedy n-check. Upstream
250+
// always runs __post_init__ at construction; Verify() alone is NOT a
251+
// substitute (it neither normalizes fields nor enforces the greedy n==1 rule).
252+
TEST_CASE("PostInit contract: mandatory __post_init__ equivalent") {
253+
SUBCASE("Verify() alone accepts a state upstream rejects (greedy, n>1)") {
254+
// temperature=0 (greedy) with n=2 is invalid upstream, but Verify() does
255+
// not run the greedy n-check, so it must NOT throw here.
256+
SamplingParams p;
257+
p.temperature = 0.0;
258+
p.n = 2;
259+
CHECK_NOTHROW(p.Verify());
260+
// The SAME object, run through PostInit(), IS rejected — proving Verify()
261+
// by itself is insufficient and PostInit() is the enforcing path.
262+
CHECK_THROWS_AS(p.PostInit(), std::runtime_error);
263+
}
264+
SUBCASE("PostInit() forces greedy sub-params at temperature 0") {
265+
SamplingParams p;
266+
p.temperature = 0.0;
267+
p.top_p = 0.5;
268+
p.top_k = 20;
269+
p.min_p = 0.3;
270+
p.PostInit();
271+
CHECK(p.top_p == doctest::Approx(1.0));
272+
CHECK(p.top_k == 0);
273+
CHECK(p.min_p == doctest::Approx(0.0));
274+
CHECK(p.Type() == SamplingType::kGreedy);
275+
}
276+
SUBCASE("PostInit() clamp precedes greedy check: 1e-6 is NOT greedy") {
277+
// Locks the ORDER in __post_init__: the (0, _MAX_TEMP) clamp raises a
278+
// near-zero positive temperature to _MAX_TEMP (1e-2) BEFORE the
279+
// < _SAMPLING_EPS (1e-5) greedy test runs. So temperature=1e-6 ends up
280+
// clamped and random, and greedy sub-params are NOT forced.
281+
SamplingParams p;
282+
p.temperature = 1e-6;
283+
p.top_p = 0.5;
284+
p.top_k = 20;
285+
p.min_p = 0.3;
286+
p.PostInit();
287+
CHECK(p.temperature == doctest::Approx(vllm::kMaxTemp));
288+
CHECK(p.top_p == doctest::Approx(0.5));
289+
CHECK(p.top_k == 20);
290+
CHECK(p.min_p == doctest::Approx(0.3));
291+
CHECK(p.Type() == SamplingType::kRandom);
292+
}
293+
SUBCASE("PostInit() clamps temperature in (0, _MAX_TEMP) up to _MAX_TEMP") {
294+
SamplingParams p;
295+
p.temperature = 0.005; // in (0, 1e-2)
296+
p.PostInit();
297+
CHECK(p.temperature == doctest::Approx(vllm::kMaxTemp));
298+
}
299+
SUBCASE("PostInit() drops seed == -1") {
300+
SamplingParams p;
301+
p.seed = -1;
302+
p.PostInit();
303+
CHECK_FALSE(p.seed.has_value());
304+
}
305+
}

0 commit comments

Comments
 (0)