Skip to content

[None][feat] Assert attention DP disabled when KV connector is in use#13448

Merged
pcastonguay merged 1 commit intoNVIDIA:mainfrom
jthomson04:jthomson04/assert-kv-connector-no-attention-dp
Apr 27, 2026
Merged

[None][feat] Assert attention DP disabled when KV connector is in use#13448
pcastonguay merged 1 commit intoNVIDIA:mainfrom
jthomson04:jthomson04/assert-kv-connector-no-attention-dp

Conversation

@jthomson04
Copy link
Copy Markdown
Collaborator

@jthomson04 jthomson04 commented Apr 24, 2026

Summary

  • KV connector initialization currently has no compatibility check against attention data parallelism. The combination has not been validated and there is no established plumbing for the connector scheduler/worker to reason about per-DP-rank KV state.
  • Adds a NotImplementedError in py_executor_creator._create_py_executor that fires when mapping.enable_attention_dp is True and a kv_connector_config is provided, matching the style of the existing scheduler-policy and VSWA guards.

Test plan

  • Launch with kv_connector_config + enable_attention_dp=True and confirm the new NotImplementedError is raised before any connector worker is instantiated.
  • Launch with kv_connector_config + enable_attention_dp=False and confirm startup is unchanged.

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced configuration validation to prevent incompatible feature combinations. The system now explicitly rejects the concurrent use of KV connector with attention data parallelism, providing a clear error message. This improvement prevents unexpected behavior and configuration errors by blocking unsupported feature interactions at initialization time.

@jthomson04 jthomson04 requested a review from a team as a code owner April 24, 2026 19:56
@jthomson04 jthomson04 requested a review from lancelly April 24, 2026 19:56
@jthomson04
Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 24, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 12d3bbf9-1eef-4b37-88bb-07471a695968

📥 Commits

Reviewing files that changed from the base of the PR and between 55e2727 and afffd53.

📒 Files selected for processing (1)
  • tensorrt_llm/_torch/pyexecutor/py_executor_creator.py

📝 Walkthrough

Walkthrough

A validation check is added to create_py_executor that explicitly rejects configurations combining KV connector with attention data parallelism by checking mapping.enable_attention_dp and raising NotImplementedError if both are enabled.

Changes

Cohort / File(s) Summary
Validation Check
tensorrt_llm/_torch/pyexecutor/py_executor_creator.py
Added validation to reject unsupported configuration combining KV connector with attention data parallelism; check occurs after scheduler/VSWA compatibility validations.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~4 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically summarizes the main change: adding an assertion that attention DP is disabled when KV connector is in use.
Description check ✅ Passed The PR description includes a clear summary of the issue and the solution, and explicitly documents the test plan with checkmarks indicating tests have been performed.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.

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

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

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

@tensorrt-cicd
Copy link
Copy Markdown
Collaborator

PR_Github #45439 [ run ] triggered by Bot. Commit: afffd53 Link to invocation

@tensorrt-cicd
Copy link
Copy Markdown
Collaborator

PR_Github #45439 [ run ] completed with state FAILURE. Commit: afffd53
/LLM/main/L0_MergeRequest_PR pipeline #35671 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

@jthomson04 jthomson04 force-pushed the jthomson04/assert-kv-connector-no-attention-dp branch from afffd53 to 4828836 Compare April 25, 2026 13:57
@jthomson04
Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast

@tensorrt-cicd
Copy link
Copy Markdown
Collaborator

PR_Github #45508 [ run ] triggered by Bot. Commit: 4828836 Link to invocation

@tensorrt-cicd
Copy link
Copy Markdown
Collaborator

PR_Github #45508 [ run ] completed with state SUCCESS. Commit: 4828836
/LLM/main/L0_MergeRequest_PR pipeline #35732 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

KV connector has not been validated with attention data parallelism, and
there is no established plumbing to make the two work together. Fail
fast at connector init rather than silently producing incorrect behavior.

Signed-off-by: jthomson04 <jwillthomson19@gmail.com>
@jthomson04 jthomson04 force-pushed the jthomson04/assert-kv-connector-no-attention-dp branch from 4828836 to 61d7d3f Compare April 25, 2026 23:41
@jthomson04
Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast

@jthomson04 jthomson04 requested a review from pcastonguay April 25, 2026 23:46
@tensorrt-cicd
Copy link
Copy Markdown
Collaborator

PR_Github #45517 [ run ] triggered by Bot. Commit: 61d7d3f Link to invocation

@tensorrt-cicd
Copy link
Copy Markdown
Collaborator

PR_Github #45517 [ run ] completed with state SUCCESS. Commit: 61d7d3f
/LLM/main/L0_MergeRequest_PR pipeline #35741 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

@pcastonguay
Copy link
Copy Markdown
Collaborator

/bot run --disable-fail-fast

@pcastonguay pcastonguay enabled auto-merge (squash) April 27, 2026 13:15
@tensorrt-cicd
Copy link
Copy Markdown
Collaborator

PR_Github #45736 [ run ] triggered by Bot. Commit: 61d7d3f Link to invocation

@tensorrt-cicd
Copy link
Copy Markdown
Collaborator

PR_Github #45736 [ run ] completed with state FAILURE. Commit: 61d7d3f
/LLM/main/L0_MergeRequest_PR pipeline #35932 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

@pcastonguay
Copy link
Copy Markdown
Collaborator

/bot run --disable-fail-fast

@tensorrt-cicd
Copy link
Copy Markdown
Collaborator

PR_Github #45781 [ run ] triggered by Bot. Commit: 61d7d3f Link to invocation

@tensorrt-cicd
Copy link
Copy Markdown
Collaborator

PR_Github #45781 [ run ] completed with state SUCCESS. Commit: 61d7d3f
/LLM/main/L0_MergeRequest_PR pipeline #35971 completed with status: 'SUCCESS'

CI Report

Link to invocation

@pcastonguay pcastonguay merged commit e7f1062 into NVIDIA:main Apr 27, 2026
5 checks passed
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