Repository navigation
[Refactor] Enter draft TP scopes by attention ownership and drop ModelRunner.tp_group - #41817
Merged
Merged
Conversation
ch-wan
requested review from
Fridge003,
Qiaolin-Yu,
Ying1123,
hnyls2002,
iforgetmyname,
ispobock,
kpham-sgl,
merrymercy,
ping1jing2,
pyc96,
whybeyoung and
xiezhq-hermann
as code owners
September 30, 2026 03:07
This was referenced Sep 30, 2026
ch-wan
force-pushed
the
cheng/refactor/make-pp-layers
branch
from
September 30, 2026 04:24
ebb248e to
be4156e
Compare
ch-wan
requested review from
1am9trash,
Alisehen,
ByronHsu,
Duyi-Wang,
HaiShaw,
OrangeRedeng,
ShangmingCai,
YAMY1234,
b8zhong,
hebiao064,
hubertlu-tw,
kkHuang-amd,
mickqian,
mmangkad,
rainj-me,
sogalin,
yeahdongcn,
yhyang201 and
yuan-luo
as code owners
September 30, 2026 04:24
ch-wan
force-pushed
the
cheng/refactor/spec-draft-scope
branch
from
September 30, 2026 04:25
5353ac8 to
215ad7b
Compare
ch-wan
force-pushed
the
cheng/refactor/make-pp-layers
branch
from
September 30, 2026 05:06
be4156e to
e92827d
Compare
ch-wan
force-pushed
the
cheng/refactor/spec-draft-scope
branch
from
September 30, 2026 05:06
215ad7b to
233eec5
Compare
ch-wan
force-pushed
the
cheng/refactor/make-pp-layers
branch
from
September 30, 2026 20:28
e92827d to
cf8b583
Compare
Each speculative worker said whether its draft owns attention three times: `self.draft_owns_attention`, `self.draft_tp_context` (the real scope under DP attention, else a no-op), and the TP group every call site passed in, taken off the draft runner. `draft_tp_context` now takes only `owns_attention`: with it, the scope installs the target's attention-TP group, the group an owning draft is built on; without it, there is nothing to enter. The scope is always entered from outside a draft scope, where `get_parallel().attn_tp_group` is the group the owning draft was built on, so every scope installs the same group as before. For drafts that do not own attention under DP attention, the old call patched the TP group with the group already installed; that no-op scope is gone. DSpark's two scopes already had this shape and now use the same helper. The Standalone worker's attention-backend and CUDA-graph overrides only re-entered scopes its base class already enters, so they are gone. A unit test covers both cases.
With drafts entering their scope through `draft_tp_context(owns_attention)`, nothing reads a runner's TP group to enter a scope any more. Every remaining reader runs while the runner is built, during a graph capture or replay, during a forward, or only on the target: graph-runner barriers, the CUDA-graph backends (full, breakable, piecewise, NPU, XPU) that keep the group they capture with, the FlashInfer autotune sync group, `patch_model`, the elastic joiner and EP-join paths, and the PP+spec seed broadcast in `TpModelWorker`. They read `get_parallel().tp_group`, which in those scopes is the group the runner copied. `ModelRunner` now keeps only its PP group. Two readers can run on a draft outside its scope: - the HiSparse coordinator, built with the pools, reads the TP group only without DP attention, where no draft owns attention and a draft's TP group is the target's; - recapturing graphs after a weight update (`recapture_cuda_graph=True`) runs with no draft scope entered. The draft forward there already issues its collectives on the target's TP group; its capture barriers now use that group too, and every rank of it runs the recapture. The DFLASH and DSpark rank-0 gates that run inside the draft scope asked the target runner's TP group for the target's rank; the context there answers the draft's. Those workers now record the target's TP rank in their constructors, outside any draft scope, and read it there. Tests that faked a runner's TP group state it on the context instead: the speculative attention test kits, the full CUDA-graph backend test, the DFLASH sampler test, and the FlashInfer autotune cache-phase test.
ch-wan
force-pushed
the
cheng/refactor/spec-draft-scope
branch
from
September 30, 2026 20:28
233eec5 to
2a1a9b6
Compare
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.
This PR is part of a stack (oldest at bottom):
Motivation
Each speculative worker states three times whether its draft owns attention:
self.draft_owns_attention;self.draft_tp_context, which isdraft_tp_contextunder DP attention and a no-op otherwise;That last one is the only reason
ModelRunner.tp_grouphas to be a snapshot that outlives the scope the runner is built in.Modifications
draft_tp_context(owns_attention). It now takes onlyowns_attention.The 28 scopes in the EAGLE, DFLASH, frozen-KV MTP, multi-layer EAGLE, Standalone and DSpark workers call it with their ownership flag.
Why this installs the same group as before:
patch_tensor_parallel_groupdoes not nest. There,get_parallel().attn_tp_groupis the group the owning draft was built on.The old two-argument form of
draft_tp_contextis gone. A caller that still passes a group gets aTypeErrorrather than a different scope.ModelRunner.tp_groupis removed. With drafts entering their scope from their ownership flag, every remaining reader runs while the runner is built, during a graph capture or replay, during a forward, or only on the target. They readget_parallel().tp_group, which in those scopes is the group the runner copied:patch_model;TpModelWorker.ModelRunnerkeeps only its PP group. Code outside the tree that readsmodel_runner.tp_groupshould readget_parallel().tp_groupin the scope it runs in.Two readers can run on a draft outside its scope:
recapture_cuda_graph=True) runs with no draft scope entered. The draft forward there already issues its collectives on the target's TP group. Its capture barriers now use that group too, and every rank of the group runs the recapture.The DFLASH and DSpark rank-0 gates that run inside the draft scope used to ask the target runner's TP group for the target's rank; inside the scope, the context answers the draft's. Those workers now record the target's TP rank in their constructors, outside any draft scope.
Tests.
test_draft_tp_context.pycovers both ownership cases.Accuracy Tests
H200, Qwen3-8B,
--tp-size 2, 4 greedy prompts × 64 tokens and GSM8K (200 questions), compared withmainat the top of this stack:spec_verify_ctAngelSlim/Qwen3-8B_eagle3)Qwen/Qwen3-0.6B)For the Standalone draft, one prompt's
spec_verify_ctalternates between 20 and 21 across repeated runs ofmainitself, and the same happens on every layer of this stack.test/registered/unitat this PR's head, compared withmain: no new failures.Speed Tests and Profiling
Not applicable.
Checklist
Review and Merge Process
/tag-and-rerun-ci,/tag-run-ci-label,/rerun-failed-ciCI States
Latest PR Test (Base): 🚫 Run #36772917062
Latest PR Test (Extra): 🚫 Run #36772916590
Latest PR Test (AMD ROCm 10): ❌ Run #36772917133