[None][doc] Update autodeploy README.md, deprecate lm_eval in examples folder #7233
Conversation
📝 WalkthroughWalkthroughRewrites Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
Fridah-nv
changed the title
Fridah-nv
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
tensorrt_llm/_torch/auto_deploy/transform/library/kvcache.py (1)
235-237: Optional: Use MiB for accuracy or show both MB and raw bytes.You’re doing binary conversion (1024**2), which is MiB. If you want precision, relabel to MiB or include both MiB and raw bytes.
Possible tweak:
- ad_logger.info( - f"Current cache size (MB): {current_cache_size // (1024 ** 2)}, " - f"Current num pages: {current_num_pages}" - ) + ad_logger.info( + f"Current cache size (MiB): {current_cache_size // (1024 ** 2)} " + f"({current_cache_size} bytes), Current num pages: {current_num_pages}" + )examples/auto_deploy/README.md (4)
30-33: Tighten script description wording.“Arbitrarily configure” reads awkwardly. Prefer “flexibly configure” or similar.
Apply this diff:
-This script demonstrates end-to-end deployment of Hugging Face (or local) checkpoints using AutoDeploy’s graph-transformation pipeline. +This script demonstrates end-to-end deployment of Hugging Face (or local) checkpoints using AutoDeploy’s graph‑transformation pipeline. @@ -You can arbitrarily configure your experiment. Use the `-h/--help` flag to see available options: +You can flexibly configure your experiment. Use the `-h/--help` flag to see available options:
75-83: Polish bullets; small grammar/style nits.Minor tweaks for noun consistency and punctuation.
Apply this diff:
-### Advanced Configuration +### Advanced Configuration @@ -The script supports flexible configs: +The script supports flexible configuration: @@ -- CLI dot notation for nested fields -- YAML configs with deep merge -- Precedence: CLI > YAML > defaults +- CLI dot notation for nested fields +- YAML configuration files with deep merge +- Precedence: CLI > YAML > defaults
85-95: Clarify tool name and tighten phrasing; add code formatting.Capitalize and format the tool name; slightly streamline the lead-in sentence.
Apply this diff:
-## Model Evaluation with LM Evaluation Harness +## Model Evaluation with LM Evaluation Harness @@ -lm-evaluation-harness is supported. To run the evaluation, please use the following command: +`lm-evaluation-harness` is supported. To run an evaluation, use:Optionally, consider adding a link to the project for convenience: https://github.com/EleutherAI/lm-evaluation-harness
93-95: Optional: Note GPU requirement for world_size > 1 to preempt confusion.The example uses
world_size=2. A brief note that this requires at least two visible GPUs would reduce user friction.Proposed addition right above the code block:
+Note: Setting `world_size > 1` requires the corresponding number of available GPUs.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
examples/auto_deploy/README.md(4 hunks)tensorrt_llm/_torch/auto_deploy/transform/library/kvcache.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (CODING_GUIDELINES.md)
**/*.py: Code must target Python 3.8+
Indent Python code with 4 spaces; do not use tabs
Preserve module namespaces when importing; import modules/packages and access members via the module (e.g., from package.subpackage import foo; foo.SomeClass())
Python file names should be snake_case
Python class names should be PascalCase
Python functions/methods and local variables should be snake_case; variables beginning with a number should be prefixed with k_ (e.g., k_99th_percentile)
Global variables should be UPPER_SNAKE_CASE prefixed with G_ (e.g., G_MY_GLOBAL); constants should be UPPER_SNAKE_CASE
Avoid shadowing variables from outer scopes; initialize all externally visible members in init
Prefer docstrings for interfaces used outside a file; comments should be reserved for in-function or file-local interfaces
Use Google-style docstrings for classes and functions; attributes and variables may be documented inline with trailing string literals
Avoid reflection when simpler, explicit code suffices (e.g., avoid dict(**locals()) patterns)
In try/except, catch the narrowest exceptions possible
For duck-typing patterns, keep the try body minimal and move logic to else to avoid masking unrelated failures
Files:
tensorrt_llm/_torch/auto_deploy/transform/library/kvcache.py
**/*.{c,cc,cpp,cxx,h,hh,hpp,hxx,cu,cuh,py}
📄 CodeRabbit inference engine (CODING_GUIDELINES.md)
Prepend the NVIDIA copyright header (current year) to all source files (.cpp, .h, .cu, .py, etc.)
Files:
tensorrt_llm/_torch/auto_deploy/transform/library/kvcache.py
🪛 LanguageTool
examples/auto_deploy/README.md
[grammar] ~3-~3: There might be a mistake here.
Context: ...n="center"> # 🔥🚀⚡ AutoDeploy Examples
(QB_NEW_EN)
[grammar] ~79-~79: There might be a mistake here.
Context: ...s: - CLI dot notation for nested fields - YAML configs with deep merge - Precedenc...
(QB_NEW_EN)
[grammar] ~80-~80: There might be a mistake here.
Context: ...ed fields - YAML configs with deep merge - Precedence: CLI > YAML > defaults Pleas...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Pre-commit Check
🔇 Additional comments (4)
examples/auto_deploy/README.md (4)
3-3: LGTM on the concise, example-focused title.
11-11: LGTM: “Quick Start” section improves discoverability.
21-26: LGTM: Minimal runnable example is clear and actionable.
60-73: LGTM: The longer example is coherent and consistent with the options table.
|
Hi @suyoggupta , Please also comment on the status of |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
examples/auto_deploy/README.md (1)
7-7: Fix comma splice and clarify phrasing in the lead sentence.Same issue already flagged by another reviewer; agree with the proposed rewrite.
Apply this diff:
-This folder contains runnable examples for **AutoDeploy**, for general AutoDeploy documentation, motivation, support matrix and feature overview, please check [official docs](https://nvidia.github.io/TensorRT-LLM/torch/auto_deploy/auto-deploy.html). +This folder contains runnable examples for **AutoDeploy**. For general AutoDeploy documentation, motivation, support matrix, and feature overview, please see the [official docs](https://nvidia.github.io/TensorRT-LLM/torch/auto_deploy/auto-deploy.html).
🧹 Nitpick comments (9)
examples/auto_deploy/README.md (9)
1-6: Remove or properly close HTML alignment divs; prefer plain Markdown for portability.The nested/unclosed
wrappers can render inconsistently on GitHub and other viewers. The simplest fix is to drop them and keep pure Markdown.Apply this diff:
-<div align="center"> - -# 🔥🚀⚡ AutoDeploy Examples - -<div align="left"> +# 🔥🚀⚡ AutoDeploy Examples
9-9: Use standard Markdown horizontal rules for consistency.Long underscore lines can wrap or look noisy. Prefer Markdown HR.
Apply this diff:
-______________________________________________________________________ +---
11-17: Qualify platform-specific install step and reduce surprise for non-Ubuntu users.The apt-get line is Debian/Ubuntu-specific. Add a qualifier to set expectations and keep the quick start broadly applicable.
Apply this diff:
-## Quick Start - -AutoDeploy is included with the TRT-LLM installation. +## Quick Start + +AutoDeploy is included with the TRT-LLM installation. @@ -```bash -sudo apt-get -y install libopenmpi-dev && pip3 install --upgrade pip setuptools && pip3 install tensorrt_llm -``` +```bash +# Debian/Ubuntu example: +sudo apt-get -y install libopenmpi-dev +python3 -m pip install --upgrade pip setuptools +python3 -m pip install tensorrt_llm +```
28-28: Use standard Markdown horizontal rules for consistency.Mirror the earlier HR change here as well.
Apply this diff:
-______________________________________________________________________ +---
32-33: Unify “Hugging Face” branding.Use “Hugging Face” (two words) consistently.
Apply this diff:
-This script demonstrates end-to-end deployment of HuggingFace checkpoints using AutoDeploy’s graph-transformation pipeline. +This script demonstrates end-to-end deployment of Hugging Face checkpoints using AutoDeploy’s graph-transformation pipeline.
58-58: Normalize boolean notation with the example invocation.The table says “true/false” but the example uses “True”. Align them for consistency.
Apply this diff:
-| `--benchmark.enabled` | Whether to run the built-in benchmark (true/false) | +| `--benchmark.enabled` | Whether to run the built-in benchmark (True/False) |
60-60: Avoid redundant linking and improve clarity.Linking both the class name and the file to the same target is noisy. Keep one link and use code formatting for the class.
Apply this diff:
-For default values and additional configuration options, refer to the [`ExperimentConfig`](./build_and_run_ad.py) class in [build_and_run_ad.py](./build_and_run_ad.py) file. +For default values and additional configuration options, see the `ExperimentConfig` class in [build_and_run_ad.py](./build_and_run_ad.py).
75-83: Tighten phrasing in Advanced Configuration bullets.Minor grammar/style polish.
Apply this diff:
-### Advanced Configuration - -The script supports flexible configs: - -- CLI dot notation for nested fields -- YAML configs with deep merge -- Precedence: CLI > YAML > defaults +### Advanced configuration + +The script supports flexible configuration: + +- CLI dot notation for nested fields +- YAML configuration files with deep merge +- Precedence: CLI > YAML > defaults
85-95: Add prerequisite note for LM Evaluation Harness.Call out that users need the harness installed and where to find its docs.
Apply this diff:
-## Model Evaluation with LM Evaluation Harness +## Model evaluation with LM Evaluation Harness @@ -lm-evaluation-harness is supported. To run the evaluation, please use the following command: +lm-evaluation-harness is supported. Ensure the harness is installed (see its documentation) before running: + +• Project: https://github.com/EleutherAI/lm-evaluation-harness @@ python lm_eval_ad.py \ --model autodeploy --model_args model=meta-llama/Meta-Llama-3.1-8B-Instruct,world_size=2 --tasks mmlu</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: Path: .coderabbit.yaml **Review profile**: CHILL **Plan**: Pro **💡 Knowledge Base configuration:** - MCP integration is disabled by default for public repositories - Jira integration is disabled by default for public repositories - Linear integration is disabled by default for public repositories You can enable these sources in your CodeRabbit configuration. <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 053a075a7408ea887178eca3aa50c0a9cfe3b863 and ce373649507cc4067092241b55c508e61f859b00. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `examples/auto_deploy/README.md` (5 hunks) * `tensorrt_llm/_torch/auto_deploy/transform/library/kvcache.py` (1 hunks) </details> <details> <summary>✅ Files skipped from review due to trivial changes (1)</summary> * tensorrt_llm/_torch/auto_deploy/transform/library/kvcache.py </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 LanguageTool</summary> <details> <summary>examples/auto_deploy/README.md</summary> [grammar] ~3-~3: There might be a mistake here. Context: ...n="center"> # 🔥🚀⚡ AutoDeploy Examples <div align="left"> This folder contains runnable examples for... (QB_NEW_EN) --- [grammar] ~79-~79: There might be a mistake here. Context: ...s: - CLI dot notation for nested fields - YAML configs with deep merge - Precedenc... (QB_NEW_EN) --- [grammar] ~80-~80: There might be a mistake here. Context: ...ed fields - YAML configs with deep merge - Precedence: CLI > YAML > defaults Pleas... (QB_NEW_EN) </details> </details> </details> <details> <summary>⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)</summary> * GitHub Check: Pre-commit Check </details> <details> <summary>🔇 Additional comments (3)</summary><blockquote> <details> <summary>examples/auto_deploy/README.md (3)</summary><blockquote> `21-26`: **LGTM: Simple runnable example is clear and accessible.** The TinyLlama example gives users a quick success path. --- `97-99`: **LGTM: Clear, honest disclaimer aligns with main docs.** Wording and tone look good. --- `40-60`: **Ensure README flag table matches the generated CLI** The CLI options in examples/auto_deploy/README.md are auto-generated via ExperimentConfig/CliApp, not via explicit add_argument calls. To keep the docs in sync, please run:python -m examples.auto_deploy.build_and_run_ad --help
(or `python examples/auto_deploy/build_and_run_ad.py --help`) and verify that every `--foo` flag (and its default) shown in the help output appears in the table, and vice versa. Update the table entries or defaults in the README accordingly. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
|
Yes I think we can deprecate lm_eval_ad |
Fridah-nv
force-pushed
the
user/fridah/update-readme
branch
from
ce37364 to
89be3a6
Compare
Fridah-nv
changed the title
|
/bot skip --comment "doc, logging change and file deprecation only" |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
examples/auto_deploy/README.md (8)
11-14: Add environment prerequisites and a container-based path; avoid implying pip-only is sufficient.AutoDeploy often requires CUDA-, PyTorch-, and driver-aligned environments. Give readers two clear choices and call out prerequisites. Also, per our prior learning, reference only published NGC container tags in docs.
Proposed additions under Quick Start, after the existing pip snippet:
AutoDeploy is included with the TRT-LLM installation. ```bash sudo apt-get -y install libopenmpi-dev && pip3 install --upgrade pip setuptools && pip3 install tensorrt_llmYou can refer to TRT-LLM installation guide for more information.
+Note: Ensure your CUDA driver matches the CUDA/PyTorch stack used by TensorRT-LLM. If you prefer a prebuilt environment, use the published NGC container:
+
+```bash
+# Replace with an actual, published NGC tag.
+docker run --gpus all --rm -it \
- -v $PWD:/workspace/TensorRT-LLM \
- nvcr.io/nvidia/tensorrt-llm: \
- bash
+```+Only reference published NGC tags in docs to avoid confusion with unreleased images.
If you want, I can suggest a specific currently published tag after verifying what’s live on NGC. <!-- retaining guidance from retrieved learnings --> Also applies to: 19-19 --- `21-26`: **Minor: clarify that the first run downloads the model and may take time; optionally show a TRT-LLM runtime example.** This reduces surprise for users in restricted networks and nudges them toward a GPU-backed runtime. ```diff -Run a simple example with a Hugging Face model: +Run a simple example with a Hugging Face model (the first run will download weights and may take several minutes):Optionally add a second example line showing
--args.runtime "trtllm"for a GPU-backed run (if supported by default on the target environment).
30-36: Nit: brand consistency — use “Hugging Face” (with space).Keep terminology consistent across the README.
-This script demonstrates end-to-end deployment of HuggingFace checkpoints using AutoDeploy’s graph-transformation pipeline. +This script demonstrates end-to-end deployment of Hugging Face checkpoints using AutoDeploy’s graph-transformation pipeline.
40-59: Document accepted enum values/defaults, or generate this table from argparse to avoid drift.Options like
--args.runtime(“demollm”, “trtllm”),--args.compile-backend, and--args.attn-backendbenefit from listing allowed values and defaults. Manually maintained tables tend to drift from code.Two options:
- Short-term: add “Accepted values” and “Default” columns for the enum-like flags.
- Longer-term: generate this section from the script’s argparse (e.g.,
--helpoutput) during docs build to keep it in sync.If you’d like, I can draft a small script to parse
build_and_run_ad.py’s argparse and emit a Markdown table for these flags.
60-60: Avoid duplicate links in the same sentence.Currently both “ExperimentConfig” and “build_and_run_ad.py” link to the same file. Keep a single link for clarity.
-For default values and additional configuration options, refer to the [`ExperimentConfig`](./build_and_run_ad.py) class in [build_and_run_ad.py](./build_and_run_ad.py) file. +For default values and additional configuration options, refer to the [`ExperimentConfig`](./build_and_run_ad.py) class in the build_and_run_ad.py file.
75-81: Bullet style and micro-grammar consistency.End each bullet with a period for consistent style; keep “Precedence:” as-is.
- - CLI dot notation for nested fields - - YAML configs with deep merge + - CLI dot notation for nested fields. + - YAML configs with deep merge. - Precedence: CLI > YAML > defaults
85-88: Add an explicit deprecation/migration note for lm_eval_ad.py (per thread consensus).Comment thread confirms deprecating
lm_eval_ad.py. Add a short note here (or above Quick Start) pointing users to the new LLM API accuracy test path to prevent confusion.Proposed insertion just above “Disclaimer”:
+## Deprecation Notice + +The AutoDeploy LM evaluation helper (`lm_eval_ad.py`) has been deprecated in favor of the LLM API accuracy test integration. Please use the LLM API accuracy workflow for future evaluations. +For details and the latest instructions, refer to the LLM API accuracy test documentation. + ## DisclaimerIf you share the canonical doc link for the LLM API accuracy workflow, I’ll wire it in here.
7-7: Documentation links confirmed — consider versioning for durabilityAll referenced URLs in
examples/auto_deploy/README.mdreturn HTTP 200 with no redirects:
- https://nvidia.github.io/TensorRT-LLM/torch/auto_deploy/auto-deploy.html
- https://github.com/NVIDIA/TensorRT-LLM/blob/main/docs/source/installation/linux.md
- https://nvidia.github.io/TensorRT-LLM/torch/auto_deploy/advanced/expert_configurations.html
These links currently point at the main branch, which may shift as the docs evolve. To ensure users always see the intended content matching your release train, you may elect to swap these for stable or versioned paths (e.g. replacing
mainwith a specific tag or using a/stable/directory if supported).• Applies to lines 7, 19, and 83 in
examples/auto_deploy/README.md.
• No immediate failures detected, but updating to versioned URLs is recommended to prevent future link rot.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
examples/auto_deploy/README.md(4 hunks)examples/auto_deploy/lm_eval_ad.py(0 hunks)tensorrt_llm/_torch/auto_deploy/transform/library/kvcache.py(1 hunks)tests/unittest/_torch/auto_deploy/integration/test_lm_eval.py(0 hunks)
💤 Files with no reviewable changes (2)
- tests/unittest/_torch/auto_deploy/integration/test_lm_eval.py
- examples/auto_deploy/lm_eval_ad.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tensorrt_llm/_torch/auto_deploy/transform/library/kvcache.py
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-21T00:16:56.457Z
Learnt from: farshadghodsian
PR: NVIDIA/TensorRT-LLM#7101
File: docs/source/blogs/tech_blog/blog9_Deploying_GPT_OSS_on_TRTLLM.md:36-36
Timestamp: 2025-08-21T00:16:56.457Z
Learning: TensorRT-LLM container release tags in documentation should only reference published NGC container images. The README badge version may be ahead of the actual published container versions.
Applied to files:
examples/auto_deploy/README.md
📚 Learning: 2025-08-01T15:14:45.673Z
Learnt from: yibinl-nvidia
PR: NVIDIA/TensorRT-LLM#6506
File: examples/models/core/mixtral/requirements.txt:3-3
Timestamp: 2025-08-01T15:14:45.673Z
Learning: In TensorRT-LLM, examples directory can have different dependency versions than the root requirements.txt file. Version conflicts between root and examples dependencies are acceptable because examples are designed to be standalone and self-contained.
Applied to files:
examples/auto_deploy/README.md
🪛 LanguageTool
examples/auto_deploy/README.md
[grammar] ~3-~3: There might be a mistake here.
Context: ...n="center"> # 🔥🚀⚡ AutoDeploy Examples
(QB_NEW_EN)
[grammar] ~79-~79: There might be a mistake here.
Context: ...s: - CLI dot notation for nested fields - YAML configs with deep merge - Precedenc...
(QB_NEW_EN)
[grammar] ~80-~80: There might be a mistake here.
Context: ...ed fields - YAML configs with deep merge - Precedence: CLI > YAML > defaults Pleas...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Pre-commit Check
🔇 Additional comments (3)
examples/auto_deploy/README.md (3)
7-7: Lead sentence reads well now.The comma splice is resolved; phrasing and Oxford comma look good.
62-73: Example looks solid and runnable.Nice balance of brevity and realism; good choice of a small HF model for quick validation.
30-73: Overall restructure improves clarity and task focus.The concise Quick Start + focused script-centric guidance is a welcome change and lowers time-to-first-success.
…update to resize_kvcache logging Signed-off-by: Frida Hou <[email protected]> minor: rephrase README, fix kvcache num_pages info Signed-off-by: Frida Hou <[email protected]> minor Signed-off-by: Frida Hou <[email protected]> minor Signed-off-by: Frida Hou <[email protected]>
Signed-off-by: Frida Hou <[email protected]>
Fridah-nv
force-pushed
the
user/fridah/update-readme
branch
from
89be3a6 to
d52f473
Compare
|
PR_Github #16576 [ skip ] triggered by Bot |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
examples/auto_deploy/README.md (8)
7-10: Add brief prerequisites note before install snippet.
Readers may try this on machines without CUDA/NCCL-compatible GPUs or the correct driver/container and hit errors. A one-line prerequisite note will reduce support churn.Apply this diff:
## Quick Start -AutoDeploy is included with the TRT-LLM installation. +AutoDeploy is included with the TRT-LLM installation. + +Prerequisites: a CUDA-capable NVIDIA GPU, compatible NVIDIA driver, and CUDA/NCCL runtime (preferably via an NGC container or the environment specified in the TRT‑LLM installation guide).
3-3: Consider adding a deprecation callout for lm_eval_ad.py removal.
Given the concurrent deprecation, a short note here will prevent confusion and re-directed issues.Apply this diff:
-This folder contains runnable examples for **AutoDeploy**. For general AutoDeploy documentation, motivation, support matrix, and feature overview, please see the [official docs](https://nvidia.github.io/TensorRT-LLM/torch/auto_deploy/auto-deploy.html). +This folder contains runnable examples for **AutoDeploy**. For general AutoDeploy documentation, motivation, support matrix, and feature overview, please see the [official docs](https://nvidia.github.io/TensorRT-LLM/torch/auto_deploy/auto-deploy.html). + +> Note: The AutoDeploy LM evaluation example (`lm_eval_ad.py`) has been deprecated. For accuracy testing, use the LLM API accuracy tests documented in the official docs.If you’d like, I can add a short “Deprecations” section at the end with links to the replacement flow.
17-22: Small clarity tweak: mention model download and first-run latency.
Users on shared clusters often wonder why the first run is slow; calling out model download helps.Apply this diff:
-Run a simple example with a Hugging Face model: +Run a simple example with a Hugging Face model (the checkpoint will be downloaded on first run):
26-31: Consistency: “Hugging Face” vs “HuggingFace”.
The doc uses both spellings. Prefer “Hugging Face” (two words) everywhere.Apply this diff:
-This script demonstrates end-to-end deployment of HuggingFace checkpoints using AutoDeploy’s graph-transformation pipeline. +This script demonstrates end-to-end deployment of Hugging Face checkpoints using AutoDeploy’s graph-transformation pipeline.
36-56: Polish option descriptions and boolean casing; ensure option names align with the script.
- The row “Whether to run the built-in benchmark (true/false)” uses lowercase booleans, while the example uses
True. Pick one style or note both.- If the CLI uses argparse with
type=bool/action='store_true', consider documenting accepted forms (e.g.,--benchmark.enabledvs--benchmark.enabled True).- Optional: clarify units for
--args.attn-page-size(tokens? pages?) and semantics of--args.max-batch-size(tokens, sequences, or requests).Proposed edits:
-| `--benchmark.enabled` | Whether to run the built-in benchmark (true/false) | +| `--benchmark.enabled` | Whether to run the built-in benchmark (`true`/`false` or `--benchmark.enabled` if using a flag) |Additionally, please verify that all option names exactly match
build_and_run_ad.pyto avoid drift. If helpful, I can script-check the argparse definitions against this table.
56-56: Link “ExperimentConfig” directly or clarify how to find it.
The current link points to the file, not to the symbol. Either deep-link to a line anchor or add a short hint (search within file).Apply this diff:
-For default values and additional configuration options, refer to the [`ExperimentConfig`](./build_and_run_ad.py) class in [build_and_run_ad.py](./build_and_run_ad.py) file. +For default values and additional configuration options, see the `ExperimentConfig` class in [build_and_run_ad.py](./build_and_run_ad.py) (search within the file for “class ExperimentConfig”).
58-69: Call out GPU count requirement in the example using--args.world-size 2.
Avoids confusion for single-GPU users copying the example.Apply this diff:
-The following is a more complete example of using the script: +The following is a more complete example of using the script (requires at least 2 GPUs because `--args.world-size 2`):
71-79: Minor grammar/style pass in “Advanced Configuration”.
Tighten phrasing and standardize list capitalization.Apply this diff:
-### Advanced Configuration - -The script supports flexible configs: - -- CLI dot notation for nested fields -- YAML configs with deep merge -- Precedence: CLI > YAML > defaults +### Advanced Configuration + +The script supports flexible configuration methods: + +- CLI dot notation for nested fields. +- YAML configs with deep merge. +- Precedence: CLI > YAML > defaults.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
examples/auto_deploy/README.md(4 hunks)examples/auto_deploy/lm_eval_ad.py(0 hunks)tensorrt_llm/_torch/auto_deploy/transform/library/kvcache.py(1 hunks)tests/unittest/_torch/auto_deploy/integration/test_lm_eval.py(0 hunks)
💤 Files with no reviewable changes (2)
- examples/auto_deploy/lm_eval_ad.py
- tests/unittest/_torch/auto_deploy/integration/test_lm_eval.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tensorrt_llm/_torch/auto_deploy/transform/library/kvcache.py
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-21T00:16:56.457Z
Learnt from: farshadghodsian
PR: NVIDIA/TensorRT-LLM#7101
File: docs/source/blogs/tech_blog/blog9_Deploying_GPT_OSS_on_TRTLLM.md:36-36
Timestamp: 2025-08-21T00:16:56.457Z
Learning: TensorRT-LLM container release tags in documentation should only reference published NGC container images. The README badge version may be ahead of the actual published container versions.
Applied to files:
examples/auto_deploy/README.md
📚 Learning: 2025-08-01T15:14:45.673Z
Learnt from: yibinl-nvidia
PR: NVIDIA/TensorRT-LLM#6506
File: examples/models/core/mixtral/requirements.txt:3-3
Timestamp: 2025-08-01T15:14:45.673Z
Learning: In TensorRT-LLM, examples directory can have different dependency versions than the root requirements.txt file. Version conflicts between root and examples dependencies are acceptable because examples are designed to be standalone and self-contained.
Applied to files:
examples/auto_deploy/README.md
🪛 LanguageTool
examples/auto_deploy/README.md
[grammar] ~2-~2: There might be a mistake here.
Context: # 🔥🚀⚡ AutoDeploy Examples This folder contains runnable examples for...
(QB_NEW_EN)
[grammar] ~75-~75: There might be a mistake here.
Context: ...s: - CLI dot notation for nested fields - YAML configs with deep merge - Precedenc...
(QB_NEW_EN)
[grammar] ~76-~76: There might be a mistake here.
Context: ...ed fields - YAML configs with deep merge - Precedence: CLI > YAML > defaults Pleas...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Pre-commit Check
🔇 Additional comments (3)
examples/auto_deploy/README.md (3)
1-3: Intro rewrite looks solid; link placement and tone are good.
Clear, concise, and consistent with the project’s “prototype” positioning. No issues spotted here.
83-83: Disclaimer aligns with main docs (“prototype stage”).
Wording is consistent and sets expectations correctly.
3-3: Optional: Add a link-check step to CI
To guard against broken external links inexamples/auto_deploy/README.md, introduce a small script that extracts all HTTP(S) URLs and fails if any return 4xx/5xx. For example:#!/usr/bin/env bash set -euo pipefail # Extract unique HTTP(S) links from README.md mapfile -t LINKS < <( rg -oP '(?<=\().+?(?=\))' examples/auto_deploy/README.md \ | grep -E '^https?://' \ | sort -u ) echo "Checking ${#LINKS[@]} links…" for url in "${LINKS[@]}"; do status=$(curl -fsSL -o /dev/null -w '%{http_code}' "$url") printf '%-80s %s\n' "$url" "$status" if (( status >= 400 )); then echo "ERROR: Broken link detected: $url" >&2 exit 1 fi done• Place this under your CI configuration (e.g.
.github/workflows/link_check.yml) or run locally before merging.
• Adjust the extraction regex or tool (e.g.markdown-link-check) as needed for your workflow.Please verify that this approach runs correctly in your CI environment and catches any dead links.
|
PR_Github #16576 [ skip ] completed with state |
Summary by CodeRabbit
Documentation
Style
Chores
Tests
Description
Test Coverage
GitHub Bot Help
/bot [-h] ['run', 'kill', 'skip', 'reuse-pipeline'] ...Provide a user friendly way for developers to interact with a Jenkins server.
Run
/bot [-h|--help]to print this help message.See details below for each supported subcommand.
run [--reuse-test (optional)pipeline-id --disable-fail-fast --skip-test --stage-list "A10-PyTorch-1, xxx" --gpu-type "A30, H100_PCIe" --test-backend "pytorch, cpp" --add-multi-gpu-test --only-multi-gpu-test --disable-multi-gpu-test --post-merge --extra-stage "H100_PCIe-TensorRT-Post-Merge-1, xxx" --detailed-log --debug(experimental)]Launch build/test pipelines. All previously running jobs will be killed.
--reuse-test (optional)pipeline-id(OPTIONAL) : Allow the new pipeline to reuse build artifacts and skip successful test stages from a specified pipeline or the last pipeline if no pipeline-id is indicated. If the Git commit ID has changed, this option will be always ignored. The DEFAULT behavior of the bot is to reuse build artifacts and successful test results from the last pipeline.--disable-reuse-test(OPTIONAL) : Explicitly prevent the pipeline from reusing build artifacts and skipping successful test stages from a previous pipeline. Ensure that all builds and tests are run regardless of previous successes.--disable-fail-fast(OPTIONAL) : Disable fail fast on build/tests/infra failures.--skip-test(OPTIONAL) : Skip all test stages, but still run build stages, package stages and sanity check stages. Note: Does NOT update GitHub check status.--stage-list "A10-PyTorch-1, xxx"(OPTIONAL) : Only run the specified test stages. Examples: "A10-PyTorch-1, xxx". Note: Does NOT update GitHub check status.--gpu-type "A30, H100_PCIe"(OPTIONAL) : Only run the test stages on the specified GPU types. Examples: "A30, H100_PCIe". Note: Does NOT update GitHub check status.--test-backend "pytorch, cpp"(OPTIONAL) : Skip test stages which don't match the specified backends. Only support [pytorch, cpp, tensorrt, triton]. Examples: "pytorch, cpp" (does not run test stages with tensorrt or triton backend). Note: Does NOT update GitHub pipeline status.--only-multi-gpu-test(OPTIONAL) : Only run the multi-GPU tests. Note: Does NOT update GitHub check status.--disable-multi-gpu-test(OPTIONAL) : Disable the multi-GPU tests. Note: Does NOT update GitHub check status.--add-multi-gpu-test(OPTIONAL) : Force run the multi-GPU tests in addition to running L0 pre-merge pipeline.--post-merge(OPTIONAL) : Run the L0 post-merge pipeline instead of the ordinary L0 pre-merge pipeline.--extra-stage "H100_PCIe-TensorRT-Post-Merge-1, xxx"(OPTIONAL) : Run the ordinary L0 pre-merge pipeline and specified test stages. Examples: --extra-stage "H100_PCIe-TensorRT-Post-Merge-1, xxx".--detailed-log(OPTIONAL) : Enable flushing out all logs to the Jenkins console. This will significantly increase the log volume and may slow down the job.--debug(OPTIONAL) : Experimental feature. Enable access to the CI container for debugging purpose. Note: Specify exactly one stage in thestage-listparameter to access the appropriate container environment. Note: Does NOT update GitHub check status.For guidance on mapping tests to stage names, see
docs/source/reference/ci-overview.mdand the
scripts/test_to_stage_mapping.pyhelper.kill
killKill all running builds associated with pull request.
skip
skip --comment COMMENTSkip testing for latest commit on pull request.
--comment "Reason for skipping build/test"is required. IMPORTANT NOTE: This is dangerous since lack of user care and validation can cause top of tree to break.reuse-pipeline
reuse-pipelineReuse 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.