Skip to content

[MiniMax-M3] Drop the local get_parallel import that shadows the module-level one - #38911

Closed
huthvincent wants to merge 1 commit into
sgl-project:mainfrom
huthvincent:fix/B015-minimax-msa-get-parallel-shadowed
Closed

huthvincent wants to merge 1 commit into
sgl-project:mainfrom
huthvincent:fix/B015-minimax-msa-get-parallel-shadowed

Conversation

@huthvincent

@huthvincent huthvincent commented Sep 10, 2026 •

Copy link
Copy Markdown

Motivation

MiniMaxSparseAttnBackend.__init__ cannot construct when MSA is active. It raises

UnboundLocalError: cannot access local variable 'get_parallel' where it is not associated with a value

at python/sglang/srt/layers/attention/minimax_sparse_backend.py line 237, which is inside
if self.use_msa:. Because attn_backend_wrapper constructs the backend with no try/except and no
fallback, an affected launch dies before the first token.

The cause is a scoping collision introduced two days ago by #36527. get_parallel is imported at module
level (lines 25-28). #36527 added a function-local from sglang.srt.runtime_context import get_parallel
at line 297, inside if self.index_cache_enabled:, in the same __init__ that already used the name at line
237. Python binds a name imported anywhere in a function body as local to that entire body, so line 237's
local slot is empty when it is read.

This happens whether or not line 297 executes, because the scoping is static. It is also why the two uses
have never collided in testing: line 297's guard self.index_cache_enabled requires
is_hip() and is_gfx95_supported() (lines 286-291), so on AMD line 237 is skipped and the local import runs
harmlessly, while on SM100 line 237 runs and the local import does not.

The gate at lines 210-217 is satisfied by the documented default Blackwell recipe, with no non-default flag:

condition how a default SM100 launch satisfies it
not envs.SGLANG_DISABLE_MSA.get() SGLANG_DISABLE_MSA = EnvBool(False), python/sglang/srt/environ.py line 1585
msa_available() needs compute capability (10, 0) or (10, 3) and an importable fmha_sm100. docs/cookbook/autoregressive/MiniMax/MiniMax-M3.mdx line 90 states that fmha_sm100 "ships pre-installed in the M3 dev image (lmsysorg/sglang:dev-minimax-m3) so the Blackwell recipe above engages it automatically with no extra setup"
self.kv_pool.page_size == self.block_size_k auto-forced. python/sglang/srt/arg_groups/model_overrides/minimax_m3.py lines 73 and 79-81: on SM100 the backend defaults to fa4 (or trtllm_mha for fp8_e4m3 KV), and then if page_resolved is None and backend_resolved in ("fa4", "trtllm_mha"): overrides["page_size"] = 128. Passing no --page-size is what triggers it.
not _main_kv_is_fp8 or _msa_fp8_ok bf16 KV leaves _main_kv_is_fp8 false; fp8_e4m3 KV with trtllm_mha sets _msa_fp8_ok. Both SM100 sub-recipes pass.

block_size_k and topk_blocks come from the checkpoint's sparse_attention_config.

Modifications

Two lines deleted in one file: python/sglang/srt/layers/attention/minimax_sparse_backend.py.

@@ -294,8 +294,6 @@ class MiniMaxSparseAttnBackend(AttentionBackend):
         # that wide. Head split mirrors MiniMaxM3 sparse attention's.
         self._idx_group_size = 1
         if self.index_cache_enabled:
-            from sglang.srt.runtime_context import get_parallel
-
             _num_idx_heads = max(
                 sparse_cfg["sparse_num_index_heads"] // get_parallel().attn_tp_size, 1
             )

Nothing else is touched: no refactor, no rename, no formatting, no adjacent fix.

Three reasons this is the right deletion rather than a rename or an added local import above line 237:

  1. Lines 25-28 already import the identical symbol from the identical module, unconditionally. The
    TYPE_CHECKING block at lines 54-55 imports only ModelRunner, so the module-level import is not
    conditional.
  2. python/sglang/srt/runtime_context.py lines 66-67 say why a module-level import is safe here:
    "Imported lazily so this module has no import-time dependencies: any module can import get_parallel at
    module level without risking an import cycle."
  3. get_spec is imported at the same site (line 27) and used at line 260 with no local shadow. That is the
    existing precedent in this file for relying on the module-level import.

Both lines have to go, not just line 297: deleting the import alone leaves a blank line at the top of the
if body, which ruff-format v0.15.1 removes, so pre-commit run --all-files would fail. With both lines
deleted, ruff format --check reports "1 file already formatted" and
ruff check --select F401,F821,UP037 reports "All checks passed!".

