[#8245][feat] Autodeploy: Guided Decoding Support#8551
[#8245][feat] Autodeploy: Guided Decoding Support#8551Superjomn merged 23 commits intoNVIDIA:mainfrom
Conversation
📝 WalkthroughWalkthroughThe pull request adds guided decoding support to AutoDeploy by propagating Changes
Sequence Diagram(s)sequenceDiagram
participant Factory as AutoModelForCausalLMFactory
participant Engine as ADEngine
participant Executor as create_autodeploy_executor
participant Decoder as GuidedDecoder
participant PyEx as PyExecutor
Factory->>Engine: vocab_size_padded
Executor->>Engine: build_from_config(ad_config)
Engine-->>Executor: vocab_size_padded
rect rgb(200, 220, 240)
note over Executor: Guided Decoding Path
Executor->>Executor: guided_decoding_backend set?
alt Yes & on_last_pp_rank
Executor->>Decoder: new GuidedDecoder(tokenizer, config)
Decoder->>Decoder: store vocab_size_padded
Executor->>PyEx: __init__(..., guided_decoder)
else No
Executor->>PyEx: __init__(..., guided_decoder=None)
end
end
Executor-->>PyEx: return instance
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes The changes span multiple files with distinct responsibilities: simple property addition in model factory, moderately complex guided decoding integration logic in executor with conditional instantiation and validation, straightforward tokenizer propagation in worker, and three test files with comprehensive coverage. The heterogeneity of changes across configuration, logic, and testing requires careful verification of each component, but the logic density is moderate and patterns are relatively consistent. Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tensorrt_llm/_torch/auto_deploy/shim/ad_executor.py (1)
1-4: Add NVIDIA Apache-2.0 header (2025).This file is missing the required copyright header. Please prepend it.
Apply this diff at the top of the file:
+# Copyright (c) 2025, NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# http://www.apache.org/licenses/LICENSE-2.0 +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License.
🧹 Nitpick comments (9)
tensorrt_llm/_torch/auto_deploy/models/hf.py (2)
127-130: Avoid redundant config fetch in init (potential perf hit).Calling
_get_model_config()here can duplicate work done later in build/load paths. Consider deferring until first access or caching the result to avoid repeated HF config downloads/parsing.
135-138: Verify “padded” semantics vs raw vocab size.
vocab_size_paddedcurrently mirrorsconfig.vocab_size. In many stacks the padded size differs (e.g., multiple-of-8/64). Please confirm this is truly padded; otherwise either compute the padded value where known or rename to avoid confusion.tests/unittest/_torch/auto_deploy/unit/singlegpu/test_ad_guided_decoding_integration.py (2)
41-46: Gate the test for CUDA and consider marking as integration/slow.This loads a real checkpoint and uses the AutoDeploy backend; it will fail or be flaky on CPU-only CI or constrained runners. Add a CUDA skip and, ideally, move to an integration suite.
Apply this diff near the imports:
+ import torch + + pytestmark = pytest.mark.skipif( + not torch.cuda.is_available(), + reason="CUDA-required AutoDeploy integration test", + )As per coding guidelines.
46-51: Stabilize output for CI.For deterministic behavior, set
temperature=0.0. Guided decoding enforces structure, but lower entropy reduces flakes.- temperature=0.1, + temperature=0.0,tests/unittest/_torch/auto_deploy/unit/singlegpu/shim/test_guided_decoding.py (1)
61-62: Instantiate the mock tokenizer.Use an instance to better reflect production usage and avoid passing a type.
- mock_tokenizer = MockTokenizer + mock_tokenizer = MockTokenizer()tests/unittest/_torch/auto_deploy/unit/singlegpu/test_ad_guided_decoding_regex.py (2)
24-32: Optionally skip when guided decoding backend isn’t available.If
xgrammar/llguidancebackends are optional builds, add a skip to prevent false CI failures when not installed.+ import importlib + + if guided_decoding_backend == "xgrammar": + pytest.skipif(importlib.util.find_spec("xgrammar") is None, reason="xgrammar not installed") + elif guided_decoding_backend == "llguidance": + pytest.skipif(importlib.util.find_spec("llguidance") is None, reason="llguidance not installed")
89-103: Also validate against the regex (not just prefixes).Complement the prefix check with a regex match to align with the test intent.
- is_valid_prefix = any( + is_valid_prefix = any( response.startswith(generated_text) or generated_text.startswith(response) for response in valid_responses ) - assert is_valid_prefix, ( + import re + regex_ok = re.match(regex_pattern, generated_text) is not None + assert is_valid_prefix or regex_ok, ( f"Test case {test_idx + 1}, Output {i} is not a valid prefix of '{valid_responses}'\n" f"Generated text: '{generated_text}'" )tensorrt_llm/_torch/auto_deploy/shim/ad_executor.py (2)
8-12: Prefer module-namespace imports per guidelines.Import modules and reference symbols via the module to keep namespaces clear.
Apply this diff and update call sites below:
-from tensorrt_llm._torch.pyexecutor.guided_decoder import GuidedDecoder -from tensorrt_llm._torch.pyexecutor.py_executor_creator import get_guided_decoding_config +from tensorrt_llm._torch.pyexecutor import guided_decoder as guided_decoder_mod +from tensorrt_llm._torch.pyexecutor import py_executor_creator as pyexec_creatorAnd later replace:
GuidedDecoder(→guided_decoder_mod.GuidedDecoder(get_guided_decoding_config(→pyexec_creator.get_guided_decoding_config(
170-175: Expose a brief docstring for the new property.Add a one-line docstring to clarify units/meaning (padded vocab size used for bitmasking).
Example:
@property def vocab_size_padded(self) -> Optional[int]: - return self._vocab_size_padded + """Model vocabulary size padded to kernel alignment; required for guided decoding.""" + return self._vocab_size_padded
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
tensorrt_llm/_torch/auto_deploy/models/hf.py(1 hunks)tensorrt_llm/_torch/auto_deploy/shim/ad_executor.py(7 hunks)tensorrt_llm/executor/base_worker.py(1 hunks)tests/unittest/_torch/auto_deploy/unit/singlegpu/shim/test_guided_decoding.py(1 hunks)tests/unittest/_torch/auto_deploy/unit/singlegpu/test_ad_guided_decoding_integration.py(1 hunks)tests/unittest/_torch/auto_deploy/unit/singlegpu/test_ad_guided_decoding_regex.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{h,hpp,hh,hxx,cpp,cxx,cc,cu,cuh,py}
📄 CodeRabbit inference engine (CODING_GUIDELINES.md)
Use only spaces, no tabs; indent with 4 spaces.
Files:
tests/unittest/_torch/auto_deploy/unit/singlegpu/shim/test_guided_decoding.pytests/unittest/_torch/auto_deploy/unit/singlegpu/test_ad_guided_decoding_integration.pytensorrt_llm/_torch/auto_deploy/models/hf.pytensorrt_llm/executor/base_worker.pytensorrt_llm/_torch/auto_deploy/shim/ad_executor.pytests/unittest/_torch/auto_deploy/unit/singlegpu/test_ad_guided_decoding_regex.py
**/*.py
📄 CodeRabbit inference engine (CODING_GUIDELINES.md)
**/*.py: Python code must target Python 3.8+.
Indent Python code with 4 spaces; do not use tabs.
Maintain module namespace when importing; prefer 'from package.subpackage import foo' then 'foo.SomeClass()' instead of importing the class directly.
Python filenames should be snake_case (e.g., some_file.py).
Python classes use PascalCase names.
Functions and methods use snake_case names.
Local variables use snake_case; prefix 'k' for variables that start with a number (e.g., k_99th_percentile).
Global variables use upper SNAKE_CASE prefixed with 'G' (e.g., G_MY_GLOBAL).
Constants use upper SNAKE_CASE (e.g., MY_CONSTANT).
Avoid shadowing variables from an outer scope.
Initialize all externally visible members of a class in the constructor.
Prefer docstrings for interfaces that may be used outside a file; comments for in-function or file-local interfaces.
Use Google-style docstrings for classes and functions (Sphinx-parsable).
Document attributes and variables inline so they render under the class/function docstring.
Avoid reflection when a simpler, explicit approach suffices (e.g., avoid dict(**locals()) patterns).
In try/except, catch the most specific exceptions possible.
For duck-typing try/except, keep the try body minimal and use else for the main logic.
Files:
tests/unittest/_torch/auto_deploy/unit/singlegpu/shim/test_guided_decoding.pytests/unittest/_torch/auto_deploy/unit/singlegpu/test_ad_guided_decoding_integration.pytensorrt_llm/_torch/auto_deploy/models/hf.pytensorrt_llm/executor/base_worker.pytensorrt_llm/_torch/auto_deploy/shim/ad_executor.pytests/unittest/_torch/auto_deploy/unit/singlegpu/test_ad_guided_decoding_regex.py
**/*.{cpp,cxx,cc,h,hpp,hh,hxx,cu,cuh,py}
📄 CodeRabbit inference engine (CODING_GUIDELINES.md)
Prepend the NVIDIA Apache-2.0 copyright header with current year to the top of all source files (e.g., .cpp, .h, .cu, .py).
Files:
tests/unittest/_torch/auto_deploy/unit/singlegpu/shim/test_guided_decoding.pytests/unittest/_torch/auto_deploy/unit/singlegpu/test_ad_guided_decoding_integration.pytensorrt_llm/_torch/auto_deploy/models/hf.pytensorrt_llm/executor/base_worker.pytensorrt_llm/_torch/auto_deploy/shim/ad_executor.pytests/unittest/_torch/auto_deploy/unit/singlegpu/test_ad_guided_decoding_regex.py
🧬 Code graph analysis (5)
tests/unittest/_torch/auto_deploy/unit/singlegpu/shim/test_guided_decoding.py (2)
tensorrt_llm/_torch/auto_deploy/llm_args.py (1)
LlmArgs(303-417)tensorrt_llm/_torch/auto_deploy/shim/ad_executor.py (1)
create_autodeploy_executor(314-457)
tests/unittest/_torch/auto_deploy/unit/singlegpu/test_ad_guided_decoding_integration.py (1)
tensorrt_llm/sampling_params.py (1)
GuidedDecodingParams(15-37)
tensorrt_llm/_torch/auto_deploy/models/hf.py (1)
tensorrt_llm/_torch/auto_deploy/shim/ad_executor.py (1)
vocab_size_padded(173-174)
tensorrt_llm/_torch/auto_deploy/shim/ad_executor.py (4)
tensorrt_llm/_torch/pyexecutor/guided_decoder.py (1)
GuidedDecoder(138-402)tensorrt_llm/_torch/pyexecutor/py_executor_creator.py (1)
get_guided_decoding_config(183-199)tensorrt_llm/llmapi/tokenizer.py (1)
TokenizerBase(24-25)tensorrt_llm/_torch/auto_deploy/models/hf.py (1)
vocab_size_padded(136-137)
tests/unittest/_torch/auto_deploy/unit/singlegpu/test_ad_guided_decoding_regex.py (2)
tests/unittest/_torch/auto_deploy/_utils_test/_model_test_utils.py (1)
get_small_model_config(508-547)tensorrt_llm/sampling_params.py (1)
GuidedDecodingParams(15-37)
🪛 Ruff (0.14.1)
tensorrt_llm/_torch/auto_deploy/shim/ad_executor.py
429-431: Avoid specifying long messages outside the exception class
(TRY003)
⏰ 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)
tensorrt_llm/executor/base_worker.py (1)
131-134: Passing tokenizer into AutoDeploy executor looks correct.Matches the PyTorch path and aligns with
create_autodeploy_executor(..., tokenizer=...). Please just confirm thattokenizer=Noneis handled gracefully when guided decoding is disabled.tensorrt_llm/_torch/auto_deploy/shim/ad_executor.py (2)
455-455: PyExecutor wiring looks correct.Passing
guided_decoderintoPyExecutorcompletes the integration.
120-126: Review comment is incorrect. The suggested defensive callable check is unnecessary because allvocab_size_paddeddefinitions across the codebase are consistently@property-decorated, not plain methods. When accessed viagetattr(), a property automatically returns its value (int or None), not a function object.Likely an incorrect or invalid review comment.
tests/unittest/_torch/auto_deploy/unit/singlegpu/shim/test_guided_decoding.py
Outdated
Show resolved
Hide resolved
tests/unittest/_torch/auto_deploy/unit/singlegpu/test_ad_guided_decoding_integration.py
Outdated
Show resolved
Hide resolved
tests/unittest/_torch/auto_deploy/unit/singlegpu/test_ad_guided_decoding_integration.py
Outdated
Show resolved
Hide resolved
tests/unittest/_torch/auto_deploy/unit/singlegpu/test_ad_guided_decoding_integration.py
Outdated
Show resolved
Hide resolved
tests/unittest/_torch/auto_deploy/unit/singlegpu/test_ad_guided_decoding_regex.py
Outdated
Show resolved
Hide resolved
|
Caution Docstrings generation - FAILED An unexpected error occurred while opening a pull request: Reference update failed - https://docs.github.com/rest/git/refs#create-a-reference |
907222c to
f4bc07b
Compare
|
/bot run |
|
PR_Github #22328 [ run ] triggered by Bot. Commit: |
|
PR_Github #22328 [ run ] completed with state |
|
/bot run |
|
PR_Github #22465 [ run ] triggered by Bot. Commit: |
Signed-off-by: Govind Ramnarayan <105831528+govind-ramnarayan@users.noreply.github.com>
Signed-off-by: Govind Ramnarayan <105831528+govind-ramnarayan@users.noreply.github.com>
…() because we call it now in the constructor Signed-off-by: Govind Ramnarayan <105831528+govind-ramnarayan@users.noreply.github.com>
…hing Signed-off-by: Lucas Liebenwein <11156568+lucaslie@users.noreply.github.com>
…) is no longer called in constructor Signed-off-by: Govind Ramnarayan <105831528+govind-ramnarayan@users.noreply.github.com>
|
PR_Github #22470 [ kill ] triggered by Bot. Commit: |
|
PR_Github #22469 [ run ] completed with state |
|
PR_Github #22470 [ kill ] completed with state |
25dd086 to
d62eb7a
Compare
|
/bot run |
|
PR_Github #22472 [ run ] triggered by Bot. Commit: |
|
PR_Github #22472 [ run ] completed with state |
Signed-off-by: Govind Ramnarayan <105831528+govind-ramnarayan@users.noreply.github.com>
|
/bot run |
|
PR_Github #22481 [ run ] triggered by Bot. Commit: |
|
PR_Github #22481 [ run ] completed with state |
|
/bot run |
|
PR_Github #22673 [ run ] triggered by Bot. Commit: |
|
PR_Github #22673 [ run ] completed with state |
Superjomn
left a comment
There was a problem hiding this comment.
LGTM on the llmapi changes
|
🎉 |
Signed-off-by: William Zhang <133824995+2ez4bz@users.noreply.github.com> Signed-off-by: Govind Ramnarayan <105831528+govind-ramnarayan@users.noreply.github.com> Signed-off-by: Lucas Liebenwein <11156568+lucaslie@users.noreply.github.com> Co-authored-by: William Zhang <133824995+2ez4bz@users.noreply.github.com> Co-authored-by: Lucas Liebenwein <11156568+lucaslie@users.noreply.github.com>
Signed-off-by: William Zhang <133824995+2ez4bz@users.noreply.github.com> Signed-off-by: Govind Ramnarayan <105831528+govind-ramnarayan@users.noreply.github.com> Signed-off-by: Lucas Liebenwein <11156568+lucaslie@users.noreply.github.com> Co-authored-by: William Zhang <133824995+2ez4bz@users.noreply.github.com> Co-authored-by: Lucas Liebenwein <11156568+lucaslie@users.noreply.github.com>
Signed-off-by: William Zhang <133824995+2ez4bz@users.noreply.github.com> Signed-off-by: Govind Ramnarayan <105831528+govind-ramnarayan@users.noreply.github.com> Signed-off-by: Lucas Liebenwein <11156568+lucaslie@users.noreply.github.com> Co-authored-by: William Zhang <133824995+2ez4bz@users.noreply.github.com> Co-authored-by: Lucas Liebenwein <11156568+lucaslie@users.noreply.github.com>
Signed-off-by: William Zhang <133824995+2ez4bz@users.noreply.github.com> Signed-off-by: Govind Ramnarayan <105831528+govind-ramnarayan@users.noreply.github.com> Signed-off-by: Lucas Liebenwein <11156568+lucaslie@users.noreply.github.com> Co-authored-by: William Zhang <133824995+2ez4bz@users.noreply.github.com> Co-authored-by: Lucas Liebenwein <11156568+lucaslie@users.noreply.github.com>
Summary by CodeRabbit
New Features
Tests
Description
Implements guided decoding in AutoDeploy. Main code change (thanks to @2ez4bz ) is to simply pass the guided decoding backend already in the TRT-LLM backend in the PyExecutor created by the AutoDeploy backend.
Test Coverage
Testing Plan:
create_autodeploy_executorwith Mocking to test that the guided decoding config makes it into thePyExecutorclass when aguided_decoding_backendis in the inputLlmArgs.test_llm_examples.pyPR 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)
Any new dependencies have been scanned for license and vulnerabilities
CODEOWNERS updated if ownership changes
Documentation updated as needed
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
/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.
Details
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.