[MLX] Support gpt-oss: sliding-window attention, attention sinks, sm_scale - #30050
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds comprehensive support for sliding-window attention models (such as gpt-oss) on the MLX backend, introducing banded attention masks, trailing-window KV truncation, and support for attention sinks. It also updates the AOT RoPE kernel gating to reject scaled RoPE variants and adds extensive end-to-end and unit tests. The reviewer feedback suggests optimizing the hot path in MLXAttentionWrapper by resolving and caching the attention scale and sinks during initialization rather than on every decode step, which also enables failing fast if the scale cannot be determined.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
8628092 to
ce2f6fb
Compare
ce2f6fb to
18644c5
Compare
|
Re-reviewed the branch end to end and re-ran the full local matrix on the final commit (18644c5): sliding-window / pool-contract / runner-init / scheduler-mixin unit suites (34 tests), gpt-oss serving smoke, gpt-oss token-for-token reference equivalence vs unpatched mlx_lm, Qwen1.5-MoE reference equivalence, and Qwen3-30B serving smoke — all green on Apple Silicon (24 GB). One hardening was amended in: the opt-in AOT RoPE gate now also rejects |
# Conflicts: # python/sglang/srt/server_args.py
|
Resolved the conflict with main. The config resolution refactor moved the GPT OSS attention backend selection into arg_groups/overrides.py, so the MLX skip now lives in _gpt_oss_overrides and the assertion guard stays in server_args.py. MLX unit tests pass locally. |
| # applied outside mx.fast.rope), while linear scaling keeps ``base`` but | ||
| # sets ``scale != 1`` on nn.RoPE. The kernel has inputs for none of | ||
| # these, so they must fall back to mx.fast.rope. | ||
| base = getattr(rope, "base", None) |
| # When serving through MLX on Apple Silicon the backend stays at | ||
| # the platform default, which attention never runs through, so | ||
| # the CUDA-oriented backend assertion below does not apply. | ||
| _mlx_serving = is_mps() and use_mlx() |
There was a problem hiding this comment.
I would revert the server_args.py part of this change. The new _mlx_serving = is_mps() and use_mlx() special case makes the generic GPT-OSS validation path understand an MLX runtime detail, which is the wrong direction for this file.
There was a problem hiding this comment.
Dropped use_mlx() here, agreed that's the wrong direction for this file. A plain revert breaks startup though: with the overrides change, nothing sets attention_backend on MPS before this assert runs (the torch_native default is only filled later, in _handle_attention_backend_compatibility), so every gpt-oss launch on Apple Silicon would fail the assert with None backends. Kept the assert and scoped it with the same platform predicate as the overrides side instead: if not is_mps():.
… knobs - overrides.py: replace the is_mps()+use_mlx() special case with a plain `elif not is_mps()` in the gpt-oss backend chain; drop the use_mlx import - server_args.py: scope the gpt-oss supported-backends assert with the same platform predicate; a plain revert would fail on MPS, where nothing sets attention_backend before this point (torch_native fills later) - environ.py: drop the SGLANG_MLX_TEST_* registry entries; the e2e test now reads them via os.environ, matching the qwen MoE correctness tests - trim the MLX stub docstring and its contract-test comments; move WINDOW_SIZE_ATTRS next to the other *_ATTRS constants
# Conflicts: # python/sglang/srt/server_args.py # test/registered/unit/hardware_backend/mlx/test_attention_patching.py
…suites pr-test-mlx.yml selects stage A tests via run_suite.py suite registration since sgl-project#30121. Register test_sliding_window_attention under stage-a-unit-test-mlx and test_gpt_oss_mlx_correctness under stage-b-e2e-mlx, mirroring the existing MLX test registrations.
|
@yeahdongcn The branch is now up to date with the latest main, and the tests are registered with the #30121 suite mechanism: |
|
/tag-and-rerun-ci |
# Conflicts: # python/sglang/srt/arg_groups/overrides.py
…ests sgl-project#30050 exempted MPS from gpt-oss's attention-backend override and from its supported-backend assert, gating both on is_mps() alone. Only the MLX runner can actually serve gpt-oss on macOS -- it owns attention -- so macOS *without* MLX fell through to torch_native, which has neither sliding-window nor attention-sink support. It used to fail fast with a backend error. Both sites now gate on is_mps() and use_mlx(). Verified on an M4 Pro: SGLANG_USE_MLX=0 -> attention_backend='triton' (was 'torch_native') SGLANG_USE_MLX=1 -> attention_backend='torch_native' (unchanged; MLX owns it) test_metal_profiler: the MPS manager drives MetalCaptureProfiler.start_mps, which calls torch.mps.profiler.metal_capture -- but both tests patched mx.metal.start_capture, the *MLX* strategy's entry point. The real Metal capture therefore ran and failed with "Capture layer is not inserted" unless MTL_CAPTURE_ENABLED=1 was set. The success test failed outright; the failure test passed for the wrong reason, so it now asserts the injected message. test_batched_decode_matches_solo: the horizon is fixed so batch composition cannot change mid-run, which walks past EOS on short answers. There the distribution is near-degenerate and batched vs solo argmax can split on a numerical tie -- float reduction order between a padded batched SDPA and an unpadded solo one, not state bleed. Measured on the fixture: case 1 reaches EOS at index 2 and first differs at index 6; truncating at EOS makes all three cases agree. Comparison now stops at the first EOS. test/registered/unit/hardware_backend/mlx/ is now fully green: 186 passed, 2 skipped, 13 subtests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…scale (sgl-project#30050) Co-authored-by: R0CKSTAR <yeahdongcn@gmail.com> Co-authored-by: Alex Nails <alex.nails@radixark.ai>
…scale (sgl-project#30050) Co-authored-by: R0CKSTAR <yeahdongcn@gmail.com> Co-authored-by: Alex Nails <alex.nails@radixark.ai>
…scale (sgl-project#30050) Co-authored-by: R0CKSTAR <yeahdongcn@gmail.com> Co-authored-by: Alex Nails <alex.nails@radixark.ai>
…scale (sgl-project#30050) Co-authored-by: R0CKSTAR <yeahdongcn@gmail.com> Co-authored-by: Alex Nails <alex.nails@radixark.ai>
Motivation
Part of #19137 (gpt_oss.py entry).
gpt-oss (
GptOssForCausalLM) could not be served on the MLX backend:find_attention_layersraisedValueErrorat load because mlx_lm's gpt_ossAttentionBlocknames its softmax scalesm_scalewhile the attention contract requiredscale. Fixing the load alone is not enough — gpt-oss interleaves sliding-window (window=128) and full-attention layers and uses per-head attention sinks, and the MLX backend had no sliding-window support at all:make_maskignoredwindow_size, so sliding-window layers silently degraded to full attention during prefill/extend. Sequences up to 128 tokens produce identical output either way (the window never engages), which makes this failure mode invisible to short-prompt tests.inner.scaledirectly._freqswith anmscalefactor applied outsidemx.fast.rope; the kernel would have silently computed vanilla RoPE with base=10000.server_args.pyforcesattention_backend=tritonwhen nothing else matches, and on macOS the scheduler then either crashed constructingTritonAttnBackendagainst the MLX stub's dummy KV pool or routedwrite_cache_indicesto a triton kernel launch.Modifications
All behavior changes are in
python/sglang/srt/hardware_backend/mlx/, plus two small mps-only carve-outs:kv_cache/attention_contract.py: the scale requirement accepts any of("scale", "sm_scale")(SCALE_ATTRS,get_attention_scale()). Newget_layer_window_sizes()reads the mlx-lm container convention —layer_types(with"sliding_attention"marking windowed layers) plus a scalar window named eitherwindow_size(gpt_oss, gemma4) orsliding_window(olmo3, llama SWA variants).kv_cache/attention_kv_cache.py: module-levelmake_attention_mask()mirrors mlx_lmcache.create_attention_maskexactly —window_sizeproduces a banded mask (including for N == 1) via mlx_lm's owncreate_causal_mask; all three cache shims delegate to it. Previously they returnedNone/"causal"unconditionally.kv_cache/attention_wrapper.py:MLXAttentionWrappertakes awindow_size. Batched decode truncates each request's KV to the trailingmin(window, seq_len)tokens and rebuilds the padding mask for the windowed lengths (the context's shared padding metadata is full-length); attention sinks are passed through tomx.fast.scaled_dot_product_attentionwhen the module has them; the scale is resolved viaget_attention_scale. Models without a window take the exact previous path.kv_cache/model_patching.py: wires per-layer window sizes into the wrappers; warns when a model declares a scalar window without alayer_typesmap (gemma3-style pattern models), where batched decode cannot apply the window yet.aot.py:_build_rope_kernelnow rejects scaled RoPE variants — missingbase, precomputed_freqs,mscale != 1, or a linearscale != 1fall back tomx.fast.ropeinstead of computing wrong rotations when the opt-in kernel is enabled.model_runner_stub.py: overridesinit_attention_backends()as a no-op (attn_backend = None). MLX performs attention itself; the base implementation constructs whatever backendserver_argsnames, and model-specific defaults can force one whose__init__reads real KV buffers (gpt-oss → triton → crash on the stub's_DummyKVCache).server_args.py: the GptOss attention-backend forcing and supported-backends assert are skipped only when serving through MLX on Apple Silicon (is_mps() and use_mlx()); the platform default (torch_native) then applies, exactly as for other MLX-served models. The non-MLX macOS torch path keeps failing fast as before, and CUDA/ROCm/XPU/CPU behavior is unchanged.environ.py: the MLX correctness-test hooks (SGLANG_MLX_TEST_MODEL/_MEM_FRACTION/_MIN_FREE_GB) are registered asEnvFielddescriptors per the env-var conventions.Design: sliding-window layers keep the full KV history in the pool and the window is applied at read time — banded mask in prefill, trailing-window truncation in decode. For softmax attention this is numerically identical to a rotating cache (masked positions do not contribute), and it keeps radix prefix reuse and chunked prefill working unchanged. The cost is that sliding layers store KV they will not read again; a per-layer windowed pool is left as a TODO.
Test
test/registered/unit/hardware_backend/mlx/test_sliding_window_attention.py, 24 tests,base-a-test-cpu, skipped without mlx): contract acceptance for a tiny random-weight gpt_oss;get_attention_scale/get_layer_window_sizes(both window attr namings, empty-convention fallback, the window-without-layer_types warning); shimmake_maskpinned against mlx_lm's ownKVCache.make_maskacross an N/offset/window/return_array grid, plus explicit banded-semantics checks (window includes self, N==1 stays banded);_batched_decodeagainst a hand-built decode reference (full untruncated KV + mlx_lm's banded mask vs the wrapper's truncation + local padding mask) for ragged batches crossing the window, all-past-window unequal lengths, single request, and a full-attention layer; sdpasinks=semantics vs manual softmax-with-sink-column; AOT RoPE gating (vanilla accepted; YarnRoPE, missing base,_freqs,mscale, linearscalerejected). The stub override is drift-guarded intest_mlx_runner_pool_contract.py.test/registered/mlx/models_e2e/test_gpt_oss_mlx_correctness.py, structure follows the qwen MoE MLX tests from [MLX] Add correctness tests for qwen2_moe and qwen3_moe #29440): black-box serving smoke including a >128-token prompt, and token-for-token equivalence ofMlxModelRunnergreedy decoding against raw, unpatched mlx_lm greedy generation. Prompts are asserted to be >128 tokens (below that the window never engages and the test would pass vacuously) and ≤2048 (past mlx_lm's prefill chunking the RotatingKVCache reference trims differently). Registered onbase-a-test-cpu; skips wherever mlx is absent (all current CI runners) and runs for real on Apple Silicon.mlx-community/gpt-oss-20b-MXFP4-Q8, mlx 0.31.2 / mlx_lm 0.31.3):sglang servewithSGLANG_USE_MLX=1answers chat completions correctly, including the >128-token sliding-window prompt (3/3 tests).mlx_lm generateon the same checkpoint: 88.8 tok/s decode, 12.2 GB peak.test_attention_patching::TestMlxOverlapScheduleronforward_ct,test_metal_profiler) fail identically on current main without this PR.Accuracy Tests
Covered by the token-for-token reference equivalence above: greedy decoding through the SGLang MLX backend is exactly the unpatched mlx_lm output for gpt-oss (sliding-window + sinks engaged) and stays exact for the qwen MoE regressions.
Speed Tests and Profiling
No perf-sensitive path changes for existing models: the no-window decode path is byte-identical logic, and windowed models did not run before. Windowed decode attends over at most
windowkeys per sliding layer, which is strictly less work than the full-attention fallback it replaces.Checklist
cc @yeahdongcn
CI States
Latest PR Test (Base): ⏳ Run #31347160622
Latest PR Test (Extra): ❌ Run #31347160551