[None][fix] remove unsupported INT8 from trtllm-bench --quantization choices by lonexreb · Pull Request #16245 · NVIDIA/TensorRT-LLM

lonexreb · 2026-07-10T16:57:59Z

Description

trtllm-bench build --help advertised INT8 as a valid --quantization value, but selecting it crashed:

AssertionError  # QuantMode.from_quant_algo: quant_algo not in QUANT_ALGO_LIST

QUANT_ALGO_LIST (in tensorrt_llm/quantization/mode.py) deliberately excludes QuantAlgo.INT8 (set(QuantAlgo) - {QuantAlgo.INT8}), so INT8 is not buildable via this path — yet it was still listed in VALID_QUANT_ALGOS, which feeds the Click --quantization choices.

Change

Remove INT8 from VALID_QUANT_ALGOS in tensorrt_llm/bench/utils/__init__.py so the advertised choices match what the build path supports (with a comment explaining the exclusion).

Test

New CPU-only unit tests in tests/unittest/others/test_bench_quant_choices.py: INT8 is not an offered choice, every offered choice is accepted by QuantMode.from_quant_algo, and INT8 still raises (documenting the exclusion).

Note

A prior identical fix (PR #13292) was closed unmerged without explanation. This aligns the advertised choices with the code's deliberate exclusion; if maintainers instead intend to add INT8 support to the build path, that is a larger change and this can be closed in favor of it.

Summary by CodeRabbit

Bug Fixes
- Removed unsupported INT8 from the advertised benchmark quantization options, preventing invalid build-time selections.
Tests
- Added validation to ensure all available benchmark quantization choices resolve correctly.
- Added coverage confirming INT8 remains rejected as an unsupported quantization mode.

chatgpt-codex-connector · 2026-07-10T16:58:05Z

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

coderabbitai · 2026-07-10T17:00:34Z

📝 Walkthrough

Walkthrough

VALID_QUANT_ALGOS no longer advertises INT8, and new tests verify that advertised algorithms convert successfully while INT8 raises an assertion.

Changes

Quantization choice alignment

Layer / File(s)	Summary
Choice contract and validation `tensorrt_llm/bench/utils/__init__.py`, `tests/unittest/others/test_bench_quant_choices.py`	`VALID_QUANT_ALGOS` excludes INT8, and tests validate the advertised choices and INT8 rejection in `QuantMode.from_quant_algo`.

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

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main fix: removing unsupported INT8 from trtllm-bench quantization choices.
Description check	✅ Passed	The description is mostly complete and explains the bug, fix, tests, and context, though it omits the template's exact PR checklist section.
Linked Issues check	✅ Passed	The change matches `#7091` by removing INT8 from the advertised choices and adding tests to keep the help output aligned with supported algorithms.
Out of Scope Changes check	✅ Passed	The changes stay scoped to the quantization choice list, its documentation comment, and targeted tests.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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

coderabbitai

🧹 Nitpick comments (1)

tests/unittest/others/test_bench_quant_choices.py (1)
23-39: 📐 Maintainability & Code Quality | 🔵 Trivial

QA list update is unnecessary.

This is narrow CPU-only unittest coverage and does not alter scheduled integration scope.
🤖 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/unittest/others/test_bench_quant_choices.py` around lines 23 - 39, The
added tests in test_int8_not_offered_as_bench_quant_choice,
test_every_offered_choice_is_buildable, and
test_int8_from_quant_algo_still_raises are unnecessary CPU-only QA coverage;
remove these tests and leave the scheduled integration test scope unchanged.
Source: Path instructions

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

Nitpick comments:
In `@tests/unittest/others/test_bench_quant_choices.py`:
- Around line 23-39: The added tests in
test_int8_not_offered_as_bench_quant_choice,
test_every_offered_choice_is_buildable, and
test_int8_from_quant_algo_still_raises are unnecessary CPU-only QA coverage;
remove these tests and leave the scheduled integration test scope unchanged.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 1cfb1d1f-1156-4889-9f1b-f0799e112328

📥 Commits

Reviewing files that changed from the base of the PR and between edf8f6f and be293ae.

📒 Files selected for processing (2)

tensorrt_llm/bench/utils/__init__.py
tests/unittest/others/test_bench_quant_choices.py

lonexreb · 2026-07-16T02:03:14Z

Thanks @dc3671 — addressed: removed the comment and slimmed to the single test_int8_not_offered_as_bench_quant_choice test. On combining into a single test_bench.py: happy to do that consolidation of test_bench_data.py + the new test as a follow-up if you'd like — let me know and I'll fold it in.

dc3671 · 2026-07-16T04:27:20Z

@lonexreb Yes, please help combine two existed files and your newly added file into test_bench.py. Thanks~

lonexreb · 2026-07-16T04:34:52Z

Done — combined test_bench_data.py and the new INT8 quant-choice test into a single tests/unittest/others/test_bench.py and removed the two originals. Thanks @dc3671!

dc3671 · 2026-07-16T04:44:38Z

@lonexreb There is another file: test_bench_statistics.py , and seems there are conflict with upstream

Combine test_bench_data.py and test_bench_statistics.py with the new INT8 quant-choice test into a single tests/unittest/others/test_bench.py, per review on NVIDIA#16245. All upstream tests are preserved verbatim. Signed-off-by: lonexreb <reach2shubhankar@gmail.com>

lonexreb · 2026-07-16T05:48:31Z

@dc3671 Thanks for catching that — done. Two things were wrong on my side:

Stale base: the branch was based on an old main, which is why it conflicted with upstream and why I'd missed test_bench_statistics.py entirely (it didn't exist on my base). I've rebased onto latest main.
Consolidation: test_bench_data.py, test_bench_statistics.py, and the new INT8 quant-choice test are now all combined into a single tests/unittest/others/test_bench.py. All upstream tests (including the recently added tokenizer-routing test) are preserved verbatim — 6 existing tests + 1 new.

GitHub now reports the PR as mergeable with no conflicts. The diff is: the one-line VALID_QUANT_ALGOS fix + the consolidated test file.

dc3671 · 2026-07-16T05:58:56Z

/bot run

tensorrt-cicd · 2026-07-16T06:05:26Z

PR_Github #59630 [ run ] triggered by Bot. Commit: ee56db3 Link to invocation

tensorrt-cicd · 2026-07-16T07:41:27Z

PR_Github #59630 [ run ] completed with state SUCCESS. Commit: ee56db3
/LLM/main/L0_MergeRequest_PR pipeline #48067 completed with status: 'FAILURE'

CI Report

⚠️ 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

CI Agent Failure Analysis

Link to invocation

…choices INT8 was advertised as a trtllm-bench build --quantization choice but is excluded from QUANT_ALGO_LIST, so selecting it crashed with an AssertionError in QuantMode.from_quant_algo. Drop INT8 from VALID_QUANT_ALGOS so the advertised choices match what the build path actually supports, and cover it with a test in test_bench_data.py. Fixes NVIDIA#7091 Signed-off-by: lonexreb <reach2shubhankar@gmail.com>

Combine test_bench_data.py and test_bench_statistics.py with the new INT8 quant-choice test into a single tests/unittest/others/test_bench.py, per review on NVIDIA#16245. All upstream tests are preserved verbatim. Signed-off-by: lonexreb <reach2shubhankar@gmail.com>

dc3671 · 2026-07-16T12:59:26Z

/bot run

tensorrt-cicd · 2026-07-16T13:13:50Z

PR_Github #59713 [ run ] triggered by Bot. Commit: 098daa2 Link to invocation

tensorrt-cicd · 2026-07-16T13:29:11Z

PR_Github #59713 [ run ] completed with state FAILURE. Commit: 098daa2
/LLM/main/L0_MergeRequest_PR pipeline #48140 completed with status: 'FAILURE'

CI Report

⚠️ 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

CI Agent Failure Analysis

Link to invocation

dc3671 · 2026-07-16T13:42:42Z

/bot run

tensorrt-cicd · 2026-07-16T13:49:41Z

PR_Github #59717 [ run ] triggered by Bot. Commit: 098daa2 Link to invocation

tensorrt-cicd · 2026-07-16T16:00:11Z

PR_Github #59717 [ run ] completed with state SUCCESS. Commit: 098daa2
/LLM/main/L0_MergeRequest_PR pipeline #48144 completed with status: 'FAILURE'

CI Report

⚠️ 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

CI Agent Failure Analysis

Link to invocation

dc3671 · 2026-07-18T01:46:26Z

/bot run --disable-fail-fast

tensorrt-cicd · 2026-07-18T01:52:26Z

PR_Github #60077 [ run ] triggered by Bot. Commit: 098daa2 Link to invocation

tensorrt-cicd · 2026-07-18T04:51:01Z

PR_Github #60077 [ run ] completed with state SUCCESS. Commit: 098daa2
/LLM/main/L0_MergeRequest_PR pipeline #48464 completed with status: 'SUCCESS'

CI Report

Link to invocation

BowenFu

Reviewed the bench quantization choice fix. The change is narrow and aligns the advertised trtllm-bench --quantization choices with the existing backend contract: QuantMode.from_quant_algo excludes QuantAlgo.INT8 for the main quantization path, so offering INT8 here would deterministically fail rather than enable a supported build mode.\n\nThe test consolidation preserved the existing bench tests and adds targeted coverage that INT8 is not offered as a bench quantization choice. Final Blossom CI on the head commit is green, and the earlier change request was addressed with a subsequent member approval.\n\nAlso ran an independent Claude cross-check before approving; it agreed this is safe to approve. Non-blocking: a future follow-up could add a broader invariant test that every advertised choice is accepted by QuantMode.from_quant_algo.

lonexreb requested a review from a team as a code owner July 10, 2026 16:58

lonexreb requested a review from dc3671 July 10, 2026 16:58

github-actions Bot assigned lonexreb Jul 10, 2026

coderabbitai Bot reviewed Jul 10, 2026

View reviewed changes

dc3671 requested changes Jul 15, 2026

View reviewed changes

Comment thread tests/unittest/others/test_bench_quant_choices.py Outdated

Comment thread tensorrt_llm/bench/utils/__init__.py Outdated

lonexreb requested review from a team as code owners July 16, 2026 02:03

lonexreb requested review from brnguyen2, dc3671, dhansen-nvidia and schetlur-nv July 16, 2026 02:03

lonexreb force-pushed the fix/bench-int8-quant-choice branch from 9486cae to aaa1ce4 Compare July 16, 2026 05:40

dc3671 approved these changes Jul 16, 2026

View reviewed changes

lonexreb added 2 commits July 16, 2026 20:59

dc3671 force-pushed the fix/bench-int8-quant-choice branch from ee56db3 to 098daa2 Compare July 16, 2026 12:59

BowenFu approved these changes Jul 20, 2026

View reviewed changes

dc3671 merged commit 26e476a into NVIDIA:main Jul 20, 2026
7 checks passed

Uh oh!

Conversation

lonexreb commented Jul 10, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Change

Test

Note

Summary by CodeRabbit

Uh oh!

chatgpt-codex-connector Bot commented Jul 10, 2026

Uh oh!

coderabbitai Bot commented Jul 10, 2026

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lonexreb commented Jul 16, 2026

Uh oh!

dc3671 commented Jul 16, 2026

Uh oh!

lonexreb commented Jul 16, 2026

Uh oh!

dc3671 commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lonexreb commented Jul 16, 2026

Uh oh!

dc3671 commented Jul 16, 2026

Uh oh!

tensorrt-cicd commented Jul 16, 2026

Uh oh!

tensorrt-cicd commented Jul 16, 2026

Uh oh!

dc3671 commented Jul 16, 2026

Uh oh!

tensorrt-cicd commented Jul 16, 2026

Uh oh!

tensorrt-cicd commented Jul 16, 2026

Uh oh!

dc3671 commented Jul 16, 2026

Uh oh!

tensorrt-cicd commented Jul 16, 2026

Uh oh!

tensorrt-cicd commented Jul 16, 2026

Uh oh!

dc3671 commented Jul 18, 2026

Uh oh!

tensorrt-cicd commented Jul 18, 2026

Uh oh!

tensorrt-cicd commented Jul 18, 2026

Uh oh!

BowenFu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

lonexreb commented Jul 10, 2026 •

edited by coderabbitai Bot

Loading

dc3671 commented Jul 16, 2026 •

edited

Loading