[None][fix] Keep hybrid KV routing for Skip Softmax - #15403
Conversation
📝 WalkthroughWalkthrough
ChangesKV Cache Manager Routing Fix
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tensorrt_llm/_torch/pyexecutor/_util.py`:
- Around line 94-99: The sparse attention algorithm validation check (that
raises ValueError for non-skip_softmax algorithms with hybrid Mamba models) is
executing before the zero-mamba fallback logic, causing degenerate hybrid
configs with zero mamba layers and algorithms like "rocket" or "dsa" to
incorrectly raise ValueError instead of being converted to non-hybrid mode. Move
the sparse attention validation block that checks sparse_attn_config and
sparse_attn_algorithm to after the zero-mamba fallback handling (currently at
line 104), so that configs with zero mamba layers can be converted to non-hybrid
mode first, and then the validation will apply only to genuine hybrid
configurations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 79766918-b75e-453e-9969-a9d65f06e00b
📒 Files selected for processing (1)
tensorrt_llm/_torch/pyexecutor/_util.py
b4c3b44 to
1d92e23
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #54746 [ run ] triggered by Bot. Commit: |
|
PR_Github #54746 [ run ] completed with state
|
Signed-off-by: Bo Li <22713281+bobboli@users.noreply.github.com>
1d92e23 to
88aa079
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #54905 [ run ] triggered by Bot. Commit: |
|
PR_Github #54905 [ run ] completed with state
|
|
/bot run --reuse-test |
|
PR_Github #54930 [ run ] triggered by Bot. Commit: |
|
PR_Github #54930 [ run ] completed with state
|
|
Folded into #15417. |
Pull request was closed
Summary
sparse_attention_config.algorithmisskip_softmax.Skip Softmax only changes attention kernel execution. It should not replace the Mamba-capable cache manager needed for recurrent state.
Testing
pre-commit run --files tensorrt_llm/_torch/pyexecutor/_util.pypython3 -m py_compile tensorrt_llm/_torch/pyexecutor/_util.pyLocal pytest was not run because this local environment does not have
pytest/torchinstalled.Summary by CodeRabbit