[AMD] Fix AttributeError in GeneratedSharedPrefixDataset.from_args for in-process callers - #27580
Conversation
…for in-process callers
`from_args` read `args.gsp_group_distribution` and `args.gsp_zipf_alpha` via
direct attribute access. These have argparse defaults ("uniform" / None) on the
CLI, but in-process callers that build a SimpleNamespace via `get_benchmark_args`
(e.g. test_gsp_multi_turn) do not set them, so the access raised:
AttributeError: 'types.SimpleNamespace' object has no attribute
'gsp_group_distribution'
This broke the nightly `test_bench_serving_functionality.py::test_gsp_multi_turn`
on both the AMD and CUDA nightly suites. Every other optional gsp_* field in the
same method already uses getattr() with a default, and the method's own comment
states it is meant to tolerate hand-built Namespaces.
Read the two fields with getattr() using the argparse defaults, matching the
sibling fields and the documented intent.
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Heads-up @Jiminator: this fixes a regression from #26378. That PR added Fix here switches the two reads to |
|
/tag-and-rerun-ci |
|
/rerun-test test/registered/bench_fn/test_bench_serving_functionality.py::TestBenchServingFunctionality.test_gsp_multi_turn |
|
Results for 🚀 |
There was a problem hiding this comment.
LGTM, thank you for catching this @michaelzhang-ai
|
closing other pr #27623, same fix |
…r in-process callers (sgl-project#27580)
Motivation
The 1-GPU nightly tests fail in
test/registered/bench_fn/test_bench_serving_functionality.py::test_gsp_multi_turnwith:GeneratedSharedPrefixDataset.from_argsreadsargs.gsp_group_distributionandargs.gsp_zipf_alphavia direct attribute access. These options have argparse defaults ("uniform"/None) on thebench_serving.pyCLI, but in-process callers that build aSimpleNamespacethroughget_benchmark_args(e.g.test_gsp_multi_turn) never set them, so the access raises before the method's own defensive validation can run. Every other optionalgsp_*field infrom_argsalready usesgetattr(args, ..., default), and the method's comment states it is meant to tolerate hand-built Namespaces.This is shared benchmark-client code and the test is registered for both suites (
register_cuda_ci+register_amd_ci), so it breaks both 1-GPU nightlies:nightly-test-1-gpu-unitnightly-test-general-1-gpu-h100Regression
Introduced by #26378 (commit f838adb, merged 2026-05-28), which added the
gsp_group_distribution/gsp_zipf_alphafeature. That PR added the argparse options (with defaults) and the hard attribute access infrom_args, but did not updateget_benchmark_argsinpython/sglang/test/test_utils.py. Becausetest_gsp_multi_turnis registered nightly-only, it does not run in pre-merge PR CI, so the break only surfaced in the scheduled nightlies after the merge. cc @JiminatorModifications
Restore-only — not a new code change. This puts the two GSP fields back on the
getattr(args, ..., default)convention that the rest offrom_args(and #19363, "remove args dependency") already use. #26378 was the only place that deviated to direct attribute access; this reverts that one deviation:No behavior change for existing usage: the CLI/argparse path always supplies these attributes, so
getattrreturns exactly the same values, and the defaults ("uniform"/None) match both the argparse defaults and the feature's own test helper (make_args). The only difference is that an in-processNamespacethat omits the fields now defaults (as the sibling fields already do) instead of raisingAttributeError.Accuracy Tests
N/A — does not affect model outputs (benchmark-client arg handling only).
Speed Tests and Profiling
N/A — pure arg-handling bugfix.
Verification
SimpleNamespacethattest_gsp_multi_turnbuilds viaget_benchmark_args(...): before →AttributeError: ... 'gsp_group_distribution'; after →GeneratedSharedPrefixDataset(..., group_distribution='uniform', zipf_alpha=None).nightly-test-amd.yml,nightly-test-1-gpu-unit): https://github.com/sgl-project/sglang/actions/runs/27150321984/job/80139313149 → 10/10 passed (test_bench_serving_functionality.pynow passes; previously 9/10).Checklist
CI States
Latest PR Test (Base): ❌ Run #27149657199
Latest PR Test (Extra): 🚫 Run #27153205907