Repository navigation
[https://nvbugs/6985375][test] Remove waiver for the mixed-precision retained-dtype test - #21227
Conversation
…retained-dtype test The test broke on main on 2026-10-24 by a merge race: NVIDIA#20167 started reading model_config.sparse_attention_config unconditionally three hours before NVIDIA#20452 merged with a config stub that lacked the field. NVIDIA#18843 added the field to the stub on 2026-10-26, so the test passes on main again and the waiver added by NVIDIA#20927 is no longer needed. Signed-off-by: Tianrui 'Hudayday' Hu <32944717+Hudayday@users.noreply.github.com>
|
/bot run --disable-fail-fast |
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Remove the test waiver tests/integration/test_lists/waives.txt |
The skip-list entry for test_mixed_precision_retained_dtype_must_match_model_dtype was removed. |
Priority:
Estimated code review effort: 1 (Trivial) | ~3 minutes
Change: Other
Suggested reviewers: trtllm-agent
Merge Risk: 
27d0b
-
No merge-blocking issue is identified; the waiver removal is ready for normal checks.
Pre-merge checks: 
-
Check name Status Explanation Title check The title follows the repository template: it has an NVBugs ID tag, a [test] type, and a short summary. It names the main change, which is removing the waiver for the mixed-precision retained-dtype te… Description check The description covers the required parts of the template. It explains the problem (a merge race that broke the test on main), the fix (removing the waiver), and the NVBug link. It also gives test cov… Docstring Coverage No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title follows the repository template: it has an NVBugs ID tag, a [test] type, and a short summary. It names the main change, which is removing the waiver for the mixed-precision retained-dtype test. The title is concise and specific.
Full details: Description check
-
Explanation
The description covers the required parts of the template. It explains the problem (a merge race that broke the test on main), the fix (removing the waiver), and the NVBug link. It also gives test coverage details and a completed checklist. The template's Test Coverage section is filled in, and the checklist is ticked. The description is mostly complete.
Full details: Docstring Coverage
-
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 0 files.
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
|
/bot run --disable-fail-fast |
|
PR_Github #81320 [ run ] triggered by Bot. Commit: |
|
Automatically added "ci: full pre-merge approved" because this PR has satisfied the required GitHub review approvals. Unresolved review conversations and other required checks remain independent merge requirements. |
|
PR_Github #81320 [ run ] completed with state |
Dev Engineer Review
The change removes the waiver for
test_mixed_precision_retained_dtype_must_match_model_dtypefromtests/integration/test_lists/waives.txt. It changes no runtime code. The stated rationale is that the test’s config stub now includessparse_attention_config, so the waiver is no longer needed.QA Engineer Review
The test-list change removes one waiver entry for the mixed-precision retained-dtype test. The author reports that the test passes in the CPU-Generic stage and that pre-commit, including test-list validation, passes; these results were not independently verified here. Coverage verdict: needs follow-up to confirm the test remains unwaived and passes in CI.
Per-File QA Perspective
tests/integration/test_lists/waives.txt: Removes the waiver for the named test. QA should confirm the test runs without the waiver in the appropriate CI stage.Description
Removes the waiver for
unittest/_torch/attention/test_mixed_precision_backends.py::test_mixed_precision_retained_dtype_must_match_model_dtypefrom
tests/integration/test_lists/waives.txt.The test broke on main on 2026-10-24 by a merge race: #20167 started reading
model_config.sparse_attention_configunconditionally three hours before #20452merged with a config stub that lacked the field, so #20452's pre-merge CI could not
see the failure. #20927 waived the test; #18843 then added
sparse_attention_config=Noneto the stub on 2026-10-26, so the test passes on mainagain and the waiver is no longer needed.
Resolves https://nvbugs/6985375 (https://nvbugs/6985374 is a duplicate filed from the
same build). Test-list change only; no runtime impact.
Test Coverage
The unwaived test itself,
test_mixed_precision_retained_dtype_must_match_model_dtype(CPU-Generic stage). Pre-commit, including the test-list validation hook, passes on the
change.
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.