[None][fix] Update stale attention and KV-cache import paths that break main - #18771
Conversation
7202d1e to
949f0ff
Compare
…ak main Two module moves landed while dependent PRs were in flight, and main now fails to import the PyTorch runtime: 1. NVIDIA#17899 moved tensorrt_llm/_torch/pyexecutor/kv_cache_manager_v2.py under the new kv_cache/ subpackage. NVIDIA#17399, which added the phased and PrimTS FMHA libraries, still imports the old path: ModuleNotFoundError: No module named 'tensorrt_llm._torch.pyexecutor.kv_cache_manager_v2' fmha/__init__.py imports phased and prims_ts, and attention/backends/__init__.py reaches fmha through trtllm.py, so the whole attention.backends package is unimportable. 2. NVIDIA#17968 moved tensorrt_llm/_torch/attention_backend/ to tensorrt_llm/_torch/attention/backends/. NVIDIA#18652 extracted pyexecutor/engine/lora.py with the old path, and model_engine.py imports engine.lora, so the executor is unimportable even with (1) fixed. Point the three fmha modules, engine/lora.py, the visual-gen FlashInfer backend and the affected unit tests at the current paths, matching the sibling modules. Adds a CPU-only import guard covering both chains so a future move is caught by the unit tests. Signed-off-by: Brian Nguyen <brnguyen@nvidia.com>
949f0ff to
0c221e3
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #71777 [ run ] triggered by Bot. Commit: |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (10)
Included review availability: Your plan provides up to 12 included reviews per hour; 4 remain after this review. WalkthroughThe change updates relocated import paths for KV-cache and attention symbols. It adds CPU-only tests that verify attention backend, FMHA, engine.lora, and KV-cache imports. ChangesImport path alignment
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This updates stale module imports after package moves and adds import coverage for affected attention, FMHA, and LoRA paths. The updated destinations provide the required symbols, with no remaining concrete merge-blocking risk. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
PR_Github #71777 [ run ] completed with state |
|
/bot run |
|
PR_Github #71812 [ run ] triggered by Bot. Commit: |
|
PR_Github #71812 [ run ] completed with state |
juney-nvidia
left a comment
There was a problem hiding this comment.
Approved to unblock the urgent PR
…ak main (NVIDIA#18771) Signed-off-by: Brian Nguyen (cherry picked from commit 75f521d) Signed-off-by: Mihai Chiorean <mihai.v.chiorean@gmail.com>
Description
Two module moves landed while dependent PRs were in flight, and current
maincannot import the PyTorch runtime.
1. KV-cache manager V2 moved; the FMHA libraries still use the old path.
#17899 moved
tensorrt_llm/_torch/pyexecutor/kv_cache_manager_v2.pyinto thenew
pyexecutor/kv_cache/subpackage and updated the importers that existed atthe time. #17399, which added the phased and PrimTS FMHA libraries, was in
flight in parallel and still refers to the pre-move path:
fmha/__init__.pyimportsphasedandprims_ts, andattention/backends/__init__.pyreaches thefmhapackage throughtrtllm.py,so importing any attention backend executes the broken import and every unit
test under
tests/unittest/_torch/attention/fails collection.2. The attention backend package moved; the new LoRA engine module still uses
the old path. #17968 moved
tensorrt_llm/_torch/attention_backend/totensorrt_llm/_torch/attention/backends/. #18652 extractedpyexecutor/engine/lora.pywith the old path, andmodel_engine.pyimportsengine.lora, so the executor stays unimportable even with (1) fixed:This PR repoints every stale reference in the tree at the current paths:
attention/backends/fmha/{phased,prims_ts,utils}.pyandtests/unittest/_torch/attention/{test_combined_fmha,test_fmha_page_index,test_prims_ts_fmha}.pyto
tensorrt_llm._torch.pyexecutor.kv_cache.kv_cache_manager_v2pyexecutor/engine/lora.py,visual_gen/attention_backend/flashinfer.pyandtests/unittest/_torch/visual_gen/test_attention_flashinfer.pyto
tensorrt_llm._torch.attention.backends.interfaceThe fully qualified KV-cache path is used rather than a shorter re-export because
pyexecutor/kv_cache/__init__.pyis intentionally empty; this matches thesibling backends (
attention/backends/interface.py,attention/backends/sparse/dsa/cache_manager.py).This is an import-path change only. No logic, signatures or behavior are
touched.
Test Coverage
New CPU-only guard,
tests/unittest/_torch/attention/test_backends_importable.py:imports
tensorrt_llm._torch.attention.backends, each of the threefmhamodules and
pyexecutor.engine.lora, and asserts the moved names resolve attheir source modules. On the parent commit it fails at collection with the
errors above; the intent is that the next time one of these modules moves, the
unit tests catch it instead of whole suites going uncollected.
Verified on a built checkout at the parent commit plus this change:
test_backends_importable.pypasses;import tensorrt_llm._torch.attention.backendssucceeds; the three previously uncollectable FMHA unit tests collect and pass
(
test_combined_fmha.py152,test_fmha_page_index.py33,test_prims_ts_fmha.py78);tests/unittest/_torch/attention/test_flashinfer_attention.pypasses on one GPU;
tests/unittest/_torch/visual_gen/test_attention_flashinfer.pycollects and passes on one GPU (15 passed, 4 skipped);
from tensorrt_llm import LLMand the model engine import succeed.
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.Dev Engineer Review
KVCacheManagerV2,Role,AttentionMetadata, andPredefinedAttentionMask.QA Engineer Review
test_attention_backends_package_importstest_fmha_module_importstest_engine_lora_importstest_kv_cache_manager_v2_names_resolvetests/integration/test_lists/,test-db/, orqa/.