Accuracy Tests

No model output changes. The patch deletes a redundant import and changes no computation, no dtype and no
kernel selection.

What we executed, and on what. We lifted __init__'s code object out of the file at
12771786f23190b1845db33366eba09cb5eacf41 and ran it with its globals stubbed, so the bytecode was this
repository's and unmodified. As shipped it raises the UnboundLocalError above at line 237; with the two
lines deleted, the same harness raises nothing.

What we did not execute. We have no B200 or B300, so we have not observed the failure on the
configuration that reaches it. msa_available() requires compute capability (10, 0) or (10, 3); our
hardware is sm_89. We have not launched MiniMax-M3.

Speed Tests and Profiling

We did not measure this. The analysis is static — read from the code and the commit history, with no
profiling and no benchmark run on our side. The check described below is what we are asking you to run.

There is no performance claim here in either direction. The patch removes an import statement.

How to check it

Two ways, and the first needs no GPU, no model and no SGLang import.

1. Read the scoping directly out of the file. On a checkout of main:

import symtable, dis
P = "python/sglang/srt/layers/attention/minimax_sparse_backend.py"
src = open(P).read()
st = symtable.symtable(src, P, "exec")
cls = next(c for c in st.get_children() if c.get_name() == "MiniMaxSparseAttnBackend")
ini = next(c for c in cls.get_children() if c.get_name() == "__init__")
s = ini.lookup("get_parallel")
print("is_local:", s.is_local(), "is_global:", s.is_global())

co = next(c for c in compile(src, P, "exec").co_consts
          if hasattr(c, "co_consts") and getattr(c, "co_name", "") == "MiniMaxSparseAttnBackend")
ini_co = next(c for c in co.co_consts if getattr(c, "co_name", "") == "__init__")
print([i.opname for i in dis.get_instructions(ini_co)
       if i.positions.lineno == 237 and i.argval == "get_parallel"])

On main this prints is_local: True is_global: False and ['LOAD_FAST_CHECK'] — LOAD_FAST_CHECK is the
opcode that raises UnboundLocalError on an empty local slot. On this branch it prints
is_local: False is_global: True and ['LOAD_GLOBAL']. If it printed LOAD_GLOBAL on main, this report
would be wrong.

2. On Blackwell hardware. sglang serve --model-path MiniMaxAI/MiniMax-M3 inside
lmsysorg/sglang:dev-minimax-m3, no other flags. Expected on main: UnboundLocalError during attention
backend construction. Expected on this branch: normal startup.

The strongest objection to this report

Stated in full rather than answered, because it is the reason to be sceptical and it is not disposed of:

main has carried this since #36527 merged, and MiniMax-M3 on SM100 is a shipped configuration, so if
use_msa were genuinely true on the default SM100 path, every Blackwell M3 launch would have died at
attention_registry.py:363 within hours of the merge. Produce the traceback or the failing CI job, not a
symtable dump — especially since no test under test/ constructs MiniMaxSparseAttnBackend at all.

The one fact that bears on it without answering it: #36527 merged with zero review comments, and the shadow
is two days old.

Checklist

  • Format your code according to Format code with pre-commit — ruff format --check and
    ruff check --select F401,F821,UP037 both clean on the changed file at v0.15.1, the version pinned in
    .pre-commit-config.yaml.
  • Add unit tests — not added. A test for this would have to assert on bytecode or construct the backend
    on SM100. Happy to add whichever you would accept.
  • Update documentation — not applicable.
  • Provide accuracy and speed benchmark results — stated above: no output change, and no measurement was
    made. Section "Speed Tests and Profiling" says so explicitly rather than implying one.
  • Follow the SGLang code style guidance.

CI States

Latest PR Test (Base): ❌ Run #34493043676
Latest PR Test (Extra): ❌ Run #34493043211
Latest PR Test (AMD ROCm 10): ❌ Run #34493043560

…le-level one

`MiniMaxSparseAttnBackend.__init__` reads `get_parallel()` under
`if self.use_msa:`, against the module-level import at the top of the file. A
function-local `from sglang.srt.runtime_context import get_parallel` added later
in the same function makes the name local to all of `__init__`, so the earlier
read compiles to LOAD_FAST_CHECK and raises UnboundLocalError whenever
`use_msa` is true -- whether or not the local import executes, because the
scoping is static rather than dynamic.

The module-level import already provides the same symbol from the same module,
and `runtime_context` documents itself as safe to import at module level for
exactly this reason. `get_spec`, imported at the same site and used later in
`__init__` with no local shadow, is the existing precedent in this file.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant