Repository navigation
test(cuda): fix too-tight tolerance in EagerVsGraphDecode_QuantizedKv_Match - #175
Merged
Merged
Conversation
…_Match (#174) This test asserted maxDiff < 1e-3f on first-step logits with a comment claiming eager and graph paths are bit-identical. Its sibling EagerVsGraphDecode_Match already documents and tolerates (< 5.0f) the real cause: PTX-JIT cache state left behind by other CUDA tests in the same process shifts SASS scheduling, which shifts FP accumulation order in identical arithmetic, perturbing logits by up to ~0.25 max-abs in the full suite (argmax unaffected). Confirmed by direct repro: standalone diff 0.000000, full DotLLM.Tests.Unit.Cuda suite diff 0.201172 — comfortably inside the sibling test's own tolerance, failing only this test's much tighter bound. Not a real eager-vs-graph bug; this is the pre-existing test-isolation flake documented across prior sessions (always passed standalone, only failed in the full suite). Loosened to 5.0f (matching the sibling exactly) and corrected the comment. Full DotLLM.Tests.Unit.Cuda suite, same conditions that previously reproduced the failure: before - 1 failed (this test); after - 339 total / 303 passed / 36 skipped / 0 failed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adjusts a flaky CUDA graph-capture equivalence unit test by aligning its logit-diff tolerance with the already-established tolerance used in the sibling test, and updates the in-test rationale to reflect observed full-suite floating-point drift behavior.
Changes:
- Loosened
EagerVsGraphDecode_QuantizedKv_Matchfirst-step logit max-abs-diff tolerance from1e-3fto5.0f. - Replaced the “bit-identical logits” rationale comment with an explanation of suite-order-dependent FP drift (PTX-JIT/module registration effects).
Comment on lines
+254
to
+258
| // Argmax MUST match at every step — this is the real correctness gate (same | ||
| // reasoning as EagerVsGraphDecode_Match above). Logit VALUES are not actually | ||
| // bit-identical in practice: PTX-JIT cache state left behind by other tests in | ||
| // the same process (module/kernel registration order shifts SASS scheduling, | ||
| // which shifts FP accumulation order in identical arithmetic) perturbs them by |
This was referenced Jul 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #174.
Summary
Root-caused the documented pre-existing full-suite-only flake in
CudaGraphCaptureEquivalenceTest.EagerVsGraphDecode_QuantizedKv_Match. It asserted a 1e-3f logittolerance claiming eager/graph paths are bit-identical — but its sibling test in the same file
already documents and tolerates (5.0f) the real cause: benign PTX-JIT/kernel-registration-order FP
drift from other tests running first in the same process. Confirmed via direct repro: 0.0 diff
standalone, ~0.20 diff in the full suite — inside the sibling's tolerance, outside this test's.
Loosened to match (5.0f) and corrected the comment.
Test plan
DotLLM.Tests.Unit.Cudasuite, same conditions that previously reproduced the failure:339 total / 303 passed / 36 skipped / 0 failed (previously 1 failed — this test)
🤖 Generated with Claude Code