Skip to content

[None][fix] Fix stale sparse attention kwargs - #15460

Merged
bobboli merged 1 commit into
NVIDIA:mainfrom
bobboli:bli/fix-sparse-stale-kwargs
Jun 18, 2026
Merged

[None][fix] Fix stale sparse attention kwargs#15460
bobboli merged 1 commit into
NVIDIA:mainfrom
bobboli:bli/fix-sparse-stale-kwargs

Conversation

@bobboli

@bobboli bobboli commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator

Summary

  1. Fix the stale sparse-attention kwarg in the encoder-decoder cross KV-cache manager path.
  2. Fix the encoder attention metadata path to pass lowered sparse_metadata_params instead of the user-facing sparse config.

Details

PR #14687 renamed and split sparse-attention plumbing so AttentionMetadata consumes SparseMetadataParams, while KV-cache manager construction uses sparse_attention_config.

Two stale call sites remained on main:

  1. KvCacheCreator._create_cross_kv_cache_manager() still passed sparse_attn_config=None to _create_kv_cache_manager(), whose parameter is now sparse_attention_config.
  2. PyTorchModelEngine._prepare_tp_inputs_encoder() still passed sparse_attention_config=self.sparse_attention_config into self.attn_backend.Metadata(...), but TrtllmAttentionMetadata now expects sparse_metadata_params.

This fixes both reported pre-merge CI failures:

  • _create_kv_cache_manager() got an unexpected keyword argument 'sparse_attn_config'
  • TrtllmAttentionMetadata.__init__() got an unexpected keyword argument 'sparse_attention_config'

Test Plan

  1. git diff --check
  2. python3 -m py_compile tensorrt_llm/_torch/pyexecutor/_util.py tensorrt_llm/_torch/pyexecutor/model_engine.py
  3. pre-commit run --files tensorrt_llm/_torch/pyexecutor/_util.py tensorrt_llm/_torch/pyexecutor/model_engine.py
  4. Not run locally: pytest tests/unittest/_torch/executor/test_dual_pool_kv_cache.py -q, because the local Python environment does not have pytest installed.

Summary by CodeRabbit

  • Refactor
    • Updated sparse attention configuration handling in encoder-decoder models for improved internal consistency.

Signed-off-by: Bo Li <22713281+bobboli@users.noreply.github.com>
@bobboli
bobboli requested a review from a team as a code owner June 17, 2026 15:09
@bobboli
bobboli requested a review from HuiGao-NV June 17, 2026 15:09
@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

An error occurred during the review process. Please try again later.

📝 Walkthrough

Walkthrough

Two small fixes adjust sparse attention handling in encoder-side paths: the cross-attention KV cache manager is now constructed with sparse_attention_config=None, and the encoder attention metadata constructor receives the derived sparse_metadata_params (via to_sparse_metadata_params) rather than the raw sparse_attention_config object.

Changes

Sparse Attention Encoder Path Fixes

Layer / File(s) Summary
Encoder sparse attention config and cross-KV cache fixups
tensorrt_llm/_torch/pyexecutor/_util.py, tensorrt_llm/_torch/pyexecutor/model_engine.py
_create_cross_kv_cache_manager passes None for sparse_attention_config instead of forwarding sparse_attn_config. In _prepare_tp_inputs_encoder, sparse_metadata_params is computed via sparse_attention_config.to_sparse_metadata_params(...) and passed to the encoder attention metadata constructor, replacing the prior direct sparse_attention_config argument.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Suggested reviewers

  • heyuhhh
  • yuxianq
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@bobboli
bobboli enabled auto-merge (squash) June 17, 2026 15:14
@bobboli

bobboli commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #54860 [ run ] triggered by Bot. Commit: c0a3c44 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #54860 [ run ] completed with state FAILURE. Commit: c0a3c44
/LLM/main/L0_MergeRequest_PR pipeline #43865 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

@yuxianq

yuxianq commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator

/bot run --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #54879 [ run ] triggered by Bot. Commit: c0a3c44 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #54879 [ run ] completed with state SUCCESS. Commit: c0a3c44
/LLM/main/L0_MergeRequest_PR pipeline #43884 completed with status: 'SUCCESS'

CI Report

Link to invocation

@bobboli
bobboli merged commit 08f4bb1 into NVIDIA:main Jun 18, 2026
12 checks passed
xinhe-nv pushed a commit to tensorrt-cicd/TensorRT-LLM that referenced this pull request Jun 23, 2026
Signed-off-by: Bo Li <22713281+bobboli@users.noreply.github.com>
Signed-off-by: GitLab CI Bot <gitlab-ci@nvidia.com>
xinhe-nv pushed a commit to tensorrt-cicd/TensorRT-LLM that referenced this pull request Jun 24, 2026
Signed-off-by: Bo Li <22713281+bobboli@users.noreply.github.com>
Signed-off-by: GitLab CI Bot <gitlab-ci@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants