diff --git a/python/sglang/srt/arg_groups/arg_utils.py b/python/sglang/srt/arg_groups/arg_utils.py index 17e4235af..5c29992cd 100644 --- a/python/sglang/srt/arg_groups/arg_utils.py +++ b/python/sglang/srt/arg_groups/arg_utils.py @@ -39,6 +39,7 @@ annotation is equivalent to ``Arg(help=that_string)``. from __future__ import annotations +import copy import dataclasses import functools import types @@ -58,6 +59,20 @@ from typing import ( A = Annotated +class _NoFallback: + """Sentinel for ``Arg.fallback``: this field declares none. + + ``None`` cannot serve, because ``None`` is what a field *holds* when the + operator did not type it -- the state a fallback answers for. + """ + + def __repr__(self) -> str: # pragma: no cover - debugging aid + return "" + + +NO_FALLBACK = _NoFallback() + + @dataclasses.dataclass(frozen=True) class Arg: """CLI argument metadata attached to a dataclass field via ``Annotated``.""" @@ -80,6 +95,14 @@ class Arg: # `resolution_result` and the config bags answer with the decision. The # field keeps what the operator passed. resolvable: bool = False + # What the field means when nobody said anything -- the bottom of the read + # chain: override, decision, input, then this. Not the dataclass default, + # which stays `None` because that is how the record spells "not typed". + # + # Only a value fixed for the life of the configuration belongs here. One + # that depends on the machine, on another field, or on anything impure is a + # decision, and decisions stay in a hook where their order is visible. + fallback: Any = NO_FALLBACK @dataclasses.dataclass(frozen=True) @@ -145,6 +168,47 @@ def resolvable_fields(cls) -> frozenset: return frozenset(names) +@functools.cache +def fallbacks_of(cls) -> dict: + """``{field_name: value}`` for every field of ``cls`` that declares one. + + Read the same way `resolvable_fields` reads its flag, so a fallback lives + beside the help text of the field it belongs to rather than in whatever + hook used to fill it in. + """ + if not dataclasses.is_dataclass(cls): + return {} + hints = get_type_hints(cls, include_extras=True) + out = {} + for field in dataclasses.fields(cls): + _, arg = _unwrap_annotated(hints.get(field.name, field.type)) + if arg is not None and arg.fallback is not NO_FALLBACK: + out[field.name] = arg.fallback + return out + + +def with_fallback(cls, name: str, value: Any) -> Any: + """``value``, or the declared fallback when nothing has answered. + + `resolution_result` calls this as its last step -- the effective surface, + which the config bags and `/server_info` read through. Deliberately not the + views a pass reads *while deciding*: `model_overrides/inkling.py` branches + on `if cfg.swa_full_tokens_ratio is None`, and a fallback answering there + would make that branch dead. `test_declared_fallbacks.py` pins both halves. + + A container fallback is copied, for the reason a dataclass spells this + `default_factory`. + """ + if value is not None: + return value + fallback = fallbacks_of(cls).get(name, NO_FALLBACK) + if fallback is NO_FALLBACK: + return value + if isinstance(fallback, (list, dict, set)): + return copy.deepcopy(fallback) + return fallback + + # --------------------------------------------------------------------------- # Internal helpers # --------------------------------------------------------------------------- diff --git a/python/sglang/srt/arg_groups/kv_cache_hook.py b/python/sglang/srt/arg_groups/kv_cache_hook.py index bb228e6fc..2a1093325 100644 --- a/python/sglang/srt/arg_groups/kv_cache_hook.py +++ b/python/sglang/srt/arg_groups/kv_cache_hook.py @@ -10,6 +10,7 @@ from sglang.srt.arg_groups.overrides import ( attention_backends_of, declare_resolution, model_config_of, + resolution_result, resolved_view, resolving_view, use_mla_backend, @@ -189,7 +190,9 @@ def handle_cache_compatibility(server_args: Any) -> None: # Validate the effective ratio: model branches may declare a reset # (e.g. Step3p forces 1.0 under hierarchical cache) that supersedes # the user input before it ever takes effect. - if not (0 < resolved_view(server_args).swa_full_tokens_ratio <= 1.0): + # `resolution_result`, not a view: a view answers `None` while nobody has + # claimed the field, and the value to range-check is the effective one. + if not (0 < resolution_result(server_args, "swa_full_tokens_ratio") <= 1.0): raise ValueError("--swa-full-tokens-ratio should be in range (0, 1.0].") diff --git a/python/sglang/srt/arg_groups/model_overrides/deepseek_v4.py b/python/sglang/srt/arg_groups/model_overrides/deepseek_v4.py index 3aa0c4711..a289c992e 100644 --- a/python/sglang/srt/arg_groups/model_overrides/deepseek_v4.py +++ b/python/sglang/srt/arg_groups/model_overrides/deepseek_v4.py @@ -24,7 +24,6 @@ def _deepseek_v4_overrides(server_args: Any, hf_config: Any) -> dict: writes, the max_running_requests fill and the validations stay in the hook at its legacy slot.""" cfg = resolving_view(server_args) - from sglang.srt.server_args import ServerArgs model_arch = hf_config.architectures[0] overrides: Dict[str, Any] = {"attention_backend": "dsv4"} @@ -44,7 +43,7 @@ def _deepseek_v4_overrides(server_args: Any, hf_config: Any) -> dict: f"Use dsv4 attention backend for {model_arch}, setting page_size to {page_size}." ) - if cfg.swa_full_tokens_ratio == ServerArgs.swa_full_tokens_ratio: + if cfg.swa_full_tokens_ratio is None: overrides["swa_full_tokens_ratio"] = 0.1 logger.info(f"Setting swa_full_tokens_ratio to 0.1 for {model_arch}.") diff --git a/python/sglang/srt/arg_groups/model_overrides/inkling.py b/python/sglang/srt/arg_groups/model_overrides/inkling.py index 41a3482e0..10182e766 100644 --- a/python/sglang/srt/arg_groups/model_overrides/inkling.py +++ b/python/sglang/srt/arg_groups/model_overrides/inkling.py @@ -27,14 +27,14 @@ def _inkling_overrides(server_args: Any, hf_config: Any) -> dict: tree (which Inkling requires — models/inkling.py asserts it). The full-graph prefill default is set separately (inline, before cuda-graph resolution) — see ServerArgs.__post_init__ / _apply_inkling_prefill_cuda_graph_default. The - server-arg defaults each yield to an explicit user value (compared against - the ServerArgs class default); the prefill declaration is materialized + server-arg defaults each yield to an explicit user value (the ratios read + `None`, the mamba strategy its "auto" token); the prefill declaration is + materialized before _parse_cuda_graph_config folds cuda_graph_backend_prefill into prefill.backend, and an explicit --cuda-graph-backend-prefill / --disable-prefill-cuda-graph still wins. The unified-radix env write follows the MiniMax-M3 handler precedent (env is not a resolvable server-arg).""" cfg = resolving_view(server_args) - from sglang.srt.server_args import ServerArgs overrides: Dict[str, Any] = {} # NOTE: the full-graph prefill default is NOT set here. cuda-graph config is @@ -42,21 +42,21 @@ def _inkling_overrides(server_args: Any, hf_config: Any) -> dict: # cuda_graph_backend_prefill declared here lands too late (the breakable # default would already have been auto-disabled for this multimodal arch). # It is set inline before _handle_cuda_graph_config instead. - if cfg.swa_full_tokens_ratio == ServerArgs.swa_full_tokens_ratio: + if cfg.swa_full_tokens_ratio is None: overrides["swa_full_tokens_ratio"] = 0.1 - if cfg.mamba_full_memory_ratio == ServerArgs.mamba_full_memory_ratio: + if cfg.mamba_full_memory_ratio is None: overrides["mamba_full_memory_ratio"] = 0.1 # Inkling requires the extra-buffer mamba strategy (inkling.py asserts # enable_mamba_extra_buffer()); the generic "auto" resolution does not cover # Inkling, so pin it here. Yields to an explicit --mamba-scheduler-strategy. # - # The default comparison answers "unset" only while nothing has declared the - # field first. `_mamba_radix_cache_resolution` would, from the slot just - # above `collect_model_override_declarations`, for an architecture whose + # Compared against the unresolved token rather than the class default: the + # default only answers "unset" while nothing has declared the field first, + # and `_mamba_radix_cache_resolution` would -- from the slot just above + # `collect_model_override_declarations` -- for an architecture whose # linear-attention spec sets `uses_mamba_radix_cache`. Inkling has no such - # spec; giving it one silently stops this pin from firing, so compare - # against the unresolved token ("auto") if that day comes. - if cfg.mamba_radix_cache_strategy == ServerArgs.mamba_radix_cache_strategy: + # spec today, but giving it one would silently stop this pin from firing. + if cfg.mamba_radix_cache_strategy == "auto": overrides["mamba_radix_cache_strategy"] = "extra_buffer" # Inkling attention runs only on the fa4 (Blackwell) or triton backends -- # models/inkling_common/attn.py asserts attention_backend in {fa4, triton}. diff --git a/python/sglang/srt/arg_groups/overrides.py b/python/sglang/srt/arg_groups/overrides.py index 070c67ddb..8cf29de05 100644 --- a/python/sglang/srt/arg_groups/overrides.py +++ b/python/sglang/srt/arg_groups/overrides.py @@ -41,7 +41,11 @@ import math from typing import Any, Callable, Dict, List, Optional, Sequence, Tuple from sglang.srt.arg_groups import model_override_base -from sglang.srt.arg_groups.arg_utils import field_names, resolvable_fields +from sglang.srt.arg_groups.arg_utils import ( + field_names, + resolvable_fields, + with_fallback, +) # Re-exported for the callers that already import these names from here; the # declarations under ``model_overrides/`` import them from the base directly. @@ -268,7 +272,13 @@ def declare_direct_writes( def resolution_result(server_args: Any, field: str, default: Any = None) -> Any: """What resolution decided for ``field``: the declaration if there is one, - otherwise what the caller supplied. + otherwise what the caller supplied, otherwise the field's declared + fallback. + + The fallback is last because it is what the field means when nobody said + anything -- an operator who types a value and a pass that decides one both + sit above it. It is read from the declaration rather than filled in by a + pass, so there is no slot to place and no second call to make idempotent. This is what the config projection reads. Reading the field instead would work whatever the caller passed onto the record -- and @@ -283,8 +293,8 @@ def resolution_result(server_args: Any, field: str, default: Any = None) -> Any: return declared[field] raw = getattr(server_args, "_raw_input", None) if raw is not None and field in raw: - return raw[field] - return getattr(server_args, field, default) + return with_fallback(type(server_args), field, raw[field]) + return with_fallback(type(server_args), field, getattr(server_args, field, default)) def resolution_projection(server_args: Any) -> Dict[str, Any]: diff --git a/python/sglang/srt/server_args.py b/python/sglang/srt/server_args.py index e6c003fe6..b433effcf 100644 --- a/python/sglang/srt/server_args.py +++ b/python/sglang/srt/server_args.py @@ -815,7 +815,7 @@ class ServerArgs: NS("schedule"), ] = 16 swa_full_tokens_ratio: A[ - float, + Optional[float], Arg( help=( "The ratio of SWA layer KV tokens / full layer KV tokens, regardless " @@ -824,9 +824,10 @@ class ServerArgs: "layer has 100 tokens." ), resolvable=True, + fallback=0.8, ), NS("schedule"), - ] = 0.8 + ] = None disable_hybrid_swa_memory: A[ bool, Arg(help="Disable the hybrid SWA memory pool.", resolvable=True), @@ -2642,13 +2643,14 @@ class ServerArgs: NS("exec.mamba"), ] = 0 mamba_full_memory_ratio: A[ - float, + Optional[float], Arg( help="The ratio of mamba state memory to full kv cache memory.", resolvable=True, + fallback=0.9, ), NS("schedule"), - ] = 0.9 + ] = None mamba_radix_cache_strategy: A[ str, Arg( diff --git a/test/registered/unit/server_args/test_declared_fallbacks.py b/test/registered/unit/server_args/test_declared_fallbacks.py new file mode 100644 index 000000000..23970d1a2 --- /dev/null +++ b/test/registered/unit/server_args/test_declared_fallbacks.py @@ -0,0 +1,184 @@ +"""A field can declare what it means when nobody said anything. + +`Arg(fallback=...)` is the value a field takes when the operator did not type +it and resolution did not decide it. It is not the dataclass default: that +stays `None`, because `None` is how the record spells "not typed" and the +record is what crosses a process boundary. + +The point of the tests below is that the three surfaces keep disagreeing, on +purpose: + +* the **record** still holds `None` -- so a model family asking "did anyone set + this?" still gets an answer, and the wire format is unchanged; +* the **views** a resolution pass reads still answer `None` -- so a family's + `if cfg.x is None` fires and its declaration lands; +* the **effective** surface -- `resolution_result`, the projection, and the + config bags every runtime reader goes through -- answers with the fallback. + +That last split is the whole design. Putting the fallback in the views instead +would make `if cfg.swa_full_tokens_ratio is None` in `model_overrides/inkling.py` +never fire, and the family's 0.1 would be silently replaced by the generic 0.8. +""" + +import dataclasses +import unittest +from typing import Optional, get_args, get_type_hints + +from sglang.srt.arg_groups.arg_utils import ( + NO_FALLBACK, + A, + Arg, + fallbacks_of, + with_fallback, +) +from sglang.srt.arg_groups.model_override_base import resolved_view +from sglang.srt.arg_groups.overrides import ( + declare_resolution, + resolution_result, + resolving_view, +) +from sglang.srt.runtime_context import get_schedule, publish, reset_context +from sglang.srt.server_args import ServerArgs +from sglang.test.ci.ci_register import register_cpu_ci +from sglang.test.test_utils import CustomTestCase + +register_cpu_ci(est_time=30, suite="base-a-test-cpu") + + +def _resolved(**kwargs) -> ServerArgs: + server_args = ServerArgs(model_path="dummy", **kwargs) + server_args.resolve_once() + return server_args + + +class TestAFallbackIsWhatNobodySaid(CustomTestCase): + def test_the_effective_value_is_the_declared_fallback(self): + server_args = _resolved() + self.assertEqual(resolution_result(server_args, "swa_full_tokens_ratio"), 0.8) + self.assertEqual(resolution_result(server_args, "mamba_full_memory_ratio"), 0.9) + + def test_the_record_still_says_the_operator_typed_nothing(self): + # The fallback is not the dataclass default. A child process that + # unpickles this record has to be able to tell "unset" from "set to + # the value resolution would have picked anyway". + server_args = _resolved() + self.assertIsNone(server_args.swa_full_tokens_ratio) + self.assertIsNone(server_args.mamba_full_memory_ratio) + + def test_what_the_operator_typed_wins(self): + server_args = _resolved(swa_full_tokens_ratio=0.25) + self.assertEqual(resolution_result(server_args, "swa_full_tokens_ratio"), 0.25) + + def test_a_value_the_operator_typed_that_equals_the_fallback_is_still_input(self): + server_args = _resolved(swa_full_tokens_ratio=0.8) + self.assertEqual(server_args.swa_full_tokens_ratio, 0.8) + self.assertEqual(resolution_result(server_args, "swa_full_tokens_ratio"), 0.8) + + def test_a_decision_wins(self): + server_args = _resolved() + declare_resolution(server_args, "a_model_family", swa_full_tokens_ratio=0.1) + self.assertEqual(resolution_result(server_args, "swa_full_tokens_ratio"), 0.1) + + def test_the_published_bag_answers_with_the_fallback(self): + # Every runtime reader goes through the bags, so this is the surface + # that decides how the pools are sized. + reset_context() + self.addCleanup(reset_context) + publish(ServerArgs(model_path="dummy"), role="test") + self.assertEqual(get_schedule().swa_full_tokens_ratio, 0.8) + self.assertEqual(get_schedule().mamba_full_memory_ratio, 0.9) + + def test_the_dummy_short_circuit_leaves_no_bag_holding_none(self): + # The dummy path returns before most of the pipeline. It used to have + # to call the ratio pass by hand on the way out; a declared fallback + # needs no slot, so there is nothing left to forget. + reset_context() + self.addCleanup(reset_context) + publish(ServerArgs(model_path="none"), role="test") + self.assertIsNotNone(get_schedule().swa_full_tokens_ratio) + self.assertIsNotNone(get_schedule().mamba_full_memory_ratio) + + +class TestAPassDecidingStillSeesUnset(CustomTestCase): + """The views are the Decision-over-Input surface, not the Effect surface. + + `model_overrides/inkling.py` and `model_overrides/deepseek_v4.py` both ask + `if cfg.swa_full_tokens_ratio is None` and declare 0.1 when it is. If a + fallback answered here, that branch would be dead and the family's value + would never be declared. + """ + + def test_the_resolving_view_answers_none(self): + server_args = _resolved() + self.assertIsNone(resolving_view(server_args).swa_full_tokens_ratio) + + def test_the_resolved_view_answers_none(self): + server_args = _resolved() + self.assertIsNone(resolved_view(server_args).mamba_full_memory_ratio) + + def test_a_family_that_tests_is_none_still_fires(self): + server_args = _resolved() + cfg = resolving_view(server_args) + declared = {} + if cfg.swa_full_tokens_ratio is None: # the family's exact shape + declared["swa_full_tokens_ratio"] = 0.1 + self.assertEqual(declared, {"swa_full_tokens_ratio": 0.1}) + + +class TestTheDeclarationIsTheOnlyPlaceTheValueLives(CustomTestCase): + def test_the_declared_set_is_what_the_record_carries(self): + self.assertEqual( + fallbacks_of(ServerArgs), + {"swa_full_tokens_ratio": 0.8, "mamba_full_memory_ratio": 0.9}, + ) + + def test_a_fallback_field_is_optional_and_defaults_to_none(self): + # `None` is what the fallback answers for. A field that defaults to + # anything else can never reach it, so the declaration would be dead. + hints = get_type_hints(ServerArgs, include_extras=True) + for name in fallbacks_of(ServerArgs): + field = next(f for f in dataclasses.fields(ServerArgs) if f.name == name) + with self.subTest(field=name): + self.assertIsNone(field.default, f"{name} must default to None") + inner = get_args(hints[name])[0] + self.assertIn( + type(None), + get_args(inner), + f"{name} must be Optional[...] to hold its unset state", + ) + + def test_the_help_text_does_not_restate_the_value(self): + # The value used to be written twice: as a literal in the hook that + # filled it, and as prose in the help. Two copies drift. + hints = get_type_hints(ServerArgs, include_extras=True) + for name, value in fallbacks_of(ServerArgs).items(): + arg = next(a for a in get_args(hints[name])[1:] if isinstance(a, Arg)) + with self.subTest(field=name): + self.assertNotIn(str(value), arg.help) + + +class TestWithFallback(CustomTestCase): + def test_a_container_fallback_is_copied_per_read(self): + @dataclasses.dataclass + class Cfg: + paths: A[Optional[list], Arg(help="x", fallback=[])] = None + + first = with_fallback(Cfg, "paths", None) + first.append("mutated") + self.assertEqual(with_fallback(Cfg, "paths", None), []) + + def test_a_field_without_a_declaration_is_untouched(self): + self.assertIsNone(with_fallback(ServerArgs, "tokenizer_path", None)) + + def test_a_non_dataclass_has_no_fallbacks(self): + self.assertEqual(fallbacks_of(int), {}) + + def test_the_sentinel_is_not_none(self): + # `None` cannot mark "declares no fallback": it is the state a + # fallback exists to answer for. + self.assertIsNot(NO_FALLBACK, None) + self.assertNotEqual(Arg().fallback, None) + + +if __name__ == "__main__": + unittest.main() diff --git a/test/registered/unit/server_args/test_server_args.py b/test/registered/unit/server_args/test_server_args.py index 729cf6cba..971fba6c9 100644 --- a/test/registered/unit/server_args/test_server_args.py +++ b/test/registered/unit/server_args/test_server_args.py @@ -2,6 +2,7 @@ import argparse import dataclasses import json import os +import shutil import socket import tempfile import unittest @@ -2933,5 +2934,94 @@ class TestDcpKvEventContract(CustomTestCase): self.assertEqual(kv_event_block_size_of(resolving_view(args)), 8) +class TestNoneMeansUnset(CustomTestCase): + """A valued field the resolution rewrites carries `None` for "not set". + + `mamba_full_memory_ratio` used to default to 0.9, so a model family asking + "did the operator leave this alone?" had to compare against the class + default -- which stops being true the moment anything declares the field + first, and says nothing at all if the operator happens to pass 0.9. `None` + answers both, and the generic value lands during resolution instead. + """ + + def _resolved(self, **kwargs): + server_args = ServerArgs(model_path=self._checkpoint(), device="cuda", **kwargs) + server_args.resolve_once() + return resolution_result(server_args, "mamba_full_memory_ratio") + + def _checkpoint(self) -> str: + directory = tempfile.mkdtemp(prefix="none_means_unset_") + self.addCleanup(shutil.rmtree, directory, ignore_errors=True) + with open(os.path.join(directory, "config.json"), "w") as handle: + json.dump( + { + "architectures": ["LlamaForCausalLM"], + "model_type": "llama", + "hidden_size": 16, + "intermediate_size": 32, + "num_attention_heads": 2, + "num_key_value_heads": 2, + "num_hidden_layers": 2, + "vocab_size": 128, + "max_position_embeddings": 2048, + }, + handle, + ) + return directory + + def test_the_record_keeps_none_and_resolution_supplies_the_value(self): + server_args = ServerArgs(model_path=self._checkpoint(), device="cuda") + self.assertIsNone(server_args.mamba_full_memory_ratio) + server_args.resolve_once() + # The record still carries what the operator typed; the value is the + # resolution's. + self.assertIsNone(server_args.mamba_full_memory_ratio) + self.assertEqual(resolution_result(server_args, "mamba_full_memory_ratio"), 0.9) + + def test_an_explicit_value_is_never_overwritten(self): + self.assertEqual(self._resolved(mamba_full_memory_ratio=0.5), 0.5) + + def test_the_swa_ratio_behaves_the_same_way(self): + server_args = ServerArgs(model_path=self._checkpoint(), device="cuda") + self.assertIsNone(server_args.swa_full_tokens_ratio) + server_args.resolve_once() + self.assertEqual(resolution_result(server_args, "swa_full_tokens_ratio"), 0.8) + + explicit = ServerArgs( + model_path=self._checkpoint(), device="cuda", swa_full_tokens_ratio=0.3 + ) + explicit.resolve_once() + self.assertEqual(resolution_result(explicit, "swa_full_tokens_ratio"), 0.3) + + def test_the_swa_ratio_is_still_range_checked(self): + """The check reads the resolved value, so `None` must be gone by then.""" + with self.assertRaisesRegex(ValueError, "swa-full-tokens-ratio"): + ServerArgs( + model_path=self._checkpoint(), + device="cuda", + swa_full_tokens_ratio=0.0, + ).resolve_once() + + def test_a_dummy_model_still_gets_the_generic_values(self): + """The dummy short circuit returns long before the normal fill slot. + + The families it skips are exactly the ones that would have claimed + these fields, so the generic values have to land on the way out -- + otherwise every bag on this path holds None where it used to hold a + ratio. + """ + server_args = ServerArgs(model_path="dummy", device="cuda") + server_args.resolve_once() + self.assertEqual(resolution_result(server_args, "swa_full_tokens_ratio"), 0.8) + self.assertEqual(resolution_result(server_args, "mamba_full_memory_ratio"), 0.9) + + def test_an_explicit_value_equal_to_the_generic_one_still_reads_as_set(self): + """The case the class-default comparison could never see.""" + server_args = ServerArgs( + model_path=self._checkpoint(), device="cuda", mamba_full_memory_ratio=0.9 + ) + self.assertIsNotNone(server_args.mamba_full_memory_ratio) + + if __name__ == "__main__": unittest.main()