Skip to content

[CI Test] Mark batch invariance test flaky - #46530

Merged
mgoin merged 2 commits into
mainfrom
wentao-set-bs-test-flacky
Jun 23, 2026
Merged

mgoin merged 2 commits into
mainfrom
wentao-set-bs-test-flacky

Conversation

@yewentao256

Copy link
Copy Markdown
Member

Purpose

Thanks for the context from @mgoin , this batch invariance test seems to be flaky in certain node.

https://buildkite.com/vllm/ci/builds/73785#019ef524-b3b6-442c-9e0c-cb4ee887c1e5/L3930, rerun passes.

Locally test passes as well pytest tests/v1/determinism/test_batch_invariance.py::test_v1_generation_is_deterministic_across_batch_sizes_with_needle[FLEX_ATTENTION]

============================ 1 passed, 19 warnings in 46.96s =============================

Maybe worth a deep dive, but let's fix CI first

Signed-off-by: yewentao256 <zhyanwentao@126.com>
@mergify mergify Bot added the v1 label Jun 23, 2026
@mgoin mgoin added ready ONLY add when PR is ready to merge/full CI is needed ci-failure Issue about an unexpected test failure in CI labels Jun 23, 2026
Comment thread tests/v1/determinism/test_batch_invariance.py Outdated
Signed-off-by: Wentao Ye <44945378+yewentao256@users.noreply.github.com>

@mgoin mgoin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks! Let's clear up CI for now

@mgoin
mgoin enabled auto-merge (squash) June 23, 2026 19:25
@mgoin
mgoin merged commit abc3313 into main Jun 23, 2026
25 checks passed
@mgoin
mgoin deleted the wentao-set-bs-test-flacky branch June 23, 2026 21:01
nkzhenhua pushed a commit to nkzhenhua/vllm that referenced this pull request Jun 24, 2026
Signed-off-by: yewentao256 <zhyanwentao@126.com>
Signed-off-by: Wentao Ye <44945378+yewentao256@users.noreply.github.com>
wincent8 pushed a commit to wincent8/vllm that referenced this pull request Jun 29, 2026
Signed-off-by: yewentao256 <zhyanwentao@126.com>
Signed-off-by: Wentao Ye <44945378+yewentao256@users.noreply.github.com>
efschu pushed a commit to efschu/shvllm that referenced this pull request Jul 18, 2026
Signed-off-by: yewentao256 <zhyanwentao@126.com>
Signed-off-by: Wentao Ye <44945378+yewentao256@users.noreply.github.com>
@GuoCheng24

Copy link
Copy Markdown

@yewentao256 you wrote "maybe worth a deep dive, but let's fix CI first". Here is the deep
dive, including the part of it that argues against me.

What the marker costs, from the test's own parameters

The test runs VLLM_NEEDLE_TRIALS (default 5) trials and fails if any mismatches;
@pytest.mark.flaky(reruns=3), backed by pytest-rerunfailures==14.0 pinned in
requirements/test/cuda.txt, retries a failing test up to three more times and the suite
reports success if any attempt is clean. With q = P(one trial exposes a violation):

detect(one attempt) = 1 - (1-q)**5
reported(suite)     = detect ** 4
per-trial q one attempt detects suite reports, with reruns without
5% 22.6% 0.26% 22.6%
10% 41.0% 2.8% 41.0%
20% 67.2% 20.4% 67.2%
50% 96.9% 88.1% 96.9%
90% 100% 100% 100%

To be reported half the time, a violation has to appear in 30.8% of trials instead of 12.9%.
The marker turns the gate from any trial mismatches into every attempt has a mismatching
trial
.

Measurements, including the one that cuts against the argument

Needle protocol reproduced faithfully — same prompt, temperature=0.0, top_p=0.95,
max_tokens=128, seed=20240919, batch size random in [N/2, N], the same
_random_prompt filler, enable_prefix_caching=False, VLLM_BATCH_INVARIANT forced as
conftest.py does. One RTX 4090 (SM89), vllm==0.28.0, FLEX_ATTENTION.

VLLM_BATCH_INVARIANT model rms batch trials mismatched q̂ (95% CI)
1 Qwen3-1.7B default ≤128 60 0 0.00 [0.00, 0.06]
1 Qwen3-1.7B vllm_c ≤128 60 0 0.00 [0.00, 0.06]
0 Qwen2.5-0.5B default ≤64 40 40 1.00 [0.91, 1.00]
0 Qwen2.5-0.5B default ≤16 30 30 1.00 [0.89, 1.00]
0 Qwen2.5-0.5B default ≤8 30 30 1.00 [0.89, 1.00]
0 Qwen2.5-0.5B default ≤4 30 30 1.00 [0.89, 1.00]

