[None][feat] BREAKING: add per-model transceiver runtime auto selection#16164
[None][feat] BREAKING: add per-model transceiver runtime auto selection#16164nv-xtf wants to merge 1 commit into
Conversation
64bcec3 to
f0376f7
Compare
📝 WalkthroughWalkthroughAdds an "auto" option for ChangesTransceiver Runtime Auto Resolution
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
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.
🧹 Nitpick comments (1)
tests/unittest/llmapi/test_llm_args.py (1)
3017-3309: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCoverage gap: no test for explicit "CPP" model preference adoption.
The suite thoroughly covers "PYTHON" adoption, fallback-to-None, and invalid-value rejection, but no fake model returns
"CPP"fromget_preferred_transceiver_runtime. Add a_PreferCppTransceiverModelfixture and atest_cpp_preference_adoptedcase (parallel totest_model_preference_adopted) to guard the direct-assignment branch in_resolve_transceiver_runtime_autoagainst future regressions.As per path instructions, "Act as a QA engineer reviewing test changes and coverage for TensorRT-LLM... suggest concrete list file names and whether coverage is sufficient, insufficient, or needs follow-up outside the PR."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unittest/llmapi/test_llm_args.py` around lines 3017 - 3309, The transceiver-runtime auto-resolution tests are missing coverage for the explicit "CPP" preference branch in _resolve_transceiver_runtime_auto. Add a _PreferCppTransceiverModel fixture alongside _PreferPythonTransceiverModel, then add a test_cpp_preference_adopted case parallel to test_model_preference_adopted to verify that get_preferred_transceiver_runtime returning "CPP" is adopted correctly when the config is auto. Keep the new test within TestTransceiverRuntimeAutoResolution and use the same _disagg_args helper so the branch is exercised consistently.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/unittest/llmapi/test_llm_args.py`:
- Around line 3017-3309: The transceiver-runtime auto-resolution tests are
missing coverage for the explicit "CPP" preference branch in
_resolve_transceiver_runtime_auto. Add a _PreferCppTransceiverModel fixture
alongside _PreferPythonTransceiverModel, then add a test_cpp_preference_adopted
case parallel to test_model_preference_adopted to verify that
get_preferred_transceiver_runtime returning "CPP" is adopted correctly when the
config is auto. Keep the new test within TestTransceiverRuntimeAutoResolution
and use the same _disagg_args helper so the branch is exercised consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 1e30e36e-8e9e-4c45-9a6b-84e07a9dcb65
📒 Files selected for processing (9)
docs/source/developer-guide/telemetry.mdtensorrt_llm/_torch/models/modeling_utils.pytensorrt_llm/_torch/pyexecutor/kv_cache_transceiver.pytensorrt_llm/_torch/pyexecutor/model_loader.pytensorrt_llm/llmapi/llm_args.pytensorrt_llm/llmapi/llm_utils.pytensorrt_llm/usage/llm_args_golden_manifest.jsontests/unittest/llmapi/test_llm_args.pytests/unittest/usage/test_llmapi_config_capture.py
Signed-off-by: Tingfeng Xian <289617005+nv-xtf@users.noreply.github.com>
f0376f7 to
3c583b0
Compare
|
/bot run |
|
PR_Github #58389 [ run ] triggered by Bot. Commit: |
|
PR_Github #58389 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #58445 [ run ] triggered by Bot. Commit: |
Description
Add per-model auto selection for the KV-cache transceiver runtime on the standard PyTorch model-loading path.
autoas the default value ofCacheTransceiverConfig.transceiver_runtime.get_preferred_transceiver_runtime()for models to declare their preferred runtime.CPP,PYTHON, orNone).get_model_defaults()from implicitly creating or enablingcache_transceiver_config.No model opts into the Python transceiver in this PR, so the effective default remains the C++ transceiver until model-specific migrations are added.
Model Opt-in
To provide an automatic preference for a model, override
get_preferred_transceiver_runtime()in itsForCausalLMimplementation:Valid return values are
"PYTHON","CPP", orNone(defer to the global default, i.e. the C++ transceiver). Shared implementation classes can inspectpretrained_config.architecturesto return different preferences for different model architectures.The user can always override the model preference explicitly:
Resolution outcomes:
autoPYTHONautoPYTHONautoNoneCPP/NonePYTHONPYTHONautoonly selects the runtime after the user enablescache_transceiver_config; it does not enable disaggregated serving by itself. On paths that skip the standard PyTorch model loader (e.g. AutoDeploy),autofalls back to the C++ transceiver — in mixed deployments, settransceiver_runtimeexplicitly on both sides to keep them aligned.The preference must not be declared through
get_model_defaults(), because model defaults must not create or enable a transceiver configuration (rejected at load time).With
TLLM_LOG_LEVEL=INFO, adoption is visible at startup:Resolved transceiver_runtime='auto' to 'PYTHON' for GptOssForCausalLM.followed byUsing KvCacheTransceiverV2.Breaking Change
The default value of
CacheTransceiverConfig.transceiver_runtimechanges fromNone(C++ runtime) toauto. Existing explicit values (CPP,PYTHON, andNone) remain supported. No model opts into the Python runtime in this PR, so the effective runtime still falls back to C++ unless a model-specific preference is added later.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-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.Summary by CodeRabbit
New Features
Bug Fixes
Documentation