Skip to content

[https://nvbugs/6240420][fix] Clamp KV pool window sizes to max_seq_len - #14905

Merged
eopXD merged 2 commits into
NVIDIA:mainfrom
eopXD:nvbugs6248764
Jun 5, 2026
Merged

[https://nvbugs/6240420][fix] Clamp KV pool window sizes to max_seq_len#14905
eopXD merged 2 commits into
NVIDIA:mainfrom
eopXD:nvbugs6248764

Conversation

@eopXD

@eopXD eopXD commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

Description

KVCacheManager.__init__ clamps each entry of max_attention_window_vec to min(w, max_seq_len) but left the pool_configurations window keys unclamped. _build_layer_to_pool_idx then keyed the window→pool map by the raw pool windows while looking up the clamped per-layer windows, raising KeyError whenever sliding_window > max_seq_len.

Seen on the AutoDeploy registry accuracy test for mistralai/Ministral-8B-Instruct-2410 (sliding_window=32768) run with the dashboard default max_seq_len=512: the single pool was keyed 32768 while every layer resolved to a clamped window of 512, giving KeyError(512).

Fix: clamp the pool windows to the same max_seq_len bound inside _resolve_max_attention_window_vec, keeping pool window keys consistent with the resolved vector. A window larger than max_seq_len can never slide, so it is functionally a full-attention pool of max_seq_len.

Test Coverage

  • test_autodeploy_from_registry[mistralai_Ministral-8B-Instruct-2410-False] passes locally with the patch.

@eopXD
eopXD requested a review from a team as a code owner June 3, 2026 10:58
@eopXD

eopXD commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast

@coderabbitai

coderabbitai Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

KVCacheManager now passes its configured pool_configurations into the _resolve_max_attention_window_vec() function during initialization. The function was updated to accept this optional parameter and, when provided, in-place clamps each pool configuration's window_size to max_seq_len to maintain consistency between the per-pool window keys and the resolved (possibly clamped) attention window vector.

Changes

Pool window configuration consistency

Layer / File(s) Summary
Attention window resolution with pool configuration clamping
tensorrt_llm/_torch/pyexecutor/resource_manager.py
_resolve_max_attention_window_vec() gains an optional pool_configurations parameter. When provided, it clamps each PoolConfiguration.window_size in-place to max_seq_len. KVCacheManager.__init__ is updated to pass self.pool_configurations to this function, ensuring the returned window vector and pool configuration keys remain synchronized during layer→pool mapping construction.

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Title check ✅ Passed The title clearly and specifically identifies the main fix: clamping KV pool window sizes to max_seq_len. It is concise, directly related to the core change in the changeset, and accurately reflects the bug fix described in the PR objectives.
Description check ✅ Passed PR description clearly explains the issue (pool window keys unclamped while per-layer windows clamped causing KeyError), the root cause, the specific failure case, the fix rationale, and includes test coverage confirmation.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

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

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


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

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #51828 [ run ] triggered by Bot. Commit: cc0d530 Link to invocation

@eopXD eopXD changed the title [https://nvbugs/6248764][fix] Clamp KV pool window sizes to max_seq_len [https://nvbugs/6240420][fix] Clamp KV pool window sizes to max_seq_len Jun 3, 2026
@eopXD

eopXD commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #51831 [ run ] triggered by Bot. Commit: 8a8fe17 Link to invocation

KVCacheManager.__init__ resolved max_attention_window_vec with each
window clamped to min(w, max_seq_len), but left pool_configurations
window keys unclamped. _build_layer_to_pool_idx then keyed the
window->pool map by the raw pool windows while looking up the clamped
per-layer windows, raising KeyError when sliding_window > max_seq_len.

This surfaced on the AutoDeploy registry accuracy test for
mistralai/Ministral-8B-Instruct-2410 (sliding_window=32768) run with the
dashboard default max_seq_len=512: the single pool was keyed 32768 while
every layer resolved to a clamped window of 512, giving KeyError(512).

Clamp the pool windows to the same max_seq_len bound inside
_resolve_max_attention_window_vec so the pool window keys stay consistent
with the returned vector. A window larger than max_seq_len can never
slide, so it is functionally a full-attention pool of max_seq_len.

Re-enable the Ministral registry accuracy test that this fixes.

Signed-off-by: Yueh-Ting Chen <yuehtingc@nvidia.com>
@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #51828 [ run ] completed with state ABORTED. Commit: cc0d530

Link to invocation

@eopXD

eopXD commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #51833 [ run ] triggered by Bot. Commit: 8503c45 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #51831 [ run ] completed with state ABORTED. Commit: 8a8fe17

Link to invocation

@eopXD

eopXD commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator Author

/bot run --extra-stage "DGX_H100-4_GPUs-AutoDeploy-Post-Merge-1"

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #51840 [ run ] triggered by Bot. Commit: 8503c45 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #51833 [ run ] completed with state ABORTED. Commit: 8503c45

Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #51840 [ run ] completed with state SUCCESS. Commit: 8503c45
/LLM/main/L0_MergeRequest_PR pipeline #41202 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

Link to invocation

@eopXD

eopXD commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator Author

/bot run --extra-stage "DGX_H100-4_GPUs-AutoDeploy-Post-Merge-1" --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #51851 [ run ] triggered by Bot. Commit: 8503c45 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #51851 [ run ] completed with state SUCCESS. Commit: 8503c45
/LLM/main/L0_MergeRequest_PR pipeline #41210 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

Link to invocation

@tburt-nv

tburt-nv commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

/bot run --extra-stage "DGX_H100-4_GPUs-AutoDeploy-Post-Merge-1" --disable-fail-fast

@tburt-nv

tburt-nv commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

DGX_H100-4_GPUs-AutoDeploy-Post-Merge-1 passed in #14905 (comment)

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #51893 [ run ] triggered by Bot. Commit: 8503c45 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #51893 [ run ] completed with state FAILURE. Commit: 8503c45
/LLM/main/L0_MergeRequest_PR pipeline #41251 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

Link to invocation

@eopXD

eopXD commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator Author

/bot run --extra-stage "DGX_H100-4_GPUs-AutoDeploy-Post-Merge-1" --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #52196 [ run ] triggered by Bot. Commit: 8503c45 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #52196 [ run ] completed with state SUCCESS. Commit: 8503c45
/LLM/main/L0_MergeRequest_PR pipeline #41516 completed with status: 'SUCCESS'

CI Report

Link to invocation

@eopXD

eopXD commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator Author

/bot skip --comment "CI passed. The latest commit is merge from origin/main to resolve conflict under waive.txt"

@eopXD
eopXD enabled auto-merge (squash) June 5, 2026 02:30
@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #52211 [ skip ] triggered by Bot. Commit: 1535e96 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #52211 [ skip ] completed with state SUCCESS. Commit: 1535e96
Skipping testing for commit 1535e96

Link to invocation

@eopXD
eopXD merged commit bd17d1b into NVIDIA:main Jun 5, 2026
8 checks passed
fbxai pushed a commit to fbxai/TensorRT-LLM that referenced this pull request Jun 5, 2026
…en (NVIDIA#14905)

Signed-off-by: Yueh-Ting Chen <yuehtingc@nvidia.com>
Signed-off-by: NVFB <186336021+NVFB@users.noreply.github.com>
2ez4bz pushed a commit to 2ez4bz/TensorRT-LLM that referenced this pull request Jun 8, 2026
…en (NVIDIA#14905)

Signed-off-by: Yueh-Ting Chen <yuehtingc@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.

4 participants