Repository navigation
Check the topology identities where the layout is written, and build at the published widths - #40340
Merged
Merged
Conversation
ch-wan
requested review from
BBuf,
Edwardf0t1,
Fridge003,
HaiShaw,
ShangmingCai,
Ying1123,
ispobock,
liusy58,
merrymercy and
yizhang2077
as code owners
September 19, 2026 11:06
ch-wan
force-pushed
the
cheng/refactor/topology-identities
branch
from
September 19, 2026 20:29
209d8fd to
b433d52
Compare
ch-wan
requested review from
ByronHsu,
Duyi-Wang,
hnyls2002,
sogalin and
xiezhq-hermann
as code owners
September 19, 2026 20:29
ch-wan
force-pushed
the
cheng/refactor/topology-identities
branch
2 times, most recently
from
September 19, 2026 23:24
171a2a2 to
181497e
Compare
ch-wan
force-pushed
the
cheng/refactor/draft-scope-and-scheduler-reads
branch
from
September 20, 2026 09:18
2fe69c7 to
dd50d6a
Compare
ch-wan
force-pushed
the
cheng/refactor/topology-identities
branch
from
September 20, 2026 09:18
b1c32c1 to
9097ca2
Compare
ch-wan
force-pushed
the
cheng/refactor/topology-identities
branch
from
September 20, 2026 11:59
9097ca2 to
9ab25ed
Compare
ch-wan
force-pushed
the
cheng/refactor/draft-scope-and-scheduler-reads
branch
from
September 21, 2026 11:29
dd50d6a to
297fbc5
Compare
ch-wan
force-pushed
the
cheng/refactor/topology-identities
branch
3 times, most recently
from
September 21, 2026 12:56
2c6baf4 to
45fe302
Compare
ch-wan
force-pushed
the
cheng/refactor/draft-scope-and-scheduler-reads
branch
from
September 21, 2026 19:05
297fbc5 to
04e8f25
Compare
ch-wan
force-pushed
the
cheng/refactor/topology-identities
branch
from
September 21, 2026 19:11
45fe302 to
5a38ad3
Compare
Base automatically changed from
cheng/refactor/draft-scope-and-scheduler-reads
to
main
September 21, 2026 19:19
`override()` validated key names and nothing else, so a self-contradicting topology -- a rank at or past its width, a `tp_size` that does not factor into the attention widths, a group whose width is not the one configured -- went in quietly and surfaced much later as a hang or a wrong answer in a collective, with nothing pointing back at the write. The three identities now hold unconditionally on every path that writes the namespace and at the group build. The caller owns the arithmetic: stating one leaf without the quotients that follow from it describes no real layout, so it is refused rather than carried. Names that cannot be read are skipped -- a process that has published nothing can still stamp a rank, and a group that has not been built answers nothing at all. Two callers were stating a width the rest of the namespace disagreed with. The draft's shared-expert scope narrows `moe_ep_size` to one without sharding anything, so it now says there is no expert-parallel group rather than leaving the wider one installed. Publish stamped the placement in two calls, and the moment between them described a half-placed process; it stamps once.
`initialize_model_parallel` took seven widths as arguments, so every caller translated the published configuration into them again: the scheduler from the per-runner record, the weight-cache daemon from its own fields, the media encoder from neither. Three translations of one configuration is three places for it to come out different, and the encoder's did -- it built a layout the context never described. The widths now come from the context. What is left in the signature is not topology: the backend is decided by the device, two flags belong to other namespaces, and the join parameters describe this particular call rather than the layout being joined. The encoder states the layout it has always built -- `tp_size` ranks wide, no pipeline, no expert or MoE-DP dimension, no decode context parallelism -- so what it answers and what it builds are the same thing again. Tests that built a topology by passing widths now publish it first, through the same door production uses.
…sors The widths are quotients of one another in two more ways than the attention triple: `moe_tp_size` is what is left of `tp_size` after the expert and MoE-DP dimensions, and the attention ranks are derived from `tp_rank` through a fixed layout. Both relations already existed as one-directional arithmetic; stating them as identities means a write that contradicts either is refused where it happens. Making them hold turned up one caller that was not saying what it meant: the draft's tensor-parallel scope narrowed `tp_size` to the group it installs while leaving the MoE widths on the target's answers. The draft runs the whole model on that one group and has no expert dimension there -- the one worker that runs a MoE draft declines this scope for exactly that reason -- so the scope says so. With the group build now checking what it made against what was configured, `get_tensor_model_parallel_world_size` and its attention sibling answer the same question as the configured widths rather than a second one, so their eight readers move to the context. `get_shared_experts_tp_group` gets a context name and its one reader follows. A guard derives the accessor list from the source and fails on any caller outside the package that defines them; the three that remain are not topology and say why.
ch-wan
force-pushed
the
cheng/refactor/topology-identities
branch
from
September 21, 2026 19:22
5a38ad3 to
2a5ffc0
Compare
This was referenced Sep 24, 2026
hdt98
added a commit
to hdt98/sglang
that referenced
this pull request
Sep 26, 2026
The topology check from sgl-project#40340 rejects moe_ep_size=4 with tp_size=1. Same change as sgl-project#41002. Co-authored-by: xinguozhu-2026 <xinguo.zhu@intel.com>
This was referenced Sep 26, 2026
This was referenced Oct 5, 2026
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
get_parallel().override(...)validated key names and nothing else. Aself-contradicting topology -- a rank at or past its width, a
tp_sizethatdoes not factor into the attention widths, a group whose width is not the one
configured -- went in quietly and surfaced much later as a hang or a wrong
answer in a collective, with nothing pointing back at the write.
At the other end,
initialize_model_paralleltook seven widths as arguments,so every caller translated the published configuration into them again: the
scheduler from a per-runner record, the weight-cache daemon from its own
fields, the media encoder from neither. Three translations of one configuration
are three places for it to come out different, and the encoder's did -- it
built a layout the context never described.
Modifications
One set of identities, checked wherever the layout is written. Every rank
inside its width;
tp_sizefactoring into the attention triple and into theMoE triple; the attention rank layout the attention ranks are derived from; and
each built group's width equal to the configured one. They hold
unconditionally, on the four paths that write the namespace and at the group
build. The caller owns the arithmetic: stating one leaf without the quotients
that follow from it describes no real layout, so it is refused rather than
carried. Names that cannot be read are skipped -- a process that has published
nothing can still stamp a rank.
Two callers were stating a width the rest of the namespace disagreed with. The
shared-expert scope narrows
moe_ep_sizeto one without sharding anything, soit now says there is no expert-parallel group rather than leaving the wider one
installed. Publish stamped the placement in two calls, and the moment between
them described a half-placed process; it stamps once.
The group build reads the widths it builds at. The seven width parameters
are gone; the function reads them from the context. What is left in the
signature is not topology: the backend is decided by the device, two flags
belong to other namespaces, and the join parameters describe this particular
call. The encoder states the layout it has always built --
tp_sizerankswide, no pipeline, no expert or MoE-DP dimension, no decode context parallelism
-- so what it answers and what it builds are the same thing again.
An elastic scale-up and the launch topology stop sharing two names. A
scale-up admits ranks into a WORLD that was pre-allocated to
--max-ep-size;it does not rebuild the process groups, which keep the width they were
constructed with. Writing the expanded replica count over
attn_dp_sizeandattn_dp_ranktherefore left the launch names answering for groups of adifferent size, and the identities say so: the rank lands outside its width and
tp_sizestops factoring into the attention triple.elastic_dp_sizeandelastic_dp_rankname the expanded replica set, each answering with its launchcounterpart until a scale-up moves it, and the readers that need the expanded
one -- the WORLD gather width, DP padding, KV routing, every place a gathered
list is indexed by replica, and KV-event sharding -- ask for it by name. That
includes the gather and scatter slices themselves: the list a scale-up gathers
spans WORLD, so indexing it by this process's place among the launch replicas
selects another rank's rows. The pair is range-checked like any other; it stays out of the identities
that describe the groups, because it does not describe them.
A draft on one context shard has no context-parallel communicator, and the
sampler asks for one only when there is more than one shard to reconcile --
otherwise the scope's answer, that there is no such group, is dereferenced.
The draft scope states a group for each width it narrows. It already said its
attention-TP width is the group it installs; it now hands over that group, and
says there is no attention-CP group, the way it already said there is no
expert-parallel one. The shared-expert scope states
moe_dp_sizefor the samereason -- narrowing two of the three MoE factors and leaving the third is a
triple that does not multiply out.
With the build checking what it made against what was configured,
get_tensor_model_parallel_world_sizeand its attention sibling answer the samequestion as the configured widths rather than a second one, so their eight
readers move to the context.
Accuracy Tests
Not run for this revision. The build now reads the widths the previous code
passed it, and the encoder states the layout it already built, so no
configuration changes what is constructed.
Speed Tests and Profiling
Not applicable. The identity check is integer comparison on names already in
hand; it runs where a topology is written, not in a forward.
Checklist
Each identity is injected in the direction that breaks it and in the direction
that keeps it, because a guard that only ever fires is as uninformative as one
that never does. Twenty-one tests that built a topology by passing widths
publish it first, and ten benchmark and example entries do the same. The
scale-up test now publishes a configuration before it asserts: without one
every term was unreadable and the check it claimed to make was skipped.
Review and Merge Process
/tag-and-rerun-ci,/tag-run-ci-label,/rerun-failed-ciCI States
Latest PR Test (Base): ❌ Run #35644382491
Latest PR Test (Extra): ❌ Run #35644381975
Latest PR Test (AMD ROCm 10): ❌ Run #35644382214