[https://nvbugs/6276842][test] Loosen rtol/atol on encoder CUDA graph logits parity check

[tingyangk]

Summary by CodeRabbit

• Tests
  • Adjusted decoder model CUDA graph accuracy test tolerances
  • Re-enabled previously skipped accuracy validation test

Description

Loosens rtol/atol for the encoder CUDA-graph vs eager logits parity check because cuBLAS picks different GEMM kernels for o_proj and down_proj at the eager-M (sum(seq_lens)) vs graph-M (padded_num_tokens), producing tiny numerical gaps that compound across multiple decoder layers to a final logits diff.

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-compatible or api-breaking. For api-breaking, include BREAKING in 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.

[coderabbitai]

📝 Walkthrough

Walkthrough

The CUDA-graph vs eager decoder encode logit comparison tolerances in test_decoder_encode_cuda_graph_matches_eager_logits are relaxed (rtol: 1e-3 → 5e-2, atol: 1e-3 → 0.5). The corresponding SKIP waiver for the tinyllama-1.1b variant on L40S is removed from the integration test waive list.

Changes

CUDA-graph tolerance fix and waiver removal

Layer / File(s)	Summary
Loosen tolerances and remove SKIP waiver tests/integration/defs/accuracy/test_llm_api_pytorch_encode.py, tests/integration/test_lists/waives.txt	assert_close tolerances for CUDA-graph vs eager logits are increased (rtol to 5e-2, atol to 0.5), and the SKIP entry for test_decoder_encode_cuda_graph_matches_eager_logits[tinyllama-1.1b] on L40S is deleted.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

• NVIDIA/TensorRT-LLM#15337: Modifies tests/integration/test_lists/waives.txt with SKIP changes for test_llm_api_pytorch_encode CUDA-graph accuracy cases, directly overlapping with this PR's waive-list edit.
• NVIDIA/TensorRT-LLM#15389: Targets the same test_decoder_encode_cuda_graph_matches_eager_logits L40S waiver entry in waives.txt.
• NVIDIA/TensorRT-LLM#15214: Also modifies tests/integration/test_lists/waives.txt to unskip specific accuracy tests.

Suggested reviewers

• jieli-matrix
• crazydemo

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description check	✅ Passed	The description provides a clear explanation of the issue (cuBLAS kernel differences) and solution (loosening tolerances). The PR checklist is completed, though Test Coverage section is empty.
Title check	✅ Passed	The title accurately summarizes the main change: loosening CUDA graph logits parity tolerances for the encoder test.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

tests/integration/defs/accuracy/test_llm_api_pytorch_encode.py (1)

439-439: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Update misleading comment about "tight numerical bound".

The docstring claims "Tight numerical bound" but the tolerances are now rtol=5e-2 (5%) and atol=0.5, which are quite loose. This will mislead future maintainers about the test's purpose and strictness.

📝 Suggested docstring update

-    def test_decoder_encode_cuda_graph_matches_eager_logits(self, model_name, model_path):
-        """Tight numerical bound: decoder single-prefill graph replay must reproduce eager logits."""
+    def test_decoder_encode_cuda_graph_matches_eager_logits(self, model_name, model_path):
+        """Decoder single-prefill CUDA graph replay should produce logits close to eager mode.
+        
+        Tolerances are relaxed to account for numerical precision differences from
+        cuBLAS kernel selection between eager (using sum of sequence lengths) and
+        CUDA graph (using padded token counts) execution modes.
+        """

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/integration/defs/accuracy/test_llm_api_pytorch_encode.py` at line 439,
The docstring at line 439 claims "Tight numerical bound" but the actual
tolerance values used in the test are rtol=5e-2 (5%) and atol=0.5, which are
considered loose tolerances. Update the docstring to accurately reflect that the
test uses loose tolerances rather than tight ones. This will ensure future
maintainers understand the actual strictness level of the numerical comparison
without being misled by the current wording.

🧹 Nitpick comments (1)

tests/integration/defs/accuracy/test_llm_api_pytorch_encode.py (1)

456-456: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff

Consider documenting tolerance rationale and adding semantic checks.

The tolerances have been loosened substantially (50x for rtol, 500x for atol). While the PR objectives provide technical justification, consider:

1. Document inline why these specific values: Add a comment explaining that these tolerances account for cuBLAS kernel selection differences between eager and CUDA graph modes, similar to the detailed explanation in the PR description.

2. Consider semantic checks: The test_decoder_encode_matches_huggingface test uses a two-tier approach (top-K overlap + focused numerical check) that may be more robust than pure numerical comparison. With atol=0.5, logits can differ by 0.5 absolute units—would a top-K semantic check provide better signal that the outputs are functionally equivalent?

3. Verify test effectiveness: Confirm that these tolerances still catch meaningful correctness issues rather than just allowing any output to pass.

💭 Example: Adding tolerance documentation

         eager = torch.stack([o.logits.cpu() for o in eager_outs])
         graph = torch.stack([o.logits.cpu() for o in graph_outs])
-        torch.testing.assert_close(graph, eager, rtol=5e-2, atol=0.5)
+        # Relaxed tolerances account for numerical precision differences from
+        # cuBLAS kernel selection (eager uses sum of seq lengths, graph uses
+        # padded tokens). Differences accumulate across decoder layers.
+        torch.testing.assert_close(graph, eager, rtol=5e-2, atol=0.5)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/integration/defs/accuracy/test_llm_api_pytorch_encode.py` at line 456,
The torch.testing.assert_close call in the comparison between graph and eager
results uses substantially loosened tolerances (rtol=5e-2, atol=0.5) without
documentation explaining the rationale. Add an inline comment immediately before
the assert_close call to explain why these specific tolerance values are
necessary, specifically documenting that they account for cuBLAS kernel
selection differences between eager and CUDA graph execution modes.
Additionally, consider supplementing this numerical tolerance check with
semantic validation checks similar to the two-tier approach used in
test_decoder_encode_matches_huggingface (such as top-K overlap validation) to
ensure the outputs remain functionally equivalent and that the loosened
tolerances do not mask meaningful correctness issues.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@tests/integration/defs/accuracy/test_llm_api_pytorch_encode.py`:
- Line 439: The docstring at line 439 claims "Tight numerical bound" but the
actual tolerance values used in the test are rtol=5e-2 (5%) and atol=0.5, which
are considered loose tolerances. Update the docstring to accurately reflect that
the test uses loose tolerances rather than tight ones. This will ensure future
maintainers understand the actual strictness level of the numerical comparison
without being misled by the current wording.

---

Nitpick comments:
In `@tests/integration/defs/accuracy/test_llm_api_pytorch_encode.py`:
- Line 456: The torch.testing.assert_close call in the comparison between graph
and eager results uses substantially loosened tolerances (rtol=5e-2, atol=0.5)
without documentation explaining the rationale. Add an inline comment
immediately before the assert_close call to explain why these specific tolerance
values are necessary, specifically documenting that they account for cuBLAS
kernel selection differences between eager and CUDA graph execution modes.
Additionally, consider supplementing this numerical tolerance check with
semantic validation checks similar to the two-tier approach used in
test_decoder_encode_matches_huggingface (such as top-K overlap validation) to
ensure the outputs remain functionally equivalent and that the loosened
tolerances do not mask meaningful correctness issues.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: f40725e7-d967-4861-bff1-c271899c31ed

📥 Commits

Reviewing files that changed from the base of the PR and between f536026 and 4de582c.

📒 Files selected for processing (2)

• tests/integration/defs/accuracy/test_llm_api_pytorch_encode.py
• tests/integration/test_lists/waives.txt

💤 Files with no reviewable changes (1)

• tests/integration/test_lists/waives.txt

[tingyangk]

/bot run --disable-fail-fast

[tensorrt-cicd]

PR_Github #55087 [ run ] triggered by Bot. Commit: b8f1041 Link to invocation

[tensorrt-cicd]

PR_Github #55087 [ run ] completed with state SUCCESS. Commit: b8f1041
/LLM/main/L0_MergeRequest_PR pipeline #44073 completed with status: 'SUCCESS'

CI Report

Link to invocation