[Bugfix] HybridAttnBackend declares forward() twice, killing the linear-attention path - #34303
Closed
noron12234 wants to merge 1 commit into
Closed
noron12234 wants to merge 1 commit into
noron12234 wants to merge 1 commit into
Conversation
noron12234
requested review from
Fridge003,
HaiShaw,
Qiaolin-Yu,
hebiao064,
ispobock and
merrymercy
as code owners
August 10, 2026 15:15
…ar-attention path HybridAttnBackend has two forward() definitions. The second one wins, so the first is dead: line 154 added by [Kimi] Support kimi-k3 (sgl-project#32541), 2026-08-04 accepts mixed_qkv / a / b and routes them to the linear-attention backend line 224 the original from [feat] Support different attention backends for prefill and decode, 2025-07-28 only forwards q / k / v sgl-project#32541 added the new forward() at the top of the class but did not remove the pre-existing one below, so Python binds the 2025 version. A caller passing mixed_qkv/a/b now lands in **kwargs and is handed to backend.forward() with q=k=v=None instead of taking the linear-attention branch. Drop the stale definition and keep the kimi-k3 one, which is a strict superset. Also add two unit tests to test_attention_backend_setup.py covering both dispatch paths, so a future duplicate definition fails CI instead of silently shadowing. Signed-off-by: Lin Junrong <noron12334@gmail.com>
noron12234
force-pushed
the
fix/hybrid-attn-duplicate-forward
branch
from
August 27, 2026 17:39
75a5700 to
d7df79d
Compare
Contributor
Author
|
Rebased onto current |
Contributor
Author
|
Closing this. I am cleaning up the PRs I have open and keeping only the ones a maintainer has already picked up; nobody has needed to look at this one, and leaving it open just adds to your queue. The branch stays on my fork, so nothing is lost if the fix turns out to be worth having. Sorry for the noise. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
HybridAttnBackenddeclaresforward()twice. Python binds the laterdefinition, so the earlier one is dead code:
q, k, v, layer, forward_batch, save_kv_cache, *, mixed_qkv, a, b, **kwargsq, k, v, layer, forward_batch, save_kv_cache, **kwargs#32541 added the linear-attention-aware
forward()at the top of the class butdid not remove the pre-existing one further down, so the 2025 definition is the
one that is actually bound.
The effect is that the linear-attention dispatch added by #32541 never runs.
A caller passing
mixed_qkv/a/b— ashybrid_linear_attn_backend.pydoes — falls into**kwargson the olddefinition and is then handed to
backend.forward()alongsideq=None, k=None, v=None, instead of taking theif mixed_qkv is not None:branch.Modifications
forward(). The kimi-k3 one is a strict superset: sameparameters for the regular path, plus the linear-attention branch.
test/registered/unit/model_executor/model_runner_components/test_attention_backend_setup.py(already a CPU test module, and it already imports
HybridAttnBackend)asserting that
forward()routesmixed_qkv/a/bto the selected backendand that the plain
q/k/vpath is unchanged. A future duplicatedefinition fails these instead of silently shadowing.
No behaviour change for the regular attention path; the only difference is
positional vs keyword argument passing to
backend.forward().Accuracy Tests
Not applicable — this restores a dispatch branch that is currently unreachable,
it does not change kernel or model math.
Verification
blackandisortclean on both touched files.I was not able to run the full test module locally: importing
sglang.srtonthis machine fails at
transformersconfig registration(
ValueError: 'qwen3_asr' is already used by a Transformers config), which is aversion-pinning issue in my environment and unrelated to this change. I
verified the dispatch by executing the real, unmodified
HybridAttnBackendclass body with stubbed imports:
Please run the added tests in CI.
Checklist
CI States
Latest PR Test (Base): ❌ Run #33099494579
Latest PR Test (Extra): ❌ Run #33099494294
Latest PR Test (AMD ROCm 7.2): ❌ Run #33099494501