[CI] Add option to raise an exception when NaNs are detected in logits - #50323
Conversation
Assisted-by: OpenAI Codex Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>
|
@claude review |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — CI-only change wiring existing NaN-logits detection infra into the eval test suite, no production code touched.
Extended reasoning...
Overview
This PR touches only test/CI infrastructure: buildkite scheduled-integration scripts and tests/evals/* correctness tests. It adds a new tests/evals/metrics.py helper (assert_no_nan_logits) that scrapes the /metrics endpoint for the vllm:corrupted_requests_total counter and asserts it stays at zero, then wires VLLM_COMPUTE_NANS_IN_LOGITS=1 into the various eval harnesses (GSM8K, GPQA, MRCR, lm-eval) so that NaN-logit corruption fails the eval instead of silently producing garbage accuracy numbers.
Security risks
None. No production code paths (attention kernels, sampler, scheduler, etc.) are modified — VLLM_COMPUTE_NANS_IN_LOGITS and the vllm:corrupted_requests counter already exist in vllm/v1/metrics/loggers.py, vllm/v1/worker/gpu_model_runner.py, and vllm/envs.py; this PR only turns the flag on in test harnesses and asserts on the pre-existing metric.
Level of scrutiny
Low-to-moderate. This is CI/test-only tooling with no impact on the served product; the new helper has direct unit test coverage (test_assert_no_nan_logits parametrized over missing-metric, zero, and multi-engine-nonzero cases) that exercises the parsing/aggregation logic exactly as it will be used against a real /metrics payload.
Other factors
I confirmed the metric name matches: the underlying Counter is a prometheus_client.Counter, which Prometheus auto-suffixes with _total on scrape, matching what assert_no_nan_logits queries for. I also confirmed --host/--port construction in gsm8k_eval.py's new --check-nan-logits path is consistent with how the buildkite scripts invoke it (--host http://127.0.0.1). The change is mechanical and consistently applied across all five eval call sites.
Assisted-by: OpenAI Codex Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>
8cca64f to
bc19528
Compare
| trap cleanup EXIT | ||
|
|
||
| for BACK in "${BACKENDS[@]}"; do | ||
| VLLM_COMPUTE_NANS_IN_LOGITS=1 \ |
There was a problem hiding this comment.
should we let each hw backend determine the value of that? tbh if this goes through i would ask if you could set it in run-am-tests.sh so that it is enabled by default on amd for everything. or you think that it should be kept in test level?
|
Thanks @tlrmchlsmth this looks great! I wonder though, rather than relying on scraping the metrics, we could maybe have a |
|
^ +1 to Nick's suggestion! I'd like this to be global on CI and raise so the pytest/script fails |
|
Going to add |
Replace per-eval metrics scraping with a runtime flag that raises as soon as NaN logits reach output bookkeeping. The flag implies existing NaN computation and covers both GPU model runners. Co-authored-by: OpenAI Codex <codex@openai.com> Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>
Co-authored-by: OpenAI Codex <codex@openai.com> Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>
Co-authored-by: OpenAI Codex <codex@openai.com> Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>
Inject NaN logits through the GPU model runner and assert that the runtime flag raises for the affected request. Co-authored-by: OpenAI Codex <codex@openai.com> Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>
Replace the direct detector test with inference tests that force model runner v1 and v2, inject a NaN from model logits computation, and assert generation fails. Co-authored-by: OpenAI Codex <codex@openai.com> Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>
Only build the corrupted-request mapping after detecting a nonzero NaN count. Co-authored-by: OpenAI Codex <codex@openai.com> Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>
|
partner PR is up here vllm-project/ci-infra#444 |
github-project-automation
Bot
moved this from To Triage
to Ready
in gpt-oss Issues & Enhancements
Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>
|
Switching the envs to 1 to see if anything breaks -- so nobody merge it yet ;) |
![depthfirst-app[bot]](https://avatars.githubusercontent.com/in/1021002?s=60&v=4){kind=small}
Avoid retaining the v2 model through a bound method in the injected-NaN test, and restore opt-in defaults for logit NaN detection. Co-authored-by: OpenAI Codex <codex@openai.com> Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>
tlrmchlsmth
changed the title
github-project-automation
Bot
moved this from Ready
to Done
in gpt-oss Issues & Enhancements
…nflicts Dropped the legacy TYPE_CHECKING block and environment_variables dict wholesale, then ported main's delta across 10 main-side commits: Additions: VLLM_ROCM_USE_AITER_MOE_SITUV2_A8W4 (vllm-project#50582), VLLM_USE_RUST_BENCH (vllm-project#50081), VLLM_KIMI_K3_SHARD_SP_SHARED_EXPERT (vllm-project#50656), VLLM_RAISE_ON_LOGIT_NANS (vllm-project#50323), VLLM_ENABLE_COHERE_API (vllm-project#47189). Modifications: VLLM_COMPUTE_NANS_IN_LOGITS is now implied by VLLM_RAISE_ON_LOGIT_NANS (cross-field, so a model_validator); _resolve_rust_frontend_path -> _resolve_rust_cli_path, resolving on either VLLM_USE_RUST_FRONTEND or VLLM_USE_RUST_BENCH. Deletions: VLLM_CPU_SGL_KERNEL (vllm-project#50801), Q_/K_/V_SCALE_CONSTANT (vllm-project#49389 -- main deleted the dict entries but left the TYPE_CHECKING annotations; followed the PR's intent). tests: ported VLLMValidationError assertions; adapted test_rust_bench_auto_path_missing_fails_fast to construct ServerSettings directly. Dropped the Q_SCALE_CONSTANT case from test_envs_pydantic.py. AI assistance (Claude) was used for this merge resolution. Co-authored-by: Claude Signed-off-by: Vinay Damodaran <vrdn@hey.com>
5 participants
This PR sets
VLLM_COMPUTE_NANS_IN_LOGITS=1in several evals so we can test against NaNs.NaNs appearing in logits often co-appear with KV cache NaNs, which is a catastrophic failure mode for a inference service since attention kernels mask with multiplication by zero. (see Dao-AILab/flash-attention#1974)
Assisted-by: OpenAI Codex