[https://nvbugs/6426860][fix] Stabilize compressor BF16 tolerance - #16376
Conversation
Signed-off-by: Mingyang Hao <200044211+mingyangHao@users.noreply.github.com>
|
/bot run |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThe variable-length DeepSeek V4 prefill test now derives relative tolerance from BF16 epsilon, while retaining the existing absolute tolerance. Its corresponding integration test waiver is removed. ChangesDeepSeek V4 prefill validation
Estimated code review effort: 2 (Simple) | ~5 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
PR_Github #59210 [ run ] triggered by Bot. Commit: |
|
PR_Github #59210 [ run ] completed with state |
The previous commit mirrored NVIDIA#16376's rtol = max(2e-3, eps). Working through the arithmetic shows one ulp is too thin, and also that the original tolerance was not merely tight but unsatisfiable above |x| ~ 1. One ulp is eps*|x| and grows with magnitude; rtol=2e-3 grows far more slowly: |x| 1 ulp allowed by rtol=2e-3,atol=5e-3 0.5 0.00391 0.00600 ok 1.0 0.00781 0.00700 FAILS 2.0 0.01562 0.00900 FAILS 4.0 0.03125 0.01300 FAILS The inputs are torch.randn, so values above 1.0 are routine, and any tie rounding there fails. That the test passes at all is down to ties being rare -- which is exactly the intermittent signature seen in CI. rtol = max(2e-3, eps) tracks magnitude correctly but leaves only ~1.6x headroom over a single ulp. Use two ulps, giving 2.2x-8.4x across the magnitude range, still orders of magnitude below the deviation a functional kernel error would produce. Apply it to the existing test_prefill_varlen site as well, so all four BF16 output comparisons in the file use one rule rather than two. Signed-off-by: Brian Nguyen <brnguyen@nvidia.com>
The BF16 output comparisons in test_compressor_kernel.py fail intermittently in pre-merge, on unrelated PRs and across architectures, all with the same signature -- a max diff of 0.007812 against rtol=2e-3, atol=5e-3: build 53189 B300-PyTorch-1 test_prefill_corner_cases[overlap_hd512_5chunks] build 53218 DGX_H100-PyTorch-6 test_prefill_then_decode[overlap_hd512_prefill20_decode12] build 53223 DGX_B200-PyTorch-2 test_prefill_corner_cases[overlap_hd512_5chunks] build 53236 B300-PyTorch-1 test_prefill_corner_cases[overlap_large_head_dim] 0.007812 is one BF16 ulp at |x| ~ 1.0. The CUDA and PyTorch reductions can land on adjacent representable values near a tie, so a one-ulp difference is expected; NVIDIA#16376 established that for this kernel. The problem is that the tolerance does not cover it: one ulp is eps*|x| and grows with magnitude, while rtol=2e-3 grows far more slowly, so the comparison is not merely tight, it is unsatisfiable above |x| ~ 1: |x| 1 ulp allowed by rtol=2e-3, atol=5e-3 0.5 0.00391 0.00600 ok 1.0 0.00781 0.00700 fails 2.0 0.01562 0.00900 fails 4.0 0.03125 0.01300 fails The inputs are torch.randn, so values above 1.0 are routine and any tie rounding there fails. That the test passes at all is down to ties being rare, which is the intermittent signature seen in CI. NVIDIA#16376 fixed this in test_prefill_varlen with rtol = max(2e-3, eps), which tracks magnitude correctly but leaves only ~1.6x headroom over a single ulp, and it was never applied to the sibling assertions. Use two ulps, and apply it to all four BF16 output comparisons in the file: the prefill output in test_prefill_corner_cases, the prefill and decode outputs in test_prefill_then_decode, and the existing site in test_prefill_varlen. Verified on SM100 by repeat-running the three affected parametrizations 20000 times per arm with a distinct seed per iteration (120000 test-body executions), instrumenting every comparison with the ratio |a-b| / (atol + rtol*|b|): parametrization fails before fails after max ratio new corner_cases[overlap_hd512_5chunks] 32/20000 0/20000 0.417 corner_cases[overlap_large_head_dim] 48/20000 0/20000 0.406 then_decode[hd512_prefill20_decode12] 51/20000 0/20000 0.377 The count of ratio>1 under the old tolerance matches the observed failure count exactly in all three cases, so the analysis and the flake are the same event. Every max observed diff is exactly one or two BF16 ulps. Worst case across all 120000 comparisons uses 42% of the new allowance, so two ulps has margin while staying orders of magnitude below any functional error. Tolerances elsewhere in the file are untouched: they compare different quantities and have not been observed flaking. Signed-off-by: Brian Nguyen <brnguyen@nvidia.com>
Summary by CodeRabbit
Bug Fixes
Tests
Description
Test Coverage
PR Checklist
Please review the following before submitting your PR:
PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.
PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.
Test cases are provided for new code paths (see test instructions)
If PR introduces API changes, an appropriate PR label is added - either
api-compatibleorapi-breaking. Forapi-breaking, includeBREAKINGin the PR title.Any new dependencies have been scanned for license and vulnerabilities
CODEOWNERS updated if ownership changes
Documentation updated as needed
Update tava architecture diagram if there is a significant design change in PR.
The reviewers assigned automatically/manually are appropriate for the PR.
Please check this after reviewing the above items as appropriate for this PR.
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.