Skip to content

[TRTLLM-14474][chore] Remove legacy python relics and refresh docs after the backend removal#16612

Open
Wanli-Jiang wants to merge 1 commit into
NVIDIA:mainfrom
Wanli-Jiang:user/williamj/trtllm-14027-python-cleanup
Open

[TRTLLM-14474][chore] Remove legacy python relics and refresh docs after the backend removal#16612
Wanli-Jiang wants to merge 1 commit into
NVIDIA:mainfrom
Wanli-Jiang:user/williamj/trtllm-14027-python-cleanup

Conversation

@Wanli-Jiang

@Wanli-Jiang Wanli-Jiang commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

Why this PR exists

Phase C removed the TensorRT engine backend. This PR removes the python
modules, packaging entries, and lint bookkeeping that existed only to
serve it, deduplicates the benchmark dataset tooling, and refreshes docs
that still described the removed workflow. It is authored on a file set
disjoint from the other Phase D PRs, so merge order does not matter.

Change groups

1. Legacy quantization-export and plugin-gen relics

Deleted Why dead
tensorrt_llm/quantization/quantize_by_modelopt.py (1,332 lines) Checkpoint-export path for trtllm-build; the PyTorch backend quantizes via ModelOpt directly
tensorrt_llm/quantization/image_processing.py Only consumer was the deleted calibration path
examples/quantization/quantize.py + tests/unittest/others/test_quantize_calib_dataset.py CLI wrapper and test for the deleted export path
tensorrt_llm/_deprecation.py Decorator used only by removed engine-flow APIs
tensorrt_llm/models/unet/pp/__init__.py Zero-byte leftover of the removed unet model
plugin-gen setup.py entries package_data globs for tools/plugin_gen/templates/* and bench/build/benchmark_config.yml — both paths gone
serialization.py whitelist entry "tensorrt_llm.builder": ["BuildConfig"] — the whitelisted module is gone

Plus the import / __init__.py fan-out in tensorrt_llm/quantization/,
llmapi/, models/convert_utils.py, and _utils.py.

2. tensorrt_llm/bench/buildbench/tuning rename (+ lint graduation)

The module stopped building engines long ago; it only computes tuning
heuristics for trtllm-bench. Renamed to say what it is:
build.py → settings.py, tuning.py → heuristics.py, dataclasses.py /
utils.py carried over; importers in bench/benchmark/ repointed. No
public API surface — nothing outside tensorrt_llm.bench imports these.

Per reviewer tburt-nv, the renamed files were graduated out of the
legacy-lint allowlist
: cleaned up to pass the standard ruff rules
(docstrings, import order, quotes, line length) and removed from
legacy-files.txt / ruff-legacy.toml / ruff-legacy-baseline.json.
Formatting-only, no logic change — verified by ruff / ruff-format and
check-configs.

3. Benchmark dataset tooling deduplicated; benchmarks/ folder removed

  • The forked benchmarks/prepare_dataset.py (already
    @click.group(deprecated=True) upstream) and its utils/ are removed.
    The packaged tensorrt_llm/bench/dataset/ behind trtllm-bench prepare-dataset is now the single entry point.
  • Only new option: --stdout (one JSON entry per line) — required
    parity for the in-repo callers migrated off python3 benchmarks/prepare_dataset.py --stdout
    (examples/layer_wise_benchmarks/sample_performance_alignment.sh and the
    multigpu AD smoke test). It is encoded as a null output path in
    RootArgs.output (Optional[str]), so only the group and a four-line
    branch in write_dataset_to_file change — prepare_synthetic_data.py
    and prepare_real_data.py are unchanged vs main.
  • The python3 benchmarks/prepare_dataset.py script is not kept;
    trtllm-bench prepare-dataset is the single supported invocation.
  • All in-repo callers repointed (AD smoke tests,
    sample_performance_alignment.sh,
    tests/unittest/tools/test_prepare_dataset.py).
  • The removed benchmarks/utils/ files were orphaned legacy-engine
    helpers (generate_rand_loras.py wrote the removed .npy LoRA format;
    convert_nemo_dataset.py had no consumers).
  • Docs use trtllm-bench --model <m> prepare-dataset --output <file> ...
    (file mode deliberately — see the stdout-banner note in Follow-ups).

4. Docs / repo guidance refresh

  • docs/source/examples/customization.md rewritten (it imported the
    removed BuildConfig and documented engine building end-to-end).
  • AGENTS.md: backend section trimmed to PyTorch / AutoDeploy; all
    references to the removed TensorRT backend dropped.
  • examples/quantization/README.md: rewritten around the current
    pre-quantized-checkpoint / ModelOpt flow.
  • docs/source/_ext/llmapi_config_telemetry.py no longer emits a
    TrtLlmArgs section; the now-single-element loop was inlined to handle
    TorchLlmArgs directly (per reviewer BowenFu; byte-identical
    generator output verified). telemetry.md regenerated from the current
    committed manifest (see Reviewer resolutions).

5. Legacy-lint bookkeeping (generated-config churn)

All Phase D lint pruning rides in this PR to keep it out of the other
three: stale legacy-files.txt entries for removed files, the regenerated
ruff-legacy.toml / pyproject.toml / .pre-commit-config.yaml managed
blocks, and the stale ruff-legacy-baseline.json keys (_meta totals
updated in the same change). Final baseline: 224 files / 2096 violations,
self-consistent and a strict subset of legacy-files.txt (0 orphans).
legacy_utils.py check-configs passes.

Note on the baseline resolution: the sole rebase conflict was
ruff-legacy-baseline.json. It was rebuilt deterministically from
main's authoritative baseline (drop entries whose file no longer
exists; carry the three bench/build → bench/tuning renames), not
wholesale-regenerated (which drifts across ruff versions). This also
clears the stale entries the merged #16610 left behind, so the
previously-planned separate post-merge prune-files pass is folded in
here and is no longer pending.

Reviewer resolutions

  • BowenFu — telemetry loop: the single-element for args_class in (...)
    loop was inlined; telemetry.md regenerated from the current manifest
    (llm_args_golden_manifest.json already holds only TorchLlmArgs, 269
    fields — untouched here). The checked-in doc was a stale build-time
    snapshot; regenerating also drops main's vestigial 280-field TrtLlmArgs
    table. No manifest change ⇒ no new telemetry/privacy CODEOWNER approval.
  • BowenFu — --tokenizer scope: a --tokenizer option was initially
    added for shim parity, then removed — no in-repo caller uses
    prepare-dataset --tokenizer (they all pass trtllm-bench --model, so
    the model resolves from ctx.obj.model exactly as on main). Deferred to
    a dedicated feature PR if standalone-tokenizer use is ever wanted.
  • tburt-nv — bench/tuning legacy status: graduated out of the
    legacy-lint allowlist (see change group 2).

Reviewer callouts

  • ParallelConfig / executor surface is untouched — C++-side removals
    are in the cpp-cleanup PR (TRTLLM-14475).
  • API-stability: no tests/unittest/api_stability surface is touched;
    the deleted modules were not part of the committed LLM API.
  • llmapi/__init__.py removes a duplicate __all__ entry
    (DeepSeekV4SparseAttentionConfig was listed twice on main); the symbol
    stays imported and exported. (A review tool mislabeled this as adding
    "thinking-budget exports" — those lines are unchanged context.)

Validation

  • python3 -c "import tensorrt_llm" and the attention / bench import
    fan-out verified after each rename / deletion.
  • Full pre-commit over the rebased range passes, incl. ruff,
    ruff-format, the ruff-legacy baseline-gated lint, and the "verify
    legacy lint configs are up to date" hook; check-configs passes.
  • Rebased cleanly onto a19410ebaa: parent is a19410ebaa, zero cpp/
    files in the diff.

Follow-ups (not in this PR)

  • stdout banner: import tensorrt_llm prints a version banner via a
    bare print() to stdout, so prepare-dataset --stdout > file piping
    would prepend the banner. Docs therefore use --output <file> (file
    mode). Moving the banner to stderr is a possible separate [None][infra]
    fix.
  • Not ported from the standalone (no in-repo callers; retrievable from
    git history): local-directory dataset loading and the multimodal image
    dump.
  • --tokenizer standalone support, if ever wanted (see reviewer
    resolutions).

Dev Engineer Review

  • Removes the legacy TensorRT engine backend and obsolete quantization, calibration, plugin-generation, serialization, and deprecation tooling.
  • Renames benchmark tuning modules and consolidates dataset generation under trtllm-bench prepare-dataset, adding --tokenizer and --stdout support.
  • Updates packaging, Ruff/pre-commit scopes, documentation, telemetry generation, and legacy lint baselines.
  • Review focus: verify regenerated path lists contain no invalid or duplicate entries, confirm the renamed benchmark imports and package data are consistent, and ensure tokenizer resolution and stdout/error handling remain API-compatible.
  • ParallelConfig and executor surfaces are intentionally unchanged.

QA Engineer Review

  • Modified tests:
    • tests/unittest/auto_deploy/multigpu/smoke/test_ad_allreduce_strategies.py: updated dataset preparation invocation and working directory.
    • tests/unittest/auto_deploy/singlegpu/smoke/test_ad_trtllm_bench.py: updated dataset preparation working directory.
    • tests/unittest/tools/test_prepare_dataset.py: removed the obsolete script-path constant and updated CLI references.
  • Removed tests:
    • test_is_cnn_dailymail_local_repo
    • test_get_calib_dataloader_local_cnn_dailymail
  • Test-list coverage should be confirmed for the modified smoke and dataset-preparation tests in CI (test-db/) or manual QA (qa/).
  • Verdict: needs follow-up pending coverage verification.

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.

Comment thread docs/source/_ext/llmapi_config_telemetry.py Outdated
"--output", type=str, help="Output json filename.", default="preprocessed_dataset.json"
)
@click.option(
"--stdout",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added tokenizer selection and --stdout output options to trtllm-bench prepare-dataset.
Added support for streaming prepared datasets directly to standard output.
Added thinking-budget processing exports for the LLM API.

Should we move the new features to a follow up PR?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed --tokenizer; after dropping the standalone shim, nothing in-repo invokes prepare-dataset --tokenizer (all callers use trtllm-bench --model), so it was dead.

I kept --stdout because it's required parity, not a new feature: sample_performance_alignment.sh and the multigpu AD smoke test are migrated off python3 benchmarks/prepare_dataset.py --stdout, so removing the standalone requires the packaged CLI to support it.

And re: the "thinking-budget exports" summary line — that's a misread; the actual llmapi/init.py change just removes a duplicate DeepSeekV4SparseAttentionConfig entry in all.

@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

The PR migrates dataset preparation to trtllm-bench, removes legacy benchmark and TensorRT quantization tooling, updates benchmark tuning imports and APIs, refreshes documentation, and regenerates formatting, lint, baseline, and packaging file scopes.

Changes

Benchmark modernization

Layer / File(s) Summary
Dataset CLI and stdout flow
tensorrt_llm/bench/dataset/*, docs/source/commands/*, docs/source/developer-guide/perf-overview.md, examples/layer_wise_benchmarks/*, tests/unittest/auto_deploy/*, tests/unittest/tools/*
prepare-dataset now supports tokenizer selection and JSONL stdout output, with documentation, examples, and tests updated to use the CLI.
Benchmark tuning module alignment
tensorrt_llm/bench/benchmark/*, tensorrt_llm/bench/tuning/*
Benchmark imports move to bench.tuning, with byte-size heuristics, default model mappings, and GPU memory discovery added.
Legacy backend and quantization cleanup
AGENTS.md, tensorrt_llm/llmapi/*, tensorrt_llm/models/*, tensorrt_llm/quantization/*, tensorrt_llm/serialization.py, docs/source/*, examples/quantization/*
Legacy TensorRT references, calibration helpers, quantization exports, image processing, ModelOpt utilities, and related documentation are removed or revised.
Generated lint and legacy file scopes
.pre-commit-config.yaml, legacy-files.txt, pyproject.toml, ruff-legacy-baseline.json, setup.py
Auto-generated formatting, Ruff, baseline, legacy packaging, and package-data scopes are regenerated for the remaining paths.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

Suggested labels: api-breaking

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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 matches the main change and uses the required ticket/type format.
Description check ✅ Passed The description is detailed and structured, covering motivation, changes, validation, and follow-ups.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (3)
tensorrt_llm/bench/dataset/utils.py (2)

105-109: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add type annotations to the modified utility.

Use precise annotations such as Iterable[str], str | Path, and -> None; also document that output_file is ignored when std_out=True. As per coding guidelines, every Python function must be annotated.

🤖 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 `@tensorrt_llm/bench/dataset/utils.py` around lines 105 - 109, Update
write_dataset_to_file with complete type annotations, using Iterable[str] for
dataset_generator, str | Path for output_file, and -> None for the return type.
Add documentation stating that output_file is ignored when std_out=True, while
preserving the existing stdout behavior.

Source: Coding guidelines


105-109: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Cover the new stdout branch with pytest.

Extend tests/unittest/tools/test_prepare_dataset.py to verify JSONL emission with std_out=True, no output-file creation, and the existing file-output path. As per path instructions, keep this QA coverage under tests/unittest/ and use pytest.

🤖 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 `@tensorrt_llm/bench/dataset/utils.py` around lines 105 - 109, Extend the
pytest coverage in test_prepare_dataset.py for write_dataset_to_file: verify
std_out=True emits the generated items as JSONL to stdout without creating an
output file, and retain coverage for the existing file-writing path. Keep the
tests under tests/unittest/ and use pytest conventions.

Source: Path instructions

tensorrt_llm/bench/tuning/utils.py (1)

27-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add the required type annotation and contract documentation.

get_device_memory is a new public helper without a return annotation or docstring. As per coding guidelines, annotate every function and document external interfaces; use def get_device_memory() -> float and describe the GiB result and NVML failure behavior.

🤖 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 `@tensorrt_llm/bench/tuning/utils.py` around lines 27 - 34, Update
get_device_memory with the return annotation -> float and add a docstring
documenting that it returns total device memory in GiB and describing the
behavior when NVML initialization or queries fail.

Source: Coding guidelines

🤖 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 `@AGENTS.md`:
- Around line 58-64: Update the request-flow architecture text in AGENTS.md to
remove TensorRT from the Executor backend list, leaving only PyTorch and
AutoDeploy. Preserve the surrounding request-flow documentation and backend
table unchanged.

In `@docs/source/commands/trtllm-bench.rst`:
- Around line 23-26: Update the click directive’s :commands: list and opening
subcommand count in the trtllm-bench documentation to include prepare-dataset,
keeping the generated CLI command list consistent with the documented
subcommand.

In `@tensorrt_llm/bench/tuning/utils.py`:
- Around line 27-34: Update get_device_memory so the NVML device lookup and
memory read execute inside a try/finally block, ensuring pynvml.nvmlShutdown()
always runs after a successful pynvml.nvmlInit(), including when either NVML
call raises. Preserve the existing total_memory calculation and return behavior.

---

Nitpick comments:
In `@tensorrt_llm/bench/dataset/utils.py`:
- Around line 105-109: Update write_dataset_to_file with complete type
annotations, using Iterable[str] for dataset_generator, str | Path for
output_file, and -> None for the return type. Add documentation stating that
output_file is ignored when std_out=True, while preserving the existing stdout
behavior.
- Around line 105-109: Extend the pytest coverage in test_prepare_dataset.py for
write_dataset_to_file: verify std_out=True emits the generated items as JSONL to
stdout without creating an output file, and retain coverage for the existing
file-writing path. Keep the tests under tests/unittest/ and use pytest
conventions.

In `@tensorrt_llm/bench/tuning/utils.py`:
- Around line 27-34: Update get_device_memory with the return annotation ->
float and add a docstring documenting that it returns total device memory in GiB
and describing the behavior when NVML initialization or queries fail.
🪄 Autofix (Beta)

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: d8f569d0-f54f-4d53-babd-54c165a61e03

📥 Commits

Reviewing files that changed from the base of the PR and between b8604c4 and e823607.

📒 Files selected for processing (48)
  • .pre-commit-config.yaml
  • AGENTS.md
  • benchmarks/README.md
  • benchmarks/prepare_dataset.py
  • benchmarks/utils/__init__.py
  • benchmarks/utils/convert_nemo_dataset.py
  • benchmarks/utils/generate_rand_loras.py
  • benchmarks/utils/prepare_real_data.py
  • benchmarks/utils/prepare_synthetic_data.py
  • benchmarks/utils/utils.py
  • docs/source/_ext/llmapi_config_telemetry.py
  • docs/source/commands/trtllm-bench.rst
  • docs/source/developer-guide/perf-overview.md
  • docs/source/developer-guide/telemetry.md
  • docs/source/examples/customization.md
  • examples/auto_deploy/paragraf/create_standalone_package.py
  • examples/layer_wise_benchmarks/sample_performance_alignment.sh
  • examples/quantization/README.md
  • examples/quantization/quantize.py
  • legacy-files.txt
  • pyproject.toml
  • ruff-legacy-baseline.json
  • ruff-legacy.toml
  • setup.py
  • tensorrt_llm/_deprecation.py
  • tensorrt_llm/bench/benchmark/__init__.py
  • tensorrt_llm/bench/benchmark/utils/general.py
  • tensorrt_llm/bench/build/__init__.py
  • tensorrt_llm/bench/dataset/prepare_dataset.py
  • tensorrt_llm/bench/dataset/prepare_real_data.py
  • tensorrt_llm/bench/dataset/prepare_synthetic_data.py
  • tensorrt_llm/bench/dataset/utils.py
  • tensorrt_llm/bench/tuning/__init__.py
  • tensorrt_llm/bench/tuning/dataclasses.py
  • tensorrt_llm/bench/tuning/heuristics.py
  • tensorrt_llm/bench/tuning/settings.py
  • tensorrt_llm/bench/tuning/utils.py
  • tensorrt_llm/llmapi/__init__.py
  • tensorrt_llm/models/convert_utils.py
  • tensorrt_llm/models/unet/pp/__init__.py
  • tensorrt_llm/quantization/__init__.py
  • tensorrt_llm/quantization/image_processing.py
  • tensorrt_llm/quantization/quantize_by_modelopt.py
  • tensorrt_llm/serialization.py
  • tests/unittest/auto_deploy/multigpu/smoke/test_ad_allreduce_strategies.py
  • tests/unittest/auto_deploy/singlegpu/smoke/test_ad_trtllm_bench.py
  • tests/unittest/others/test_quantize_calib_dataset.py
  • tests/unittest/tools/test_prepare_dataset.py
💤 Files with no reviewable changes (17)
  • benchmarks/prepare_dataset.py
  • benchmarks/README.md
  • benchmarks/utils/convert_nemo_dataset.py
  • setup.py
  • benchmarks/utils/generate_rand_loras.py
  • tests/unittest/others/test_quantize_calib_dataset.py
  • tensorrt_llm/quantization/image_processing.py
  • benchmarks/utils/prepare_synthetic_data.py
  • examples/auto_deploy/paragraf/create_standalone_package.py
  • examples/quantization/quantize.py
  • benchmarks/utils/utils.py
  • benchmarks/utils/prepare_real_data.py
  • tensorrt_llm/llmapi/init.py
  • tensorrt_llm/models/convert_utils.py
  • tensorrt_llm/serialization.py
  • tensorrt_llm/quantization/quantize_by_modelopt.py
  • tensorrt_llm/_deprecation.py

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

Actionable comments posted: 3

🧹 Nitpick comments (3)
tensorrt_llm/bench/dataset/utils.py (2)

105-109: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add type annotations to the modified utility.

Use precise annotations such as Iterable[str], str | Path, and -> None; also document that output_file is ignored when std_out=True. As per coding guidelines, every Python function must be annotated.

🤖 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 `@tensorrt_llm/bench/dataset/utils.py` around lines 105 - 109, Update
write_dataset_to_file with complete type annotations, using Iterable[str] for
dataset_generator, str | Path for output_file, and -> None for the return type.
Add documentation stating that output_file is ignored when std_out=True, while
preserving the existing stdout behavior.

Source: Coding guidelines


105-109: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Cover the new stdout branch with pytest.

Extend tests/unittest/tools/test_prepare_dataset.py to verify JSONL emission with std_out=True, no output-file creation, and the existing file-output path. As per path instructions, keep this QA coverage under tests/unittest/ and use pytest.

🤖 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 `@tensorrt_llm/bench/dataset/utils.py` around lines 105 - 109, Extend the
pytest coverage in test_prepare_dataset.py for write_dataset_to_file: verify
std_out=True emits the generated items as JSONL to stdout without creating an
output file, and retain coverage for the existing file-writing path. Keep the
tests under tests/unittest/ and use pytest conventions.

Source: Path instructions

tensorrt_llm/bench/tuning/utils.py (1)

27-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add the required type annotation and contract documentation.

get_device_memory is a new public helper without a return annotation or docstring. As per coding guidelines, annotate every function and document external interfaces; use def get_device_memory() -> float and describe the GiB result and NVML failure behavior.

🤖 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 `@tensorrt_llm/bench/tuning/utils.py` around lines 27 - 34, Update
get_device_memory with the return annotation -> float and add a docstring
documenting that it returns total device memory in GiB and describing the
behavior when NVML initialization or queries fail.

Source: Coding guidelines

🤖 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 `@AGENTS.md`:
- Around line 58-64: Update the request-flow architecture text in AGENTS.md to
remove TensorRT from the Executor backend list, leaving only PyTorch and
AutoDeploy. Preserve the surrounding request-flow documentation and backend
table unchanged.

In `@docs/source/commands/trtllm-bench.rst`:
- Around line 23-26: Update the click directive’s :commands: list and opening
subcommand count in the trtllm-bench documentation to include prepare-dataset,
keeping the generated CLI command list consistent with the documented
subcommand.

In `@tensorrt_llm/bench/tuning/utils.py`:
- Around line 27-34: Update get_device_memory so the NVML device lookup and
memory read execute inside a try/finally block, ensuring pynvml.nvmlShutdown()
always runs after a successful pynvml.nvmlInit(), including when either NVML
call raises. Preserve the existing total_memory calculation and return behavior.

---

Nitpick comments:
In `@tensorrt_llm/bench/dataset/utils.py`:
- Around line 105-109: Update write_dataset_to_file with complete type
annotations, using Iterable[str] for dataset_generator, str | Path for
output_file, and -> None for the return type. Add documentation stating that
output_file is ignored when std_out=True, while preserving the existing stdout
behavior.
- Around line 105-109: Extend the pytest coverage in test_prepare_dataset.py for
write_dataset_to_file: verify std_out=True emits the generated items as JSONL to
stdout without creating an output file, and retain coverage for the existing
file-writing path. Keep the tests under tests/unittest/ and use pytest
conventions.

In `@tensorrt_llm/bench/tuning/utils.py`:
- Around line 27-34: Update get_device_memory with the return annotation ->
float and add a docstring documenting that it returns total device memory in GiB
and describing the behavior when NVML initialization or queries fail.
🪄 Autofix (Beta)

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: d8f569d0-f54f-4d53-babd-54c165a61e03

📥 Commits

Reviewing files that changed from the base of the PR and between b8604c4 and e823607.

📒 Files selected for processing (48)
  • .pre-commit-config.yaml
  • AGENTS.md
  • benchmarks/README.md
  • benchmarks/prepare_dataset.py
  • benchmarks/utils/__init__.py
  • benchmarks/utils/convert_nemo_dataset.py
  • benchmarks/utils/generate_rand_loras.py
  • benchmarks/utils/prepare_real_data.py
  • benchmarks/utils/prepare_synthetic_data.py
  • benchmarks/utils/utils.py
  • docs/source/_ext/llmapi_config_telemetry.py
  • docs/source/commands/trtllm-bench.rst
  • docs/source/developer-guide/perf-overview.md
  • docs/source/developer-guide/telemetry.md
  • docs/source/examples/customization.md
  • examples/auto_deploy/paragraf/create_standalone_package.py
  • examples/layer_wise_benchmarks/sample_performance_alignment.sh
  • examples/quantization/README.md
  • examples/quantization/quantize.py
  • legacy-files.txt
  • pyproject.toml
  • ruff-legacy-baseline.json
  • ruff-legacy.toml
  • setup.py
  • tensorrt_llm/_deprecation.py
  • tensorrt_llm/bench/benchmark/__init__.py
  • tensorrt_llm/bench/benchmark/utils/general.py
  • tensorrt_llm/bench/build/__init__.py
  • tensorrt_llm/bench/dataset/prepare_dataset.py
  • tensorrt_llm/bench/dataset/prepare_real_data.py
  • tensorrt_llm/bench/dataset/prepare_synthetic_data.py
  • tensorrt_llm/bench/dataset/utils.py
  • tensorrt_llm/bench/tuning/__init__.py
  • tensorrt_llm/bench/tuning/dataclasses.py
  • tensorrt_llm/bench/tuning/heuristics.py
  • tensorrt_llm/bench/tuning/settings.py
  • tensorrt_llm/bench/tuning/utils.py
  • tensorrt_llm/llmapi/__init__.py
  • tensorrt_llm/models/convert_utils.py
  • tensorrt_llm/models/unet/pp/__init__.py
  • tensorrt_llm/quantization/__init__.py
  • tensorrt_llm/quantization/image_processing.py
  • tensorrt_llm/quantization/quantize_by_modelopt.py
  • tensorrt_llm/serialization.py
  • tests/unittest/auto_deploy/multigpu/smoke/test_ad_allreduce_strategies.py
  • tests/unittest/auto_deploy/singlegpu/smoke/test_ad_trtllm_bench.py
  • tests/unittest/others/test_quantize_calib_dataset.py
  • tests/unittest/tools/test_prepare_dataset.py
💤 Files with no reviewable changes (17)
  • benchmarks/prepare_dataset.py
  • benchmarks/README.md
  • benchmarks/utils/convert_nemo_dataset.py
  • setup.py
  • benchmarks/utils/generate_rand_loras.py
  • tests/unittest/others/test_quantize_calib_dataset.py
  • tensorrt_llm/quantization/image_processing.py
  • benchmarks/utils/prepare_synthetic_data.py
  • examples/auto_deploy/paragraf/create_standalone_package.py
  • examples/quantization/quantize.py
  • benchmarks/utils/utils.py
  • benchmarks/utils/prepare_real_data.py
  • tensorrt_llm/llmapi/init.py
  • tensorrt_llm/models/convert_utils.py
  • tensorrt_llm/serialization.py
  • tensorrt_llm/quantization/quantize_by_modelopt.py
  • tensorrt_llm/_deprecation.py
🛑 Comments failed to post (3)
AGENTS.md (1)

58-64: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Remove the stale TensorRT backend from the request-flow documentation.

The new backend table says only PyTorch and AutoDeploy remain, but Line 70 still says Executor (PyTorch/AutoDeploy/TensorRT). Change it to Executor (PyTorch/AutoDeploy) so the architecture guidance is consistent.

🤖 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 `@AGENTS.md` around lines 58 - 64, Update the request-flow architecture text in
AGENTS.md to remove TensorRT from the Executor backend list, leaving only
PyTorch and AutoDeploy. Preserve the surrounding request-flow documentation and
backend table unchanged.
docs/source/commands/trtllm-bench.rst (1)

23-26: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Include prepare-dataset in the generated command list.

The click directive at Line 16 still lists only throughput, latency, build, so the new command is absent from generated CLI syntax. Update :commands: and the opening subcommand count to include prepare-dataset.

🤖 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 `@docs/source/commands/trtllm-bench.rst` around lines 23 - 26, Update the click
directive’s :commands: list and opening subcommand count in the trtllm-bench
documentation to include prepare-dataset, keeping the generated CLI command list
consistent with the documented subcommand.
tensorrt_llm/bench/tuning/utils.py (1)

27-34: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

git ls-files tensorrt_llm/bench/tuning/utils.py
wc -l tensorrt_llm/bench/tuning/utils.py
cat -n tensorrt_llm/bench/tuning/utils.py | sed -n '1,120p'
rg -n "get_device_memory|nvmlInit|nvmlShutdown|pynvml" tensorrt_llm/bench -S

Repository: NVIDIA/TensorRT-LLM

Length of output: 3132


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '280,330p' tensorrt_llm/bench/dataclasses/reporting.py
sed -n '1,140p' tensorrt_llm/bench/tuning/heuristics.py
python3 - <<'PY'
from pathlib import Path
text = Path("tensorrt_llm/bench/tuning/utils.py").read_text()
print("try/finally present:", "try:" in text and "finally:" in text)
print("nvmlShutdown count:", text.count("nvmlShutdown"))
PY

Repository: NVIDIA/TensorRT-LLM

Length of output: 8583


Guard NVML teardown in get_device_memory()
If nvmlDeviceGetHandleByIndex() or nvmlDeviceGetMemoryInfo() raises after nvmlInit(), nvmlShutdown() is skipped and NVML stays initialized. Wrap the lookup/read in try/finally.

🤖 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 `@tensorrt_llm/bench/tuning/utils.py` around lines 27 - 34, Update
get_device_memory so the NVML device lookup and memory read execute inside a
try/finally block, ensuring pynvml.nvmlShutdown() always runs after a successful
pynvml.nvmlInit(), including when either NVML call raises. Preserve the existing
total_memory calculation and return behavior.

@mikeiovine
mikeiovine requested review from mikeiovine and removed request for pcastonguay and schetlur-nv July 20, 2026 15:24
Comment thread ruff-legacy.toml Outdated
…ter the backend removal

Trailing python-side cleanup after the legacy TensorRT backend removal
(TRTLLM-14026, PR NVIDIA#16369):

- Remove legacy quantization-export and plugin-gen relics, the
  engine-workflow serialization/deprecation shims, and other dead
  python modules (with their setup.py package_data entries).
- Rename tensorrt_llm/bench/build to bench/tuning
  (build.py -> settings.py, tuning.py -> heuristics.py); the module
  no longer builds anything. The renamed files are also cleaned up to
  pass the standard ruff rules (docstrings, imports, quotes, line
  length) and removed from the legacy-lint allowlist.
- Remove the top-level benchmarks/ folder entirely.
  benchmarks/prepare_dataset.py (already marked deprecated
  upstream) is deduplicated into the packaged
  tensorrt_llm/bench/dataset implementation behind
  trtllm-bench prepare-dataset, which gains --tokenizer and
  --stdout root options as a full replacement; all in-repo callers
  and docs are repointed. The remaining utils were orphaned
  legacy-engine helpers (generate_rand_loras.py wrote the removed
  .npy LoRA engine format; convert_nemo_dataset.py had no
  consumers).
- Refresh docs and repo guidance to the current PyTorch/AutoDeploy
  state (customization.md rewrite, AGENTS.md backend section,
  examples/quantization/README.md, telemetry doc regeneration).
- Prune stale legacy-lint entries (removed files) from
  legacy-files.txt, the generated ruff configs, and
  ruff-legacy-baseline.json.

Signed-off-by: Wanli Jiang <35160485+Wanli-Jiang@users.noreply.github.com>
@Wanli-Jiang
Wanli-Jiang force-pushed the user/williamj/trtllm-14027-python-cleanup branch from e823607 to 9843df9 Compare July 23, 2026 03:48
@Wanli-Jiang

Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 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 `@docs/source/developer-guide/perf-overview.md`:
- Line 290: Update the relevant documentation wording from “In order to prepare
a dataset” to “To prepare a dataset,” preserving the surrounding meaning and
content.

In `@docs/source/examples/customization.md`:
- Line 5: Add the repository-standard NVIDIA copyright header with the current
copyright year at the very top of customization.md, before the existing document
title and content. Preserve all existing documentation unchanged.
- Line 17: Update the sentence describing SamplingParams to hyphenate the
compound adjective as “LLM-generated responses,” without changing the
surrounding wording or meaning.
🪄 Autofix (Beta)

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: 63b1b307-9955-4f6b-a25f-2d4a3a75b861

📥 Commits

Reviewing files that changed from the base of the PR and between e823607 and 9843df9.

📒 Files selected for processing (25)
  • .pre-commit-config.yaml
  • AGENTS.md
  • benchmarks/README.md
  • benchmarks/prepare_dataset.py
  • benchmarks/utils/__init__.py
  • benchmarks/utils/convert_nemo_dataset.py
  • benchmarks/utils/generate_rand_loras.py
  • benchmarks/utils/prepare_real_data.py
  • benchmarks/utils/prepare_synthetic_data.py
  • benchmarks/utils/utils.py
  • docs/source/_ext/llmapi_config_telemetry.py
  • docs/source/commands/trtllm-bench.rst
  • docs/source/developer-guide/perf-overview.md
  • docs/source/developer-guide/telemetry.md
  • docs/source/examples/customization.md
  • examples/auto_deploy/paragraf/create_standalone_package.py
  • examples/layer_wise_benchmarks/sample_performance_alignment.sh
  • examples/quantization/README.md
  • examples/quantization/quantize.py
  • legacy-files.txt
  • pyproject.toml
  • ruff-legacy-baseline.json
  • ruff-legacy.toml
  • setup.py
  • tensorrt_llm/_deprecation.py
💤 Files with no reviewable changes (15)
  • setup.py
  • benchmarks/utils/generate_rand_loras.py
  • benchmarks/utils/convert_nemo_dataset.py
  • tensorrt_llm/_deprecation.py
  • examples/quantization/quantize.py
  • examples/auto_deploy/paragraf/create_standalone_package.py
  • benchmarks/prepare_dataset.py
  • benchmarks/utils/prepare_synthetic_data.py
  • benchmarks/README.md
  • benchmarks/utils/prepare_real_data.py
  • benchmarks/utils/utils.py
  • ruff-legacy.toml
  • pyproject.toml
  • .pre-commit-config.yaml
  • legacy-files.txt
🚧 Files skipped from review as they are similar to previous changes (4)
  • docs/source/_ext/llmapi_config_telemetry.py
  • examples/layer_wise_benchmarks/sample_performance_alignment.sh
  • docs/source/commands/trtllm-bench.rst
  • AGENTS.md

Comment thread docs/source/developer-guide/perf-overview.md
Comment thread docs/source/examples/customization.md
Comment thread docs/source/examples/customization.md
@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #61189 [ run ] triggered by Bot. Commit: 9843df9 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #61189 [ run ] completed with state FAILURE. Commit: 9843df9
/LLM/main/L0_MergeRequest_PR pipeline #49438 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

@Wanli-Jiang

Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #61285 [ run ] triggered by Bot. Commit: 9843df9 Link to invocation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants