[https://nvbugs/6577550][chore] Waive TestNemotron3Super120B::test_ctx_dp2_gen_tp4 - #17504
Conversation
…x_dp2_gen_tp4 The GSM8K accuracy regression tracked by nvbugs/6577550 is not specific to B300. The same test with the same signature first failed on B200 under nvbugs/6546605 (64.594 vs. a 89.497 threshold), was closed as fixed by NVIDIA#16072, and has since resurfaced at 26.876 on B300. Widen the waive from the B300 GPU prefix to all stages so B200 and DGX_B200 are covered too -- note that `full:B200/` would not have matched the `DGX_B200-*` stages anyway, since parse_test_name only substitutes a test prefix that appears in the line. Signed-off-by: Yihan Wang <yihwang@nvidia.com>
|
/bot --help |
GitHub Bot Help
Provide a user friendly way for developers to interact with a Jenkins server. Run See details below for each supported subcommand. Details
Launch build/test pipelines. All previously running jobs will be killed.
kill
Kill all running builds associated with pull request. skip
Skip testing for latest commit on pull request. reuse-pipeline
Reuse a previous pipeline to validate current commit. This action will also kill all currently running builds associated with the pull request. IMPORTANT NOTE: This is dangerous since lack of user care and validation can cause top of tree to break. |
WalkthroughThe standard accuracy waiver list adds a skip for the Nemotron3 Super 120B context/data-parallel test. ChangesNemotron3 test waiver
Estimated code review effort: 1 (Trivial) | ~2 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@tests/integration/test_lists/waives.txt`:
- Line 16: Add the repository-standard NVIDIA copyright header, including 2026,
at the top of the waiver-list file while preserving the existing waiver entry
and parser-compatible format.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 1c6baf6f-a133-44ba-832d-eebdecd90f78
📒 Files selected for processing (1)
tests/integration/test_lists/waives.txt
|
/bot skip --comment "Just waived 1 test" |
|
PR_Github #65315 [ skip ] triggered by Bot. Commit: |
|
PR_Github #65315 [ skip ] completed with state |
Description
accuracy/test_disaggregated_serving.py::TestNemotron3Super120B::test_ctx_dp2_gen_tp4is currently waived only under thefull:B300/GPU prefix (nvbugs/6577550). The underlying GSM8K accuracy regression is not B300-specific:d924d9f1f0dc88f2Same test, same failure signature, both categorized
Module_Disaggregated servingand marked as regressions. The failure originally surfaced on B200; it was declared fixed by #16072 and has since resurfaced on B300 at an even lower score, so #16072 appears to have masked rather than fixed it. The test has now been observed failing on B200 again.This PR widens the waive from the
full:B300/prefix to an unprefixed entry so every stage is covered.Note that simply adding a
full:B200/line would not have been sufficient:parse_test_nameintests/integration/defs/test_list_parser.pyonly substitutes the stage's test prefix whentest_prefix.split('-')[0]appears in the line, sofull:B200/matchesB200-*stages but not theDGX_B200-*stages where this test also runs (l0_dgx_b200.yml).Only
test_ctx_dp2_gen_tp4is affected. The siblingtest_auto_dtypeparametrizations keep their existing, separately tracked waives.Test Coverage
Test-list-only change; no product code is touched. The
waives.txtduplicate check and the test-list-entry AST validation pre-commit hooks both pass.PR Checklist
Please review the following before submitting your PR:
PR description clearly explains what and why.
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.🤖 Generated with Claude Code
Dev Engineer Review
tests/integration/test_lists/waives.txt.full:B300/waiver with an unprefixed waiver fortest_ctx_dp2_gen_tp4.DGX_B200-*.test_auto_dtypewaivers remain unchanged.Verdict: sufficient
QA Engineer Review
tests/integration/test_lists/waives.txt.accuracy/test_disaggregated_serving.py::TestNemotron3Super120B::test_ctx_dp2_gen_tp4.test-db/orqa/files changed.Verdict: needs follow-up because CBTS coverage data is unavailable.