Migrate all ServerArgs fields to Annotated style, reduce add_cli_args by ~2400 lines (#28919)
This commit is contained in:
@@ -70,6 +70,7 @@ class Arg:
|
|||||||
required: Optional[bool] = None
|
required: Optional[bool] = None
|
||||||
action: Optional[Any] = None
|
action: Optional[Any] = None
|
||||||
action_kwargs: Optional[dict] = None
|
action_kwargs: Optional[dict] = None
|
||||||
|
const: Optional[Any] = None
|
||||||
# When True, this field is skipped by add_cli_args_from_dataclass.
|
# When True, this field is skipped by add_cli_args_from_dataclass.
|
||||||
# Use for fields that have no CLI surface (e.g. injected via Python only).
|
# Use for fields that have no CLI surface (e.g. injected via Python only).
|
||||||
no_cli: bool = False
|
no_cli: bool = False
|
||||||
@@ -185,12 +186,18 @@ def add_cli_args_from_dataclass(parser, cls, *, fields: Optional[List[str]] = No
|
|||||||
cli_name = arg_meta.cli_name or _field_to_cli_name(field.name)
|
cli_name = arg_meta.cli_name or _field_to_cli_name(field.name)
|
||||||
names = [cli_name] + (arg_meta.aliases or [])
|
names = [cli_name] + (arg_meta.aliases or [])
|
||||||
default = _field_default(field)
|
default = _field_default(field)
|
||||||
|
# Anchor dest to the field name so argparse stores the value
|
||||||
|
# under the dataclass attribute directly, even when cli_name
|
||||||
|
# differs (e.g. --tensor-parallel-size → tp_size).
|
||||||
|
auto_dest = cli_name.lstrip("-").replace("-", "_")
|
||||||
|
dest_kwarg = {"dest": field.name} if field.name != auto_dest else {}
|
||||||
|
|
||||||
# Handle custom action
|
# Handle custom action
|
||||||
if arg_meta.action is not None:
|
if arg_meta.action is not None:
|
||||||
kwargs = {
|
kwargs = {
|
||||||
"action": arg_meta.action,
|
"action": arg_meta.action,
|
||||||
"help": arg_meta.help,
|
"help": arg_meta.help,
|
||||||
|
**dest_kwarg,
|
||||||
}
|
}
|
||||||
if default is not _MISSING:
|
if default is not _MISSING:
|
||||||
kwargs["default"] = default
|
kwargs["default"] = default
|
||||||
@@ -209,34 +216,42 @@ def add_cli_args_from_dataclass(parser, cls, *, fields: Optional[List[str]] = No
|
|||||||
# Infer type from first literal value
|
# Infer type from first literal value
|
||||||
val_type = type(literal_vals[0]) if literal_vals else str
|
val_type = type(literal_vals[0]) if literal_vals else str
|
||||||
type_func = arg_meta.type_parser or _infer_type_func(val_type)
|
type_func = arg_meta.type_parser or _infer_type_func(val_type)
|
||||||
kwargs = dict(type=type_func, choices=choices, help=arg_meta.help)
|
kwargs = dict(
|
||||||
|
type=type_func, choices=choices, help=arg_meta.help, **dest_kwarg
|
||||||
|
)
|
||||||
if default is not _MISSING:
|
if default is not _MISSING:
|
||||||
kwargs["default"] = default
|
kwargs["default"] = default
|
||||||
|
if arg_meta.const is not None:
|
||||||
|
kwargs["const"] = arg_meta.const
|
||||||
parser.add_argument(*names, **kwargs)
|
parser.add_argument(*names, **kwargs)
|
||||||
continue
|
continue
|
||||||
|
|
||||||
# Check for List[X]
|
# Check for List[X] — but skip if type_parser is set (the parser
|
||||||
|
# handles the whole value as a single string, e.g. json_list_type).
|
||||||
origin = get_origin(inner_type)
|
origin = get_origin(inner_type)
|
||||||
if origin is list or origin is List:
|
if (origin is list or origin is List) and arg_meta.type_parser is None:
|
||||||
elem_args = get_args(inner_type)
|
elem_args = get_args(inner_type)
|
||||||
elem_type = elem_args[0] if elem_args else str
|
elem_type = elem_args[0] if elem_args else str
|
||||||
type_func = arg_meta.type_parser or _infer_type_func(elem_type)
|
type_func = _infer_type_func(elem_type)
|
||||||
nargs = arg_meta.nargs or "+"
|
nargs = arg_meta.nargs or "+"
|
||||||
kwargs = dict(
|
kwargs = dict(
|
||||||
type=type_func,
|
type=type_func,
|
||||||
nargs=nargs,
|
nargs=nargs,
|
||||||
help=arg_meta.help,
|
help=arg_meta.help,
|
||||||
|
**dest_kwarg,
|
||||||
)
|
)
|
||||||
if arg_meta.choices:
|
if arg_meta.choices:
|
||||||
kwargs["choices"] = arg_meta.choices
|
kwargs["choices"] = arg_meta.choices
|
||||||
if default is not _MISSING:
|
if default is not _MISSING:
|
||||||
kwargs["default"] = default
|
kwargs["default"] = default
|
||||||
|
if arg_meta.const is not None:
|
||||||
|
kwargs["const"] = arg_meta.const
|
||||||
parser.add_argument(*names, **kwargs)
|
parser.add_argument(*names, **kwargs)
|
||||||
continue
|
continue
|
||||||
|
|
||||||
# Bool → store_true
|
# Bool → store_true
|
||||||
if inner_type is bool:
|
if inner_type is bool:
|
||||||
kwargs = dict(action="store_true", help=arg_meta.help)
|
kwargs = dict(action="store_true", help=arg_meta.help, **dest_kwarg)
|
||||||
if default is not _MISSING:
|
if default is not _MISSING:
|
||||||
kwargs["default"] = default
|
kwargs["default"] = default
|
||||||
parser.add_argument(*names, **kwargs)
|
parser.add_argument(*names, **kwargs)
|
||||||
@@ -244,13 +259,15 @@ def add_cli_args_from_dataclass(parser, cls, *, fields: Optional[List[str]] = No
|
|||||||
|
|
||||||
# Scalar types (str, int, float, etc.)
|
# Scalar types (str, int, float, etc.)
|
||||||
type_func = arg_meta.type_parser or _infer_type_func(inner_type)
|
type_func = arg_meta.type_parser or _infer_type_func(inner_type)
|
||||||
kwargs = dict(type=type_func, help=arg_meta.help)
|
kwargs = dict(type=type_func, help=arg_meta.help, **dest_kwarg)
|
||||||
if arg_meta.choices:
|
if arg_meta.choices:
|
||||||
kwargs["choices"] = arg_meta.choices
|
kwargs["choices"] = arg_meta.choices
|
||||||
if arg_meta.nargs:
|
if arg_meta.nargs:
|
||||||
kwargs["nargs"] = arg_meta.nargs
|
kwargs["nargs"] = arg_meta.nargs
|
||||||
if default is not _MISSING:
|
if default is not _MISSING:
|
||||||
kwargs["default"] = default
|
kwargs["default"] = default
|
||||||
|
if arg_meta.const is not None:
|
||||||
|
kwargs["const"] = arg_meta.const
|
||||||
if (
|
if (
|
||||||
arg_meta.required is True
|
arg_meta.required is True
|
||||||
or (arg_meta.required is None and default is _MISSING)
|
or (arg_meta.required is None and default is _MISSING)
|
||||||
|
|||||||
+1706
-2753
File diff suppressed because it is too large
Load Diff
@@ -128,7 +128,7 @@ class TestServerArgsMigratedCliMetadata(CustomTestCase):
|
|||||||
for option in ("--data-parallel-size", "--dp-size"):
|
for option in ("--data-parallel-size", "--dp-size"):
|
||||||
with self.subTest(option=option):
|
with self.subTest(option=option):
|
||||||
args = self.parser.parse_args(["--model", "dummy", option, "3"])
|
args = self.parser.parse_args(["--model", "dummy", option, "3"])
|
||||||
self.assertEqual(args.data_parallel_size, 3)
|
self.assertEqual(args.dp_size, 3)
|
||||||
self.assertEqual(ServerArgs.from_cli_args(args).dp_size, 3)
|
self.assertEqual(ServerArgs.from_cli_args(args).dp_size, 3)
|
||||||
|
|
||||||
def test_migrated_and_manual_options_parse_together(self):
|
def test_migrated_and_manual_options_parse_together(self):
|
||||||
|
|||||||
@@ -0,0 +1,126 @@
|
|||||||
|
"""Tests for ServerArgs CLI argument auto-derivation from A[T, Arg(...)] annotations.
|
||||||
|
|
||||||
|
Each test covers a distinct edge case in how add_cli_args_from_dataclass
|
||||||
|
translates field annotations into argparse arguments.
|
||||||
|
"""
|
||||||
|
|
||||||
|
import argparse
|
||||||
|
import unittest
|
||||||
|
|
||||||
|
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=10, suite="base-a-test-cpu")
|
||||||
|
|
||||||
|
|
||||||
|
class TestServerArgsAnnotatedCli(CustomTestCase):
|
||||||
|
|
||||||
|
@classmethod
|
||||||
|
def setUpClass(cls):
|
||||||
|
cls.parser = argparse.ArgumentParser()
|
||||||
|
ServerArgs.add_cli_args(cls.parser)
|
||||||
|
|
||||||
|
def _parse(self, args_list):
|
||||||
|
args = self.parser.parse_args(["--model", "dummy"] + args_list)
|
||||||
|
return ServerArgs.from_cli_args(args)
|
||||||
|
|
||||||
|
def test_aliases_and_dest(self):
|
||||||
|
"""Field name drives dest; long forms and short aliases both work."""
|
||||||
|
cases = [
|
||||||
|
("--tp-size", "--tensor-parallel-size", "tp_size"),
|
||||||
|
("--pp-size", "--pipeline-parallel-size", "pp_size"),
|
||||||
|
("--dp-size", "--data-parallel-size", "dp_size"),
|
||||||
|
("--ep-size", "--expert-parallel-size", "ep_size"),
|
||||||
|
("--attn-cp-size", "--attention-context-parallel-size", "attn_cp_size"),
|
||||||
|
("--moe-dp-size", "--moe-data-parallel-size", "moe_dp_size"),
|
||||||
|
]
|
||||||
|
for short, long, field in cases:
|
||||||
|
with self.subTest(field=field):
|
||||||
|
sa_short = self._parse([short, "4"])
|
||||||
|
sa_long = self._parse([long, "4"])
|
||||||
|
self.assertEqual(getattr(sa_short, field), 4)
|
||||||
|
self.assertEqual(getattr(sa_long, field), 4)
|
||||||
|
|
||||||
|
def test_cli_name_differs_from_field_name(self):
|
||||||
|
"""cli_name maps a different CLI flag to the dataclass field via dest."""
|
||||||
|
sa = self._parse(
|
||||||
|
["--fp8-gemm-backend", "triton", "--fp4-gemm-backend", "cutlass"]
|
||||||
|
)
|
||||||
|
self.assertEqual(sa.fp8_gemm_runner_backend, "triton")
|
||||||
|
self.assertEqual(sa.fp4_gemm_runner_backend, "cutlass")
|
||||||
|
|
||||||
|
def test_nargs_question_with_const(self):
|
||||||
|
"""nargs='?' + const='' for --model-checksum."""
|
||||||
|
self.assertIsNone(self._parse([]).model_checksum)
|
||||||
|
self.assertEqual(self._parse(["--model-checksum"]).model_checksum, "")
|
||||||
|
self.assertEqual(self._parse(["--model-checksum", "abc"]).model_checksum, "abc")
|
||||||
|
|
||||||
|
def test_boolean_optional_action(self):
|
||||||
|
"""BooleanOptionalAction supports --flag and --no-flag."""
|
||||||
|
self.assertIsNone(self._parse([]).experts_shared_outer_loras)
|
||||||
|
self.assertTrue(
|
||||||
|
self._parse(["--experts-shared-outer-loras"]).experts_shared_outer_loras
|
||||||
|
)
|
||||||
|
self.assertFalse(
|
||||||
|
self._parse(["--no-experts-shared-outer-loras"]).experts_shared_outer_loras
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_json_type_parsers(self):
|
||||||
|
"""json.loads and json_list_type parse single-string arguments."""
|
||||||
|
sa = self._parse(
|
||||||
|
[
|
||||||
|
"--extra-metric-labels",
|
||||||
|
'{"k": "v"}',
|
||||||
|
"--forward-hooks",
|
||||||
|
'[{"type": "test"}]',
|
||||||
|
]
|
||||||
|
)
|
||||||
|
self.assertEqual(sa.extra_metric_labels, {"k": "v"})
|
||||||
|
self.assertEqual(sa.forward_hooks, [{"type": "test"}])
|
||||||
|
|
||||||
|
def test_literal_auto_derives_choices(self):
|
||||||
|
"""Literal type annotations produce argparse choices automatically."""
|
||||||
|
sa = self._parse(
|
||||||
|
["--deepep-mode", "low_latency", "--elastic-ep-backend", "none"]
|
||||||
|
)
|
||||||
|
self.assertEqual(sa.deepep_mode, "low_latency")
|
||||||
|
self.assertEqual(sa.elastic_ep_backend, "none")
|
||||||
|
|
||||||
|
def test_deprecated_flags_still_work(self):
|
||||||
|
"""Deprecated flags set the correct dest field."""
|
||||||
|
sa = self._parse(["--stream-output"])
|
||||||
|
self.assertTrue(sa.incremental_streaming_output)
|
||||||
|
|
||||||
|
def test_combined_parse(self):
|
||||||
|
"""Multiple option types parsed together in one invocation."""
|
||||||
|
sa = self._parse(
|
||||||
|
[
|
||||||
|
"--dtype",
|
||||||
|
"bfloat16",
|
||||||
|
"--tp-size",
|
||||||
|
"4",
|
||||||
|
"--data-parallel-size",
|
||||||
|
"2",
|
||||||
|
"--enable-lora",
|
||||||
|
"--watchdog-timeout",
|
||||||
|
"600",
|
||||||
|
"--bucket-time-to-first-token",
|
||||||
|
"0.5",
|
||||||
|
"1.0",
|
||||||
|
"--lora-paths",
|
||||||
|
"p1",
|
||||||
|
"p2",
|
||||||
|
]
|
||||||
|
)
|
||||||
|
self.assertEqual(sa.dtype, "bfloat16")
|
||||||
|
self.assertEqual(sa.tp_size, 4)
|
||||||
|
self.assertEqual(sa.dp_size, 2)
|
||||||
|
self.assertTrue(sa.enable_lora)
|
||||||
|
self.assertEqual(sa.watchdog_timeout, 600.0)
|
||||||
|
self.assertEqual(sa.bucket_time_to_first_token, [0.5, 1.0])
|
||||||
|
self.assertIsNotNone(sa.lora_paths)
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
unittest.main()
|
||||||
Reference in New Issue
Block a user