config: spell the parallel config tier at the call site - #36250
Conversation
github-actions
Bot
added
documentation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 762949f4c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| self._config = None # parallel config bag, wired at publish | ||
|
|
||
| @property | ||
| def config(self) -> _ConfigBag: |
There was a problem hiding this comment.
_ConfigBag forward reference
On supported Python 3.10–3.13, annotations are evaluated while the class body executes, but _ConfigBag is not defined until later in this module. Importing runtime_context therefore raises NameError: name '_ConfigBag' is not defined, preventing SGLang from starting. Quote this annotation or enable postponed annotation evaluation.
Useful? React with 👍 / 👎.
762949f to
bd01f13
Compare
bd01f13 to
1303cea
Compare
1303cea to
18e7fd8
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 18e7fd86c0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| elif isinstance(node, ast.Import): | ||
| for a in node.names: | ||
| if a.name.endswith("runtime_context"): | ||
| modules.add(a.asname or a.name.split(".")[0]) |
There was a problem hiding this comment.
When a module uses import sglang.srt.runtime_context without an alias, Python binds only sglang, so this records sglang in modules; however, is_bag_call only recognizes a single-name receiver such as sglang.get_parallel() and cannot match the actual sglang.srt.runtime_context.get_parallel() attribute chain. Consequently, a configured shadowed-size read through that fully qualified spelling silently escapes _CONFIGURED_SIZE_CALL_SITES, even though this scanner explicitly claims to cover module-qualified calls.
Useful? React with 👍 / 👎.
18e7fd8 to
27cde8d
Compare
`get_parallel()` served two facts through one surface: a bare name was the live process group where a `@property` happened to exist and the published `parallel` bag otherwise, and the five sizes that are both had to be read through a separate `configured_*_size()` free-function family. A call site could not tell which tier it got -- `get_parallel().ep_size` was config, `get_parallel().moe_ep_size` was live -- and the two tiers are not two spellings of one number: elastic EP scales the live world away from the configured one, and `initialize_model_parallel` aliases `_MOE_DP` to `_ATTN_CP` when `attn_cp_size > moe_dp_size`, which makes a live comparison of that pair degenerate. Bare is now the live group and `get_parallel().config.<leaf>` is the published bag. The five `configured_*_size()` functions are gone (76 call sites converted), the remaining 291 config-leaf reads moved under the same hop, and a bare read of a config-only leaf raises an AttributeError naming the `.config` spelling instead of quietly answering from the bag. Every converted read keeps its tier; the `config` hop is a plain property over a slot, so config-leaf reads inside compiled model forwards still trace under `fullgraph=True`. The two guards follow: `_CONFIGURED_SIZE_CALL_SITES` keys on (file, size) and derives its subject set from the two sides themselves (ParallelContext properties intersected with the `parallel` NS leaves) instead of a hand list, and the launch-path guard builds its remedy text from the same derivation. Both resolve `get_parallel` aliases and the `getattr` spelling, so neither an import rename nor a new shadowed size escapes them.
27cde8d to
06caed3
Compare
1 participant
Motivation
get_parallel()exposes two different things through one spelling. Bareget_parallel().tp_sizeanswers from the live process groups; theconfigured_*_size()accessors answered from the publishedparallelconfig. Bothspellings work everywhere, neither says which one it is, and they are not
interchangeable — they provably differ in three situations:
torch.distributedis initialised, or in a process that has no group, the livesurface has no answer at all while the configured one does;
initialize_model_parallelaliases the MoE-DP group onto the attention-CP group whenattn_cp_size > moe_dp_size, so the live sizes stop matching what was configured;A reader that wants "what did this deployment ask for" and reaches for the live surface
gets a different number in exactly the cases that are hardest to reproduce.
What changes
ParallelContext.configis added: it returns the publishedparallelnamespace, andraises
ValueError("config namespace 'parallel' not published")if nothing published yet.ParallelContext.__getattr__becomes error-only: it never answers a config leaf. Whenthe
parallelnamespace is published it recognises the name and says where to read it —'nccl_port' is a parallel config leaf, not live topology; read it as get_parallel().config.nccl_port— and otherwise raises the plainParallelContext has no 'nccl_port'. (A live size read before dist init still surfacesthe distributed layer's own
tensor model parallel group is not initialized; that pathdoes not go through
__getattr__because those names are real properties.)_configured_paralleland the fiveconfigured_*_size()accessors are deleted.get_parallel().config.<size>,and the reads that meant the configured value but were written against the live surface
get the same treatment. Reads that genuinely want the live groups keep the bare spelling.
test_global_config_read_ratchet.pyenumerates the result as 33 (file, size) cells over23 files, each with the reason the live property cannot serve it, and asserts that the
registry and the code agree in both directions. Its subject set — which sizes are
"live-shadowed" at all — is derived from
ParallelContextproperties ∩ theparallelnamespace leaves, so a size that gains a live property joins the watched set without an
edit here. Every spelling of the call resolves — an aliased import, a module-qualified
receiver, a local bound to either hop — and
TestParallelConfigReadSpellingsruns eachone, because a spelling the scanner cannot resolve drops the read instead of failing.
No behaviour change is intended: every rewritten read keeps the value it had, and the
sites that must stay live are unchanged.
How to verify
🤖 Generated with Claude Code
CI States
Latest PR Test (Base): ❌ Run #32953996109
Latest PR Test (Extra): ❌ Run #32953995839
Latest PR Test (AMD ROCm 7.2): ❌ Run #32953996116