Skip to content

Commit b01cb71

Browse files
mudlerclaude
andcommitted
test(sampler): harden the mixed where-merge case against predicate inversion
Post-review: the mixed greedy+random test used a peaked greedy row, so an inverted merge predicate (random where temp<eps) would still pass — a peaked row's greedy-argmax equals the mode the random path samples from. Reconstruct row 0 as UNIFORM over 8 tokens (greedy argmax=0 by tie-break) and cross-check against an all-random reference proving its random draw != 0, so merge-correct => row0==0 while merge-inverted => the random value (!=0) and the test bites. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 38a8846 commit b01cb71

1 file changed

Lines changed: 31 additions & 7 deletions

File tree

tests/vllm/v1/sample/test_sampler.cpp

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -65,27 +65,51 @@ TEST_CASE("Sampler: all-greedy batch returns the argmax per row, no logprobs") {
6565

6666
// ---------------------------------------------------------------------------
6767
// Mixed greedy + random: the temp<eps where-merge picks greedy for the greedy
68-
// row and random for the random row. Row 1 is a peaked distribution so the
69-
// random draw is deterministic for the assertion.
68+
// row and random for the random row.
69+
//
70+
// This case is DELIBERATELY constructed so an INVERTED merge predicate (picking
71+
// random where temp<eps) fails deterministically — a peaked greedy row cannot do
72+
// that, because greedy-argmax always equals the mode the random path samples
73+
// from, so on a peaked row both branches coincide. Instead row 0 is UNIFORM over
74+
// 8 tokens: its greedy argmax is index 0 (lowest-index tie-break), but its random
75+
// draw under the fixed seed lands elsewhere (asserted below via an all-random
76+
// reference). So merge-correct => row 0 == 0; merge-inverted => row 0 == the
77+
// random draw (!= 0) => the test bites.
7078
TEST_CASE("Sampler: mixed batch merges greedy (temp<eps) and random per row") {
71-
// Row 0 (greedy, temp 0): argmax at index 3.
79+
const int64_t V = 8;
80+
// Row 0 (greedy, temp 0): uniform -> argmax 0 by tie-break; random draw != 0.
7281
// Row 1 (random, temp 1): logit 100 at index 2 -> softmax ~= one-hot(2).
73-
std::vector<float> logits = {0.0f, 1.0f, 2.0f, 3.0f,
74-
0.0f, 0.0f, 100.0f, 0.0f};
75-
Tensor tl = Logits(logits, 2, 4);
82+
std::vector<float> logits(2 * V, 0.0f);
83+
logits[V + 2] = 100.0f;
7684
SamplingMetadata sm;
7785
sm.all_greedy = false;
7886
sm.all_random = false;
7987
sm.temperature = std::vector<float>{0.0f, 1.0f};
88+
sm.generators[0] = 424242; // fixed seed for the (greedy) row 0's random path
8089
sm.generators[1] = 20260704; // per-request seed for the random row
8190
sm.max_num_logprobs = std::nullopt;
8291

8392
Sampler sampler;
8493
Queue q = Q();
94+
Tensor tl = Logits(logits, 2, V);
8595
auto out = sampler.forward(q, tl, sm);
8696

87-
CHECK(out.sampled_token_ids[0][0] == 3); // greedy row -> argmax
97+
CHECK(out.sampled_token_ids[0][0] == 0); // greedy row -> argmax (tie-break)
8898
CHECK(out.sampled_token_ids[1][0] == 2); // random row -> the dominant token
99+
100+
// Reference: what the RANDOM path alone produces for row 0 under the same seed.
101+
// It must differ from the greedy argmax (0), proving the merge above genuinely
102+
// selected greedy — an inverted predicate would have emitted this value.
103+
std::vector<float> row0(logits.begin(), logits.begin() + V);
104+
SamplingMetadata rnd;
105+
rnd.all_greedy = false;
106+
rnd.all_random = true;
107+
rnd.temperature = std::vector<float>{1.0f};
108+
rnd.generators[0] = 424242;
109+
rnd.max_num_logprobs = std::nullopt;
110+
Tensor trow0 = Logits(row0, 1, V);
111+
auto ref = sampler.forward(q, trow0, rnd);
112+
CHECK(ref.sampled_token_ids[0][0] != 0); // random draw diverges from greedy
89113
}
90114

91115
// ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)