[TRTLLM-13176][feat] Enables CUDA graph execution for PyTorch encoder-decoder models - #15637
Conversation
Signed-off-by: Guiju Zhang <7135567+cascade812@users.noreply.github.com>
Signed-off-by: Guiju Zhang <7135567+cascade812@users.noreply.github.com>
📝 WalkthroughWalkthroughEncoder-decoder CUDA graph handling now threads encoder lengths through metadata and dummy requests, adds in-place cross-metadata updates, extends warmup and replay paths for encoder-decoder models, and adds BART and T5 integration coverage for batch-size-aware and mixed-context execution. ChangesEncoder-decoder CUDA graph support
Estimated code review effort🎯 5 (Critical) | ⏱️ ~90+ minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tests/integration/defs/llmapi/test_llm_api_pytorch_bart.py (2)
125-150: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd batch-size-aware coverage for the other enabled BART CUDA graph paths.
Coverage is currently insufficient for beam-search and KVCacheManagerV2 batch-size-aware CUDA graph behavior: these enabled cases default to batch size
[1], so only the v1 greedy case exercises the[2]padding/dummy-request path. Consider addingcuda_graph_batch_sizes=[2]to these enabled cases or adding separate list entries for beam2 and kv-v2.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integration/defs/llmapi/test_llm_api_pytorch_bart.py` around lines 125 - 150, The BART CUDA graph test matrix in test_llm_api_pytorch_bart.py is missing batch-size-aware coverage for the enabled beam-search and KVCacheManagerV2 paths, so only the existing v1 greedy case exercises the batch size [2] padding/dummy-request behavior. Update the relevant _test_case entries to include cuda_graph_batch_sizes=[2] or add separate feature_id variants for the beam2 and kv-v2 enabled CUDA graph cases, so the enabled paths are validated with non-default batch sizing.Source: Path instructions
327-356: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winWire
kv_cache_dtypeintoKvCacheConfigor remove it from the parameter set.
kv_cache_dtypeis accepted and forwarded into this helper, but it is not applied when constructingKvCacheConfig, so any non-default parametrized case would silently still run with"auto".Proposed fix
kv_cache_config=KvCacheConfig( enable_block_reuse=False, max_tokens=_MAX_KV_TOKENS, free_gpu_memory_fraction=_FREE_GPU_MEMORY_FRACTION, cross_kv_cache_fraction=_CROSS_KV_CACHE_FRACTION, use_kv_cache_manager_v2=use_kv_cache_manager_v2, + dtype=kv_cache_dtype, ),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integration/defs/llmapi/test_llm_api_pytorch_bart.py` around lines 327 - 356, The helper currently accepts kv_cache_dtype but never uses it when building KvCacheConfig, so non-default test cases still run with the default value. Update the LLM setup in test_llm_api_pytorch_bart.py so the kv_cache_dtype argument is wired into KvCacheConfig, or remove kv_cache_dtype from the helper signature and any callers if it is intentionally unused; use the existing _sampling_params and KvCacheConfig construction in the test helper to locate the change.
🤖 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 `@tensorrt_llm/_torch/pyexecutor/py_executor.py`:
- Around line 572-577: The encoder-decoder CUDA-graph guard in PyExecutor
currently only blocks speculative decoding when model_engine.spec_config is
present, but _prepare_and_schedule_batch() can also enable speculation via
self.drafter, so the unsupported path can still be reached. Update the check in
py_executor.py to reject CUDA graph whenever cuda_graph_config is set and either
spec_config or drafter-based speculation is enabled, using the existing
PyExecutor model_engine/self.drafter logic, and keep the NotImplementedError
path covering both speculative sources.
In `@tests/integration/defs/llmapi/test_llm_api_pytorch_t5.py`:
- Around line 861-866: The mixed-context test only checks response shape, so it
can miss swapped or incorrect encoder context. Update the test around the second
request in this T5 mixed-context case to validate the actual decoded content,
using the existing _assert_t5_response helper and the
first_response/second_response values, by asserting prompt-specific expected
fragments or at minimum that the two generated outputs differ and align with
their intended source texts.
---
Outside diff comments:
In `@tests/integration/defs/llmapi/test_llm_api_pytorch_bart.py`:
- Around line 125-150: The BART CUDA graph test matrix in
test_llm_api_pytorch_bart.py is missing batch-size-aware coverage for the
enabled beam-search and KVCacheManagerV2 paths, so only the existing v1 greedy
case exercises the batch size [2] padding/dummy-request behavior. Update the
relevant _test_case entries to include cuda_graph_batch_sizes=[2] or add
separate feature_id variants for the beam2 and kv-v2 enabled CUDA graph cases,
so the enabled paths are validated with non-default batch sizing.
- Around line 327-356: The helper currently accepts kv_cache_dtype but never
uses it when building KvCacheConfig, so non-default test cases still run with
the default value. Update the LLM setup in test_llm_api_pytorch_bart.py so the
kv_cache_dtype argument is wired into KvCacheConfig, or remove kv_cache_dtype
from the helper signature and any callers if it is intentionally unused; use the
existing _sampling_params and KvCacheConfig construction in the test helper to
locate the change.
🪄 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: e9c34301-d9a0-4d51-9cb9-a029750a6062
📒 Files selected for processing (9)
tensorrt_llm/_torch/attention_backend/interface.pytensorrt_llm/_torch/pyexecutor/cuda_graph_runner.pytensorrt_llm/_torch/pyexecutor/kv_cache_manager_v2.pytensorrt_llm/_torch/pyexecutor/model_engine.pytensorrt_llm/_torch/pyexecutor/py_executor.pytensorrt_llm/_torch/pyexecutor/resource_manager.pytests/integration/defs/llmapi/test_llm_api_pytorch_bart.pytests/integration/defs/llmapi/test_llm_api_pytorch_t5.pytests/unittest/_torch/helpers.py
Signed-off-by: Guiju Zhang <7135567+cascade812@users.noreply.github.com>
|
@lowsfer @yizhang-nv could you review this? thanks! |
|
/bot run |
|
PR_Github #56138 [ run ] triggered by Bot. Commit: |
|
PR_Github #56138 [ run ] completed with state
|
Signed-off-by: Guiju Zhang <7135567+cascade812@users.noreply.github.com>
Signed-off-by: Guiju Zhang <7135567+cascade812@users.noreply.github.com>
|
/bot run --disable-fail-fast |
|
PR_Github #56308 [ run ] triggered by Bot. Commit: |
|
PR_Github #56308 [ run ] completed with state
|
Signed-off-by: Guiju Zhang <7135567+cascade812@users.noreply.github.com>
Signed-off-by: Guiju Zhang <7135567+cascade812@users.noreply.github.com>
|
/bot run --disable-fail-fast |
|
PR_Github #57097 [ run ] triggered by Bot. Commit: |
|
/bot run |
|
PR_Github #57301 [ run ] triggered by Bot. Commit: |
|
PR_Github #57097 [ run ] completed with state |
|
PR_Github #57301 [ run ] completed with state
|
|
/bot run |
|
PR_Github #57361 [ run ] triggered by Bot. Commit: |
|
PR_Github #57361 [ run ] completed with state
|
|
/bot run |
Signed-off-by: Guiju Zhang <7135567+cascade812@users.noreply.github.com>
|
/bot run |
|
PR_Github #57505 [ run ] triggered by Bot. Commit: |
|
PR_Github #57505 [ run ] completed with state
|
|
/bot run |
|
PR_Github #57511 [ run ] triggered by Bot. Commit: |
|
PR_Github #57511 [ run ] completed with state
|
|
/bot run |
|
PR_Github #57528 [ run ] triggered by Bot. Commit: |
|
PR_Github #57528 [ run ] completed with state |
Signed-off-by: Guiju Zhang <7135567+cascade812@users.noreply.github.com>
|
/bot skip --comment "pipeline 46259 passed" |
|
PR_Github #57584 [ skip ] triggered by Bot. Commit: |
|
PR_Github #57584 [ skip ] completed with state |
…-decoder models (NVIDIA#15637) Signed-off-by: Guiju Zhang <7135567+cascade812@users.noreply.github.com>
Description
This PR enables CUDA graph execution for the decoder generation path of PyTorch encoder-decoder models, with initial coverage for T5 and BART.
The encoder prepass for encoder-decoder generation remains eager. Encoder CUDA graph capture/replay is preserved only for the encode-only / encoder-only path.
Key changes:
CudaGraphConfigdecoder-style configuration for encoder-decoder models.Why decoder-only CUDA graphs for encoder-decoder models
We evaluated enabling CUDA graph capture for both the encoder forward pass and the decoder generation pass. The measured end-to-end latency gain from encoder+decoder CUDA graphs was almost the same as decoder-only CUDA graphs.
That result is expected for the target encoder-decoder serving path:
(batch_size, encoder_num_tokens, encoder_seq_len), warmup inputs, padding behavior, and cross-KV/cache-state interactions.For that reason this PR intentionally does not enable CUDA graph support for the encoder forward pass of encoder-decoder models. Users should enable CUDA graphs for the decoder path only.
Customer Usage: Enabling Decoder CUDA Graph for Encoder-Decoder Models
For T5/BART-style PyTorch encoder-decoder models, use the existing
CudaGraphConfigto configure decoder CUDA graph batch buckets.EncodeCudaGraphConfigremains for encode-only models. It is not supported for encoder-decoder generation.Test Coverage
python3 -m py_compile tensorrt_llm/_torch/pyexecutor/model_engine.py tensorrt_llm/llmapi/llm_args.py tensorrt_llm/llmapi/__init__.py tensorrt_llm/llmapi/llm_utils.py tests/integration/defs/llmapi/test_llm_api_pytorch_t5.py tests/integration/defs/llmapi/test_llm_api_pytorch_bart.py tests/unittest/llmapi/test_llm_args.pygit diff --checkPYTHONPATH=/home/scratch.guijuz_coreai/TensorRT-LLM pytest tests/unittest/llmapi/test_llm_args.py::TestTorchLlmArgsCudaGraphSettings -qPYTHONPATH=/home/scratch.guijuz_coreai/TensorRT-LLM pytest tests/unittest/llmapi/test_llm_encode.py -qPYTHONPATH=/home/scratch.guijuz_coreai/TensorRT-LLM pytest --collect-only tests/integration/defs/llmapi/test_llm_api_pytorch_t5.py tests/integration/defs/llmapi/test_llm_api_pytorch_bart.py -qPR 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-compatibleorapi-breaking. Forapi-breaking, includeBREAKINGin 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.