[https://nvbugs/6777501][fix] Fix nemotron breakable cuda graph test parity check by dominicshanshan · Pull Request #19335 · NVIDIA/TensorRT-LLM

dominicshanshan · 2026-09-17T07:04:06Z

Verified on local H100 with Nano-30B-A3B-BF16: tp1 and tp2 pass (worst first-token log-prob diff 0.16 / 0.24 nats, greedy-pick margin 0.0 nats).

Dev Engineer Review

Updated Nemotron CUDA graph parity prompts to use trained tokenizer ID 17.
Replaced top-2 membership checks with symmetric log-probability margin checks using the 2.30 tolerance.
Removed the obsolete NVBUG 6777501 waiver.
No material API, configuration, performance, or scope risks were identified.

QA Engineer Review

Modified test_nemotron_h_breakable_prefill_cuda_graph(tp_size).
Covered context batches, mixed batches, unequal lengths, and chunked prefill.
Removed the tp1 waiver from waives.txt.
The test is listed in CI test-db files:
- l0_h100.yml for tp1
- l0_dgx_h100.yml for tp2
Local H100 validation passed for Nano-30B-A3B-BF16. Worst first-token differences were 0.16 nats for tp1 and 0.24 nats for tp2.
Coverage verdict: sufficient.

Per-File QA Perspective

tests/unittest/_torch/modeling/test_modeling_nemotron_h.py: Validates Nemotron breakable prefill CUDA graph parity across single- and multi-request scenarios. The tp1 and tp2 test variants are listed in the H100 CI test-db configurations.
tests/integration/test_lists/waives.txt: Removes the tp1 skip entry for test_nemotron_h_breakable_prefill_cuda_graph, allowing the test to run in CI.

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-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.

…mmune and use a trained prompt token test_nemotron_h_breakable_prefill_cuda_graph[tp1] flaked with "request 3: BCG first token not in eager top-2", with eager's own NVIDIA#2/NVIDIA#3 candidates swapping between runs. Request 3 is `[23] * 65`: in the Nemotron-3 tokenizer ids 0-513 are added control tokens and 23/31 are untrained <SPECIAL_n> placeholders (the prompt ids were ported from the Qwen3.5-4B parity test, where they are ordinary BPE tokens). Repeating an untrained token yields a flat next-token distribution whose leading candidates tie within run-to-run noise, so a rank test on BCG's greedy pick is a coin flip even though the distributions agree within the accepted log-prob bound. Padding-induced MoE drift under BCG only reorders those near-tied candidates. - Build every prompt from id 17 (`</tool_response>`, trained); block reuse is off by default for NemotronH so the shared prefix is never reused. - Replace the top-2 membership assertion with a symmetric margin check: each arm's greedy pick must lie within _BCG_LOGPROB_TOLERANCE of the other arm's pick under the other arm's distribution. Near ties pass, genuine divergence still fails. - Drop the waives.txt entry. Verified on H100 with Nano-30B-A3B-BF16: tp1 and tp2 pass (worst first-token log-prob diff 0.16 / 0.24 nats, greedy-pick margin 0.0 nats). Signed-off-by: Wangshanshan <30051912+dominicshanshan@users.noreply.github.com>

dominicshanshan · 2026-09-17T07:15:49Z

/bot run

coderabbitai · 2026-09-17T07:20:34Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 59f15c1d-9562-41a2-b032-235b8961b706

📥 Commits

Reviewing files that changed from the base of the PR and between 73c7063 and 9233f6b.

📒 Files selected for processing (2)

tests/integration/test_lists/waives.txt
tests/unittest/_torch/modeling/test_modeling_nemotron_h.py

💤 Files with no reviewable changes (1)

tests/integration/test_lists/waives.txt

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.

Walkthrough

The Nemotron-H CUDA-graph test now uses token 17 consistently and validates eager and BCG greedy-token probability margins. The associated skip waiver was removed.

Changes

Nemotron-H validation

Layer / File(s)	Summary
Repeated-token prompt fixtures `tests/unittest/_torch/modeling/test_modeling_nemotron_h.py`	BCG context, streaming decode, and mixed-batch requests now use the shared token ID 17.
Greedy-token margin validation `tests/unittest/_torch/modeling/test_modeling_nemotron_h.py`, `tests/integration/test_lists/waives.txt`	The test replaces the eager top-two check with bidirectional 2.30-nat probability-margin checks and reports the worst margin. The related CUDA-graph skip entry was removed.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: bowenfu

Merge Risk: ⚪ Minimal · up to 9233f

The validation now uses the trained prompt token and checks both greedy-token probability margins, while the matching tp1 waiver is removed. The change is ready to merge with normal test validation.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description includes local validation results, but the required Description and Test Coverage sections remain empty, and the checklist is not substantively completed.	Add a concise explanation of the issue and solution under Description. List the relevant tests and configurations under Test Coverage. Review and mark applicable checklist items.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files.
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.
Title check	✅ Passed	The title follows the required NVBugs and fix format and clearly identifies the Nemotron breakable CUDA graph parity check change.

Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests

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

tensorrt-cicd · 2026-09-17T07:24:47Z

PR_Github #74057 [ run ] triggered by Bot. Commit: 9233f6b Link to invocation

tensorrt-cicd · 2026-09-17T09:27:36Z

PR_Github #74057 [ run ] completed with state SUCCESS. Commit: 9233f6b
/LLM/main/L0_MergeRequest_PR pipeline #60899 completed with status: 'UNSTABLE'

CI Report

⚠️ Multi-GPU Label Required:
Multi-GPU tests require the ci: full pre-merge approved label on this PR. Either:

Ask a member of NVIDIA/trt-llm-ci-approvers to add the label, or
Wait for the PR to be fully approved — the label is added automatically once approval is complete.
Then re-trigger CI with the same bot command (no rebase needed).

⚠️ Action Required:

Please check the failed tests and fix your PR
If you cannot view the failures, ask the CI triggerer to share details
Once fixed, request an NVIDIA team member to trigger CI again

Link to invocation

dominicshanshan · 2026-09-17T09:48:11Z

/bot run

tensorrt-cicd · 2026-09-17T09:54:32Z

PR_Github #74098 [ run ] triggered by Bot. Commit: 9233f6b Link to invocation

tensorrt-cicd · 2026-09-17T10:17:15Z

PR_Github #74098 [ run ] completed with state FAILURE. Commit: 9233f6b
/LLM/main/L0_MergeRequest_PR pipeline #60937 completed with status: 'UNSTABLE'

CI Report

⚠️ Multi-GPU Label Required:
Multi-GPU tests require the ci: full pre-merge approved label on this PR. Either:

Ask a member of NVIDIA/trt-llm-ci-approvers to add the label, or
Wait for the PR to be fully approved — the label is added automatically once approval is complete.
Then re-trigger CI with the same bot command (no rebase needed).

⚠️ Action Required:

Please check the failed tests and fix your PR
If you cannot view the failures, ask the CI triggerer to share details
Once fixed, request an NVIDIA team member to trigger CI again

Link to invocation

dominicshanshan requested a review from a team as a code owner September 17, 2026 07:04

dominicshanshan requested review from 2ez4bz and yechank-nvidia September 17, 2026 07:04

github-actions Bot assigned dominicshanshan Sep 17, 2026

dominicshanshan force-pushed the user/shanshan/fix_bug_BCG_nemotron branch from 24fcf49 to 9233f6b Compare September 17, 2026 07:07

dominicshanshan changed the title ~~[https://nvbugs/6777501][Fix] Fix Nemotron BCG test parity check~~ [https://nvbugs/6777501][fix] Fix Nemotron BCG test parity check Sep 17, 2026

dominicshanshan changed the title ~~[https://nvbugs/6777501][fix] Fix Nemotron BCG test parity check~~ [https://nvbugs/6777501][fix]Fix Nemotron BCG test parity check Sep 17, 2026

dominicshanshan changed the title ~~[https://nvbugs/6777501][fix]Fix Nemotron BCG test parity check~~ [https://nvbugs/6777501][fix]Fix nemotron BCG test parity check Sep 17, 2026

dominicshanshan changed the title ~~[https://nvbugs/6777501][fix]Fix nemotron BCG test parity check~~ [https://nvbugs/6777501][fix] Fix nemotron BCG test parity check Sep 17, 2026

dominicshanshan changed the title ~~[https://nvbugs/6777501][fix] Fix nemotron BCG test parity check~~ [https://nvbugs/6777501][fix] Fix nemotron breakable cuda graph test parity check Sep 17, 2026

dominicshanshan changed the title ~~[https://nvbugs/6777501][fix] Fix nemotron breakable cuda graph test parity check~~ [https://nvbugs/6777501][fix] Fix nemotron breakable cuda graph test parity check Sep 17, 2026

Navigation Menu

[https://nvbugs/6777501][fix] Fix nemotron breakable cuda graph test parity check - #19335

[https://nvbugs/6777501][fix] Fix nemotron breakable cuda graph test parity check#19335
dominicshanshan wants to merge 1 commit into
NVIDIA:mainfrom
dominicshanshan:user/shanshan/fix_bug_BCG_nemotron

dominicshanshan commented Sep 17, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

dominicshanshan commented Sep 17, 2026

Uh oh!

coderabbitai Bot commented Sep 17, 2026 •

edited

Loading

❌ Failed checks (1 warning)

Uh oh!

tensorrt-cicd commented Sep 17, 2026

Uh oh!

tensorrt-cicd commented Sep 17, 2026

Uh oh!

dominicshanshan commented Sep 17, 2026

Uh oh!

tensorrt-cicd commented Sep 17, 2026

Uh oh!

tensorrt-cicd commented Sep 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

dominicshanshan commented Sep 17, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dev Engineer Review

QA Engineer Review

Per-File QA Perspective

Description

Test Coverage

PR Checklist

GitHub Bot Help

Uh oh!

dominicshanshan commented Sep 17, 2026

Uh oh!

coderabbitai Bot commented Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

tensorrt-cicd commented Sep 17, 2026

Uh oh!

tensorrt-cicd commented Sep 17, 2026

Uh oh!

dominicshanshan commented Sep 17, 2026

Uh oh!

tensorrt-cicd commented Sep 17, 2026

Uh oh!

tensorrt-cicd commented Sep 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dominicshanshan commented Sep 17, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Sep 17, 2026 •

edited

Loading