[Config] Round 6.5: a namespace declares what it derives, next to what it derives it from (#38113)
Fifth of five; stacked on #38049. The split gave every namespace a file, but only for the half an operator types. This is the other half. ## The parallel quotients are declared, not written out `attn_tp_size` and its five siblings were sixty lines of near-identical properties in the runtime context, a file away from the leaves they are quotients of, so reading `parallel.py` told you what you could set and nothing about what that decides. They are declared in `Parallel` now, in the same class as those leaves. They carry no annotation, so they are not dataclass fields and `collect_input_fields` never puts them on the record -- the same mechanism that already keeps `_NS_PATH` off it. That is the right exclusion: a quotient has no operator input to preserve, and the record is what crosses a process boundary, where a stamped width is one an elastic scale-up will not refresh. ## A quotient is a value in the bag, like every other derived one `_derived_width` answered from a stamp or, failing that, a live process group. The group read could never disagree with the stamp: - `initialize_model_parallel` stamps all six as its last statement, unconditionally; - an elastic scale-up restamps `attn_dp_size` through `update_dp_attention_post_scale` -- the comment claiming it does *not* was wrong; - no hardware backend builds groups of its own; - `multimodal_gen`, which has its own `initialize_model_parallel` and does not stamp, never reads a quotient. So a built group was always already stamped, and the group read goes -- and with it the last reason for a quotient to be resolved on every read. Every input to `derive_parallel_widths` is a record field. `dcp_enabled` is `decode_context_parallel_size > 1`, not a fact about a built group; it was spelled `_DCP is not None`, which is a longer way to say the same thing. So the six are fixed once the configuration is fixed -- the same test every other `Derived(fn=...)` in this PR passes. They are declared the same way and computed the same way: once, at publish, into ordinary bag leaves. What remains is override -> stamp -> published leaf. The stamp stays above the leaf because an elastic scale-up restamps `attn_dp_size`; the override stays on top because that is how a test names a width. ## One answer for the config-derived predicates `enable_mamba_extra_buffer` and its lazy variant, `is_ep_joiner`, `is_ep_scale_joiner`, `is_startup_weight_load_overlap`: each existed as a `ServerArgs` member for the resolution pipeline and, for most of them, again as a `runtime_context` function for readers after publish. Three places to keep saying the same thing. A `Derived(fn=...)` is a pure function of the published configuration, so `publish` computes it once and stores it as an ordinary bag leaf -- a plain attribute load, which is what a read inside compiled model code needs. The function is handed the whole resolved config rather than the bag it lands in, because a derivation is free to span namespaces and the mamba one does: it reads `memory.disable_radix_cache` alongside its own `exec.mamba` strategy, which is why it could never have been a method on either bag. The pre-publish helpers stay -- resolution needs the predicate before there is a bag to read -- and three readers keep them, because they run before their own process publishes: `initialize_dp_attention`, which the weight-cache daemon calls while building its groups thirty lines before its `publish`, and `PortArgs.init_new`, a factory handed the record that already reads eighteen other fields off it. ## Notes for a reviewer **Overriding a leaf does not move its quotient.** `override(tp_size=2)` leaves `attn_tp_size` where the published config put it, because nothing is recomputed on read. A test states a topology by publishing a config -- which is what a real process does -- or by naming the width it wants, `override(attn_tp_size=2)`. Six tests say it that way now. This is the price of having one answer computed once, and it is the same price every other derived value in the config already carries. A caller that reads a quotient without publishing or overriding now gets an explicit error naming the field, instead of a default that an uninitialised group happened to supply. One fixture was in that state -- `TestMlaWriteDoorsUnderDcp` built a bare pool and asked whether DCP was on -- and it publishes a config now, which is what the process it stands in for does. Eighteen sites read these predicates without calling them. That is correct -- they are properties -- but it is worth saying they were checked, because a census that assumes otherwise reports eighteen always-true conditions. ## The skill that documents this subsystem is updated with it `.claude/rules/modify-component-must-read.md` points at `.claude/skills/sglang-runtime-context/SKILL.md` before anyone touches these files, so a stale sentence there is a wrong instruction rather than a stale note. Four of its load-bearing statements stopped being true across this series and are corrected here: `NS(...)` is no longer how a field states its namespace (the declaring class is); the DCP degrade rule is gone, because the quotients are not live reads; `mamba_extra_buffer_enabled()` and the other predicate functions it named as the shape to copy no longer exist; and the namespace-coverage ratchet is described in terms of the marker. The docstring of `test_server_args_namespaces.py` said the same thing and is fixed too. The consequence a test author actually trips over is stated there as well: overriding a leaf no longer moves its quotient, so a topology is stated by publishing a config or by naming the width. ## Verification A full registered-unit sweep (648 files) against this stack's merge-base: 19 failures on both sides, the same 19 -- AMD `gfx950`, `modelopt`, `cuda_vmm`, `weight_checker` and friends, none of them config. The narrower 139-file config sweep used earlier in this series does not contain the files this change reaches -- `test_kv_index_translator` never names `get_parallel()`, it constructs an object that does -- which is why the baseline differential over everything is what is quoted here.
This commit is contained in:
Cheng Wan
@@ -53,21 +53,22 @@ _SRT = _pathlib.Path(next(iter(_sglang.__path__))).resolve() / "srt"
|
||||
_PS = "sglang.srt.distributed.parallel_state"
|
||||
_DP = "sglang.srt.layers.dp_attention"
|
||||
|
||||
# Ranks and the world size read the live group: they are not implied by
|
||||
# anything, so there is nothing to derive them from. The quotients used to be
|
||||
# in this table and are not any more -- `attn_tp_size` and its siblings are
|
||||
# functions of the configured leaves, and `TestDerivedWidthsComeFromTheLeaves`
|
||||
# is what pins them.
|
||||
SIZE_RANK_DELEGATIONS = [
|
||||
("world_size", f"{_PS}.get_world_size"),
|
||||
("world_rank", f"{_PS}.get_world_rank"),
|
||||
("tp_rank", f"{_PS}.get_tensor_model_parallel_rank"),
|
||||
("dcp_rank", f"{_PS}.get_dcp_rank"),
|
||||
("pp_rank", f"{_PS}.get_pipeline_model_parallel_rank"),
|
||||
("moe_ep_size", f"{_PS}.get_moe_expert_parallel_world_size"),
|
||||
("moe_ep_rank", f"{_PS}.get_moe_expert_parallel_rank"),
|
||||
("moe_dp_rank", f"{_PS}.get_moe_data_parallel_rank"),
|
||||
("moe_tp_size", f"{_PS}.get_moe_tensor_parallel_world_size"),
|
||||
("moe_tp_rank", f"{_PS}.get_moe_tensor_parallel_rank"),
|
||||
("attn_tp_size", f"{_PS}.get_attn_tensor_model_parallel_world_size"),
|
||||
("attn_tp_rank", f"{_PS}.get_attn_tensor_model_parallel_rank"),
|
||||
("attn_cp_rank", f"{_PS}.get_attn_context_model_parallel_rank"),
|
||||
("attn_dp_size", f"{_DP}.get_attention_dp_size"),
|
||||
("attn_dp_rank", f"{_DP}.get_attention_dp_rank"),
|
||||
]
|
||||
|
||||
@@ -176,36 +177,49 @@ class TestParallelOverride(_IsolatedOverrides):
|
||||
|
||||
|
||||
class TestParallelDCP(_IsolatedOverrides):
|
||||
def test_attn_dcp_defaults_when_group_is_uninitialized(self):
|
||||
"""The DCP width is a quotient; the DCP rank is a live reading.
|
||||
|
||||
They used to be tested the same way, by mocking the group getters, because
|
||||
the width read the group too. It does not: `attn_dcp_size` is
|
||||
`dcp_size if dcp_enabled else 1`, so the way to state it is to state the
|
||||
leaves.
|
||||
"""
|
||||
|
||||
def _published(self, **fields):
|
||||
reset_context()
|
||||
self.addCleanup(reset_context)
|
||||
publish(ServerArgs(model_path="dummy", **fields), role="test")
|
||||
return get_parallel()
|
||||
|
||||
def test_attn_dcp_is_one_when_dcp_is_off(self):
|
||||
parallel = self._published(tp_size=8, dcp_size=1)
|
||||
self.assertFalse(parallel.dcp_enabled)
|
||||
self.assertEqual(parallel.attn_dcp_size, 1)
|
||||
|
||||
def test_attn_dcp_is_the_configured_width_when_on(self):
|
||||
parallel = self._published(tp_size=8, dcp_size=8)
|
||||
self.assertTrue(parallel.dcp_enabled)
|
||||
self.assertEqual(parallel.attn_dcp_size, 8)
|
||||
|
||||
def test_the_dcp_rank_still_reads_the_group(self):
|
||||
"""A rank is not implied by the configuration, so it reads the group --
|
||||
gated on a width that is."""
|
||||
with (
|
||||
patch(f"{_PS}.get_dcp_group_no_assert", return_value=None),
|
||||
patch(f"{_PS}.get_dcp_world_size", side_effect=AssertionError),
|
||||
get_parallel().override(tp_size=8, dcp_size=8, dcp_enabled=False),
|
||||
patch(f"{_PS}.get_dcp_rank", side_effect=AssertionError),
|
||||
):
|
||||
self.assertFalse(get_parallel().dcp_enabled)
|
||||
self.assertEqual(get_parallel().attn_dcp_size, 1)
|
||||
self.assertEqual(get_parallel().attn_dcp_rank, 0)
|
||||
|
||||
def test_attn_dcp_delegates_when_enabled(self):
|
||||
with (
|
||||
patch(f"{_PS}.get_dcp_group_no_assert", return_value=object()),
|
||||
patch(f"{_PS}.get_dcp_world_size", return_value=8),
|
||||
get_parallel().override(tp_size=8, dcp_size=8, dcp_enabled=True),
|
||||
patch(f"{_PS}.get_dcp_rank", return_value=3),
|
||||
):
|
||||
self.assertTrue(get_parallel().dcp_enabled)
|
||||
self.assertEqual(get_parallel().attn_dcp_size, 8)
|
||||
self.assertEqual(get_parallel().attn_dcp_rank, 3)
|
||||
|
||||
def test_dcp_enablement_is_platform_agnostic(self):
|
||||
with (
|
||||
patch(f"{_PS}.get_dcp_group_no_assert", return_value=object()),
|
||||
patch("sglang.srt.utils.is_cuda", return_value=False) as is_cuda,
|
||||
patch(f"{_PS}.get_dcp_world_size", return_value=8),
|
||||
patch(f"{_PS}.get_dcp_rank", return_value=3),
|
||||
):
|
||||
self.assertTrue(get_parallel().dcp_enabled)
|
||||
self.assertEqual(get_parallel().attn_dcp_size, 8)
|
||||
self.assertEqual(get_parallel().attn_dcp_rank, 3)
|
||||
def test_the_width_does_not_consult_the_platform(self):
|
||||
with patch("sglang.srt.utils.is_cuda", return_value=False) as is_cuda:
|
||||
parallel = self._published(tp_size=8, dcp_size=8)
|
||||
self.assertTrue(parallel.dcp_enabled)
|
||||
self.assertEqual(parallel.attn_dcp_size, 8)
|
||||
is_cuda.assert_not_called()
|
||||
|
||||
|
||||
@@ -1150,27 +1164,32 @@ class TestDerivedPredicatesAgreeAcrossTiers(_IsolatedServerArgs):
|
||||
|
||||
_STRATEGIES = ("auto", "no_buffer", "extra_buffer", "extra_buffer_lazy")
|
||||
|
||||
def test_mamba_extra_buffer_matches_the_member(self):
|
||||
from sglang.srt.runtime_context import (
|
||||
mamba_extra_buffer_enabled,
|
||||
mamba_extra_buffer_lazy_enabled,
|
||||
)
|
||||
|
||||
def test_the_mamba_extra_buffer_predicate_has_one_answer(self):
|
||||
"""It used to be asserted that two spellings agreed. There is one now:
|
||||
the declaration computes it at publish, and the bag carries it."""
|
||||
for disable_radix_cache in (False, True):
|
||||
for strategy in self._STRATEGIES:
|
||||
with self.subTest(radix=disable_radix_cache, strategy=strategy):
|
||||
args = _FakeResolvedArgs(
|
||||
disable_radix_cache=disable_radix_cache,
|
||||
mamba_radix_cache_strategy=strategy,
|
||||
reset_context()
|
||||
publish(
|
||||
ServerArgs(
|
||||
model_path="dummy",
|
||||
disable_radix_cache=disable_radix_cache,
|
||||
mamba_radix_cache_strategy=strategy,
|
||||
),
|
||||
role="test",
|
||||
)
|
||||
get_context().set_server_args(args)
|
||||
self.assertEqual(
|
||||
ServerArgs.enable_mamba_extra_buffer(args),
|
||||
mamba_extra_buffer_enabled(),
|
||||
expected = disable_radix_cache is False and strategy in (
|
||||
"extra_buffer",
|
||||
"extra_buffer_lazy",
|
||||
)
|
||||
self.assertEqual(
|
||||
ServerArgs.enable_mamba_extra_buffer_lazy(args),
|
||||
mamba_extra_buffer_lazy_enabled(),
|
||||
get_exec().mamba.enable_mamba_extra_buffer, expected
|
||||
)
|
||||
self.assertEqual(
|
||||
get_exec().mamba.enable_mamba_extra_buffer_lazy,
|
||||
disable_radix_cache is False
|
||||
and strategy == "extra_buffer_lazy",
|
||||
)
|
||||
|
||||
def test_prefill_buffer_ceiling_matches_the_member(self):
|
||||
@@ -1446,6 +1465,62 @@ class TestDerivedWidths(_IsolatedOverrides):
|
||||
)
|
||||
)
|
||||
|
||||
def test_the_published_configuration_decides_the_widths(self):
|
||||
"""The quotients are computed once, at publish, from the leaves.
|
||||
|
||||
Every input is a record field, so there is nothing to recompute on a
|
||||
read: `publish` fills the bag and the bag is the answer.
|
||||
"""
|
||||
reset_context()
|
||||
self.addCleanup(reset_context)
|
||||
publish(
|
||||
ServerArgs(
|
||||
model_path="dummy", tp_size=8, dp_size=2, enable_dp_attention=True
|
||||
),
|
||||
role="test",
|
||||
)
|
||||
self.assertEqual(get_parallel().attn_tp_size, 4)
|
||||
self.assertEqual(get_parallel().attn_dp_size, 2)
|
||||
self.assertEqual(get_parallel().moe_tp_size, 8)
|
||||
|
||||
reset_context()
|
||||
publish(
|
||||
ServerArgs(model_path="dummy", tp_size=8, ep_size=4, moe_dp_size=2),
|
||||
role="test",
|
||||
)
|
||||
self.assertEqual(get_parallel().moe_tp_size, 1)
|
||||
|
||||
def test_a_topology_is_stated_by_naming_the_width(self):
|
||||
"""Overriding a leaf does not move the quotient -- the quotient is not
|
||||
recomputed on read. Naming it is how a test states one."""
|
||||
reset_context()
|
||||
self.addCleanup(reset_context)
|
||||
publish(ServerArgs(model_path="dummy", tp_size=8), role="test")
|
||||
self.assertEqual(get_parallel().attn_tp_size, 8)
|
||||
with get_parallel().override(tp_size=2):
|
||||
self.assertEqual(get_parallel().attn_tp_size, 8)
|
||||
with get_parallel().override(attn_tp_size=4):
|
||||
self.assertEqual(get_parallel().attn_tp_size, 4)
|
||||
|
||||
def test_an_unstated_topology_still_fails(self):
|
||||
"""Neutral leaves are for the dimensions a caller is not using, not for
|
||||
a caller that stated nothing: every width would come back 1, which is a
|
||||
plausible-looking number invented out of nothing."""
|
||||
with self.assertRaises(RuntimeError) as caught:
|
||||
get_parallel().attn_tp_size
|
||||
self.assertIn("not available", str(caught.exception))
|
||||
|
||||
def test_a_stamp_and_a_live_group_both_win_over_the_leaves(self):
|
||||
"""Order is stamp, then live group, then the leaves. Where a group
|
||||
exists it is the truth -- elastic scale-up moves the group without
|
||||
restamping -- so the leaf derivation only answers where there is none.
|
||||
"""
|
||||
parallel = get_parallel()
|
||||
parallel.stamp_derived_widths(attn_tp_size=7)
|
||||
self.addCleanup(parallel.clear_derived_widths)
|
||||
with parallel.override(tp_size=8, attn_dp_size=2):
|
||||
self.assertEqual(parallel.attn_tp_size, 7)
|
||||
|
||||
def test_the_quotients_come_from_the_leaves(self):
|
||||
widths = derive_parallel_widths(
|
||||
tp_size=8,
|
||||
@@ -1497,10 +1572,24 @@ class TestDerivedWidths(_IsolatedOverrides):
|
||||
self.assertEqual(parallel.attn_tp_size, 1)
|
||||
self.assertEqual(parallel.attn_tp_size, 4)
|
||||
|
||||
def test_without_a_stamp_the_live_group_still_answers(self):
|
||||
"""A process that installed groups by hand keeps working."""
|
||||
with patch(f"{_PS}.get_attn_tensor_model_parallel_world_size", return_value=2):
|
||||
self.assertEqual(get_parallel().attn_tp_size, 2)
|
||||
def test_the_group_is_never_consulted(self):
|
||||
"""There is no third source. A quotient comes from an override, a stamp
|
||||
or the published leaf -- never from a group coordinator, which could
|
||||
only ever agree, since `initialize_model_parallel` stamps as its last
|
||||
statement."""
|
||||
reset_context()
|
||||
self.addCleanup(reset_context)
|
||||
with patch(
|
||||
f"{_PS}.get_attn_tensor_model_parallel_world_size",
|
||||
side_effect=AssertionError("the group must not be consulted"),
|
||||
):
|
||||
publish(
|
||||
ServerArgs(
|
||||
model_path="dummy", tp_size=8, dp_size=2, enable_dp_attention=True
|
||||
),
|
||||
role="test",
|
||||
)
|
||||
self.assertEqual(get_parallel().attn_tp_size, 4)
|
||||
|
||||
def test_with_neither_the_failure_names_the_cause(self):
|
||||
with patch(
|
||||
@@ -1533,24 +1622,23 @@ class TestDerivedWidths(_IsolatedOverrides):
|
||||
parallel.stamp_derived_widths(attn_dp_size=4)
|
||||
self.assertEqual(parallel.attn_dp_size, 4)
|
||||
parallel.clear_derived_widths()
|
||||
with patch(f"{_DP}.get_attention_dp_size", return_value=1):
|
||||
with parallel.override(tp_size=8, attn_dp_size=1):
|
||||
self.assertEqual(parallel.attn_dp_size, 1)
|
||||
|
||||
def test_reset_context_drops_the_stamp(self):
|
||||
"""The stamp belongs to the lifecycle that made it.
|
||||
|
||||
`_derived_width` prefers the stamp over the live group, so a stamp that
|
||||
outlived `reset_context()` would let the next test read the previous
|
||||
topology.
|
||||
`_derived_width` prefers the stamp over the published leaf, so a stamp
|
||||
that outlived `reset_context()` would let the next test read the
|
||||
previous topology.
|
||||
"""
|
||||
from sglang.srt.runtime_context import reset_context
|
||||
|
||||
parallel = get_parallel()
|
||||
parallel.stamp_derived_widths(attn_tp_size=4)
|
||||
self.assertEqual(parallel.attn_tp_size, 4)
|
||||
reset_context()
|
||||
with patch(f"{_PS}.get_attn_tensor_model_parallel_world_size", return_value=1):
|
||||
self.assertEqual(get_parallel().attn_tp_size, 1)
|
||||
self.addCleanup(reset_context)
|
||||
publish(ServerArgs(model_path="dummy", tp_size=1), role="test")
|
||||
self.assertEqual(get_parallel().attn_tp_size, 1)
|
||||
|
||||
def test_the_arithmetic_has_one_home(self):
|
||||
"""`parallel_state` builds its groups from the same dict it stamps, and
|
||||
@@ -1589,5 +1677,67 @@ class TestDerivedWidths(_IsolatedOverrides):
|
||||
self.assertEqual(attn_dp_size, widths["attn_dp_size"])
|
||||
|
||||
|
||||
class TestTheDerivedHalfIsDeclared(CustomTestCase):
|
||||
"""The quotients are declared beside the leaves, in the same class.
|
||||
|
||||
A namespace is one file and one class. `Parallel` says both what an
|
||||
operator can set and what that decides; the quotients are unannotated, so
|
||||
they are not dataclass fields and never reach the record.
|
||||
`ParallelContext` installs a property per declaration rather than carrying
|
||||
its own list, so the two cannot drift.
|
||||
"""
|
||||
|
||||
def test_every_declared_quotient_has_a_property(self):
|
||||
from sglang.srt.arg_groups.arg_utils import Derived
|
||||
from sglang.srt.arg_groups.fields.parallel import Parallel
|
||||
|
||||
declared = {
|
||||
name for name, value in vars(Parallel).items() if isinstance(value, Derived)
|
||||
}
|
||||
self.assertTrue(declared, "the derived half is empty")
|
||||
for name in declared:
|
||||
self.assertIsInstance(
|
||||
getattr(type(get_context().parallel), name, None),
|
||||
property,
|
||||
f"{name} is declared but no property was installed",
|
||||
)
|
||||
|
||||
def test_the_declared_set_is_what_derive_parallel_widths_produces(self):
|
||||
"""The declaration is not a second list to keep in step: it names
|
||||
exactly the quotients the derivation returns."""
|
||||
from sglang.srt.arg_groups.arg_utils import Derived
|
||||
from sglang.srt.arg_groups.fields.parallel import Parallel
|
||||
|
||||
declared = {
|
||||
name for name, value in vars(Parallel).items() if isinstance(value, Derived)
|
||||
}
|
||||
produced = set(
|
||||
derive_parallel_widths(
|
||||
tp_size=8,
|
||||
attn_cp_size=1,
|
||||
attn_dp_size=2,
|
||||
moe_ep_size=1,
|
||||
moe_dp_size=1,
|
||||
dcp_size=1,
|
||||
dcp_enabled=False,
|
||||
)
|
||||
)
|
||||
self.assertEqual(declared, produced)
|
||||
|
||||
def test_a_declared_quotient_is_not_a_record_field(self):
|
||||
"""It has no operator input to preserve, and the record is what crosses
|
||||
a process boundary."""
|
||||
import dataclasses
|
||||
|
||||
from sglang.srt.arg_groups.arg_utils import Derived
|
||||
from sglang.srt.arg_groups.fields.parallel import Parallel
|
||||
from sglang.srt.server_args import ServerArgs
|
||||
|
||||
fields = {f.name for f in dataclasses.fields(ServerArgs)}
|
||||
for name, value in vars(Parallel).items():
|
||||
if isinstance(value, Derived):
|
||||
self.assertNotIn(name, fields)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
Reference in New Issue
Block a user