The bottom four rows are the counterweight to the table above. Every violation I could
actually produce is systematic: turning batch invariance off puts q at 1.0 and keeps it there
down to a batch of 4. At q = 1 the marker costs exactly nothing — it reruns, the mismatch
reappears, the suite goes red. I tried to construct an intermediate case and could not.

So the masking is not a general cost. It is specific to intermittent violations, and the
reason I still think this one is worth changing is that the failure which prompted the marker
was intermittent by construction. You wrote it was "flaky in certain node" and that "rerun
passes". A violation with q = 1 cannot pass on rerun. Whatever that CI failure was, it sat at
q < 1 — the regime the first table says is hidden. The marker was added for the one kind of
failure it conceals best.

I could not reproduce that node's behaviour: on this 4090 the intact arms are clean to within
6%, consistent with your "certain node", and I cannot tell you whether the original failure
was a real invariance violation or infrastructure noise. That is the open question, and it is
the one the marker now prevents CI from answering.

Why it is worth resolving either way

Since this merged, 19 batch-invariance PRs have landed, including kernel bugfixes whose
verification rests on this gate — #48391 RMSNorm block pinning, #52960 fp32 matmul OOR on
SM89, #45683 deterministic MoE combine — and a growing list of models documented as
batch-invariant (#55691 OLMo 2, #53839 EXAONE-4.0, #53650 Granite 3.1, #46396 Phi-3.5-mini).
Each says "this is batch-invariant" on the strength of a gate that currently passes if one
attempt in four is clean.

Two options that keep CI green without that

  1. Raise trials instead of retrying. VLLM_NEEDLE_TRIALS is already an env var; more
    trials in one attempt raise detection monotonically where reruns lower it. If the goal is
    tolerating node noise, that is the knob that does not trade away sensitivity.
  2. Quarantine rather than rerun. xfail(strict=False) on the affected configuration with
    a tracking issue, so a failure stays visible and attributed instead of being retried away.

Happy to open the PR for whichever you prefer, or to drop this if you would rather keep the
current behaviour — you have the context on that node that I do not. The arithmetic is a
short script over the test's own parameters, no GPU and no vLLM import, and the reproduction
harness is a single file, if anyone wants to check or contradict either.

@GuoCheng24

Copy link
Copy Markdown

Following up on my own comment, because I read #46531 afterwards and it changes what I should
have said. I had written that the marker "prevents CI from answering" whether the original
failure was real, as though nobody had looked. Someone did: @drisspg picked it up, tried to
reproduce on both Blackwell and Hopper, and found everything deterministic. That is real
evidence gathered outside CI and I should have found it before posting.

What is left after that is narrower than my first comment, and it is about how #46531 was
closed rather than about the marker hiding anything.

The issue was closed on two things. One is @drisspg's non-reproduction — on Blackwell and
Hopper, while the report was that it was "flaky in certain node", so different hardware from
the one that failed. The other is "I didn't see it failing recently on ci". That second one is
circular: the marker was added precisely so that this test would stop failing CI, and by the
arithmetic in my earlier comment it changes the chance of a violation appearing in 10% of
trials being reported from 41% to 2.8%. Whether or not anything is actually wrong, CI silence
after the marker cannot carry the weight of "perhaps be fixed" — the marker is what produced
the silence.

The suspected fix does not close the gap either. The hunch points at #46564, which is
[Model] Support Unlimited OCR, and the sentence naming the change ends at "with this change:"
with nothing after it. So the root cause was never identified.

The argument against me, which I want on the record: I could not construct a violation with
0 < q < 1. Every violation I could produce is systematic — VLLM_BATCH_INVARIANT=0 gives
q = 1.00 [0.91, 1.00] and stays there down to a batch of 4 — and at q = 1 the marker costs
nothing, because the mismatch simply reappears on every rerun. If real violations are always
systematic then the marker is untidy history and nothing more. I have no measurement showing
the intermediate regime exists; I only have the fact that the failure which prompted the marker
passed on rerun, which no q = 1 violation does.

So I am not claiming something is broken. The concrete suggestion is smaller: since the cause
was never found, drop the marker now and let CI test the "perhaps be fixed" hypothesis for
real. If it stays green at full power across the next few hundred runs, that is evidence worth
having and the question closes properly. If it goes red, that is the reproduction @drisspg
could not get on his own hardware. Either outcome is better than the current state, where the
test cannot produce evidence either way and the issue is closed on the absence of a signal the
marker suppresses. The test file has been edited three times since June — #48391, and twice for
XPU kernels — and the marker has outlived the failure it was added for.

Happy to open that one-line PR if you want it, or to leave it if you would rather keep the
belt-and-braces on a test that takes a GPU minute to run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-failure Issue about an unexpected test failure in CI ready ONLY add when PR is ready to merge/full CI is needed v1

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants