From ec452d79cac764c447fab74601bd9b31f1551a77 Mon Sep 17 00:00:00 2001 From: Tianyi Zhang <123608656+tianyi-zhang-02@users.noreply.github.com> Date: Thu, 6 Aug 2026 00:21:47 -0700 Subject: [PATCH] fix(sglang): expose cfg on SGLangGeneration create_weight_synchronizer reads generation.cfg unconditionally, before any backend dispatch. SGLangGeneration stored the generation config only as self.sglang_cfg and defined no cfg attribute, so backend=sglang raised AttributeError there instead of getting its HTTPWeightSynchronizer. VllmGeneration, TRTLLMGeneration and MegatronGeneration all expose cfg; megatron_generation.py documents it as the GenerationInterface contract. sglang_cfg already is that same config object, so alias it with a read-only property rather than keep a second reference that could drift. Signed-off-by: Tianyi Zhang <123608656+tianyi-zhang-02@users.noreply.github.com> --- .../generation/sglang/sglang_generation.py | 11 +++++++ .../weight_sync/test_weight_synchronizer.py | 30 +++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/nemo_rl/models/generation/sglang/sglang_generation.py b/nemo_rl/models/generation/sglang/sglang_generation.py index 9aa21547a6..6c7ac384e4 100644 --- a/nemo_rl/models/generation/sglang/sglang_generation.py +++ b/nemo_rl/models/generation/sglang/sglang_generation.py @@ -122,6 +122,17 @@ def __init__( if init_handles: ray.get(init_handles) + @property + def cfg(self) -> SGLangConfig: + """Full generation config, matching the ``GenerationInterface`` contract. + + ``sglang_cfg`` already is that config, so alias it rather than keep a + second reference. Backend-agnostic callers such as + ``nemo_rl.weight_sync.factory.create_weight_synchronizer`` read + ``generation.cfg`` regardless of backend. + """ + return self.sglang_cfg + # ------------------------------------------------------------------ # Engine topology properties (formerly ``ServerGroup``) # ------------------------------------------------------------------ diff --git a/tests/unit/weight_sync/test_weight_synchronizer.py b/tests/unit/weight_sync/test_weight_synchronizer.py index e799fc56bd..6cbda470d8 100644 --- a/tests/unit/weight_sync/test_weight_synchronizer.py +++ b/tests/unit/weight_sync/test_weight_synchronizer.py @@ -444,6 +444,36 @@ def test_colocated_sglang_returns_http(self): ) assert isinstance(sync, HTTPWeightSynchronizer) + def test_colocated_sglang_accepts_real_generation_object(self): + """A real SGLangGeneration must expose ``cfg`` like the other backends. + + ``create_weight_synchronizer`` reads ``generation.cfg`` before any + backend dispatch. The MagicMock double used above answers ``cfg`` + automatically, so only the real class can catch a missing attribute. + """ + from nemo_rl.models.generation.sglang.sglang_generation import ( + SGLangGeneration, + ) + + # ``__new__`` skips ``__init__``, which would need Ray, a router and + # live engines; ``cfg`` only depends on ``sglang_cfg``. + gen = SGLangGeneration.__new__(SGLangGeneration) + gen.sglang_cfg = {"backend": "sglang", "model_name": "dummy"} + # ``__del__`` calls ``shutdown()``, which reads these four attributes. + gen.all_engines = [] + gen._router_actor = None + gen._http_client = None + gen._async_loop = None + assert gen.cfg is gen.sglang_cfg + + sync = create_weight_synchronizer( + policy=_mock_policy(), + generation=gen, + generation_backend=SGLANG_BACKEND, + colocated=True, + ) + assert isinstance(sync, HTTPWeightSynchronizer) + def test_colocated_megatron_returns_ipc(self): policy = _mock_policy() gen = _mock_generation()