Skip to content

[TRTLLM-15893][fix] Fix the dead import in the indexer-Q CuTe DSL prewarm - #20734

Merged
YihuiLu512 merged 1 commit into
NVIDIA:mainfrom
YihuiLu512:feat-dev-J15893
Oct 21, 2026
Merged

YihuiLu512 merged 1 commit into
NVIDIA:mainfrom
YihuiLu512:feat-dev-J15893

Conversation

@YihuiLu512

@YihuiLu512 YihuiLu512 commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator

Description

DecoderWarmup._prewarm_cute_dsl_indexer_q imported DeepseekV4Indexer from a module path that does not exist, so the ImportError was swallowed and the DSv4 indexer-Q CuTe DSL prewarm never ran. This PR fixes the import and skips indexers whose weights belong to another pipeline stage (with the import fixed, those would otherwise crash startup when PP > 1).

Test Coverage

  • tests/unittest/_torch/executor/test_pytorch_model_engine_warmup.py: new cases fail on the old code and pass with the fix (B300).
  • DSv4 smoke run on B300: the prewarm now compiles all three token counts.

PR Checklist

  • Please check this after reviewing the above items as appropriate for this PR.

Dev Engineer Review

  • warmup.py fixes the DeepseekV4Indexer import path and prevents processing indexers owned by another pipeline stage.
  • The method now requires wq_b.weight, checks the fused MXFP4 Q-projection path, and prewarms the 4-, 8-, and 16-token tactics.
  • Failure handling remains best effort. Tensor-parallel synchronization still occurs after compilation.
  • No current review findings or test execution results were supplied.

QA Engineer Review

  • tests/unittest/_torch/executor/test_pytorch_model_engine_warmup.py adds seven unit tests.
  • The tests cover successful fallback-tactic compilation, missing local indexers, disabled fused paths, first-indexer selection, per-tactic failures, all-failure completion behavior, and pipeline-stage filtering.
  • The test file is not an integration test-list file. No test-db/ or qa/ listing change applies.
  • Coverage verdict: sufficient for the changed warmup branches. Execution results were not supplied.

Per-File QA Perspective

  • tensorrt_llm/_torch/pyexecutor/engine/runners/decoder/warmup.py: Verify DSv4 indexer-Q prewarming with local and cross-stage indexers, fused and non-fused paths, all fallback token counts, and partial compilation failures. Confirm tensor-parallel barriers still execute.
  • tests/unittest/_torch/executor/test_pytorch_model_engine_warmup.py: Covers the new import, gating, synchronization, failure, and pipeline-parallel behaviors. The file is not listed in integration CI or manual-QA lists because it is a unit-test file.

…warm

DecoderWarmup._prewarm_cute_dsl_indexer_q imported DeepseekV4Indexer from a
module path that does not exist, so the ImportError was swallowed and the
prewarm never ran. Import it from the sparse deepseek_v4 package, skip
indexers whose weights belong to another pipeline stage, and add unit tests.

Signed-off-by: Yihui Lu <269394165+YihuiLu512@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: NVIDIA/TensorRT-LLM/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: e9820102-6d8f-4bb2-b6e9-488595b89ac8
📥 Commits

Reviewing files that changed from the base of the PR and between 5f90e89 and e4dc6cf.

📒 Files selected for processing: 2
  • tensorrt_llm/_torch/pyexecutor/engine/runners/decoder/warmup.py
  • tests/unittest/_torch/executor/test_pytorch_model_engine_warmup.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

Walkthrough

The indexer-Q warmup now selects local DeepSeek V4 indexers with the fused CuTe DSL projection enabled. It attempts prewarming for three fallback shapes, counts successful compilations, and retains best-effort failure handling and TP synchronization.

Changes

DeepSeek V4 indexer-Q warmup

Layer / File(s) Summary
Select and prewarm the local fused indexer
tensorrt_llm/_torch/pyexecutor/engine/runners/decoder/warmup.py, tests/unittest/_torch/executor/test_pytorch_model_engine_warmup.py
The warmup skips indexers without local weights or the fused projection. It attempts the 4-, 8-, and 16-token shapes for the first eligible indexer. Tests cover shape inputs, selection, and synchronization behavior.
Handle compilation failures and retain the barrier
tensorrt_llm/_torch/pyexecutor/engine/runners/decoder/warmup.py, tests/unittest/_torch/executor/test_pytorch_model_engine_warmup.py
A failed shape compilation logs a warning without stopping later attempts. Tests verify that the TP barrier still runs and that completion is not logged when all attempts fail.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: lori-ren

Merge Risk: ⚪ Minimal · up to e4dc6

No merge-blocking issue remains identified. The warmup can continue past a failed shape without failing startup.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 46.15% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly identifies the dead-import fix in the indexer-Q CuTe DSL prewarm and uses the required ticket and type format.
Description check Passed The description explains the import failure, the pipeline-stage handling, and the relevant unit and smoke tests. It includes the required Description, Test Coverage, and PR Checklist sections. The che…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description explains the import failure, the pipeline-stage handling, and the relevant unit and smoke tests. It includes the required Description, Test Coverage, and PR Checklist sections. The checklist details are not repeated, but the final review confirmation is present, so the description is sufficiently complete.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@YihuiLu512

Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast

@github-actions

Copy link
Copy Markdown

Automatically added "ci: full pre-merge approved" because this PR has satisfied the required GitHub review approvals. Unresolved review conversations and other required checks remain independent merge requirements.

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #79568 [ run ] triggered by Bot. Commit: e4dc6cf Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #79568 [ run ] completed with state SUCCESS. Commit: e4dc6cf
/LLM/main/L0_MergeRequest_PR pipeline #65522 completed with status: 'SUCCESS'
Pipeline passed with automatic retried tests. Check the rerun report for details.

CI Report

Link to invocation

@YihuiLu512
YihuiLu512 merged commit 370fe4c into NVIDIA:main Oct 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants