Skip to content

[Fix] deterministic: Validate the per-phase attention backends - #44438

Merged
1am9trash merged 3 commits into
sgl-project:mainfrom
aarnetalman:deterministic-validate-phase-attention-backends
Oct 30, 2026
Merged

1am9trash merged 3 commits into
sgl-project:mainfrom
aarnetalman:deterministic-validate-phase-attention-backends

Conversation

@aarnetalman

@aarnetalman aarnetalman commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

Deterministic mode checked only --attention-backend, but this does not guarantee that prefill and decode attention are deterministic: --prefill-attention-backend and --decode-attention-backend override it for their phase, and they were not checked. This PR adds the check for both flags. Without it, deterministic mode could be on while prefill or decode used a backend that is not deterministic, with no warning.

For example, on MI350X this starts without an error and runs AITER's paged-attention kernels for decode, although aiter is not on the deterministic list:

python3 -m sglang.launch_server --model-path Qwen/Qwen3-8B \
  --attention-backend triton --decode-attention-backend aiter \
  --enable-deterministic-inference

The same applies on every platform and to any backend outside the list (for example flashmla or trtllm_mha for one phase on NVIDIA).

Which models this affects. All of them: the check is model-independent. It matters wherever the per-phase flags are set: AMD serving configurations use them to pick AITER per phase (the MI35x configuration for Qwen3.8 sets --prefill-attention-backend aiter --decode-attention-backend aiter), and some model defaults set them (Kimi-K3, below). Without this check, such a configuration silently loses the deterministic guarantee.

AITER decode attention is in fact batch-invariant on gfx950; #44489 allows it there on top of this PR, with the evidence. This PR only makes deterministic mode check the per-phase flags at all.

Modifications

  • _deterministic_attention_backend (in arg_groups/overrides.py): in deterministic mode, also check prefill_attention_backend and decode_attention_backend against DETERMINISTIC_ATTENTION_BACKEND_CHOICES when they are set. The error names the flag and the backend.

  • test_model_overrides.py:

    • a non-deterministic backend in either per-phase flag is refused, even when --attention-backend is on the list;
    • per-phase backends that are on the list are accepted.
  • Unchanged:

    • deterministic mode off;
    • per-phase flags unset (the existing fallback for --attention-backend is untouched);
    • which backends are on the list.
  • Scope: the check runs where deterministic mode is resolved, so it covers per-phase backends that are set explicitly or by model-specific defaults before it. Two later automatic choices are not covered here:

    • the NVFP4 KV-cache prefill/decode pair;
    • CuteDSL MLA's trtllm_mla prefill pairing.

    Both are tied to opt-in features (NVFP4 KV cache, CuteDSL MLA decode). Covering them needs a check after those passes; I'd leave that to a follow-up.

  • Behaviour change: configurations that combine --enable-deterministic-inference with a per-phase backend outside the list now fail at startup instead of running without the deterministic guarantee. This includes model defaults that set such a backend, for example Kimi-K3's backends for decode context parallelism.

Accuracy Tests

No numerical change: this only refuses configurations at startup.

Unit tests, python3 -m pytest test/registered/unit/test_model_overrides.py -k deterministic (MI350X, rocm/sgl-dev:v0.5.21-rocm724-mi35x-20261013): before, the new refusal test fails for both flags (2 failed, 5 passed); after, 5 passed.

Speed Tests and Profiling

No runtime change: the check runs once while the server arguments are resolved.

Checklist

Review and Merge Process

  1. Ping Merge Oncalls to start the process. See the PR Merge Process.
  2. Get approvals from CODEOWNERS and other reviewers.
  3. Trigger CI tests with comments or contact authorized users to do so.
    • Common commands include /tag-and-rerun-ci, /tag-run-ci-label, /rerun-failed-ci
  4. After green CI and required approvals, ask Merge Oncalls or people with Write permission to merge the PR.

CI States

Latest PR Test (Base): ✅ Run #40817696448
Latest PR Test (Extra): ❌ Run #40817696393
Latest PR Test (AMD ROCm 10): ✅ Run #40817696482

@rkarhila-amd rkarhila-amd left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another tiny PR that ensures users don't mess up deterministic inference by making bad choices.

I'd personally have combined with #44701, but I don't make the policies for PR scope.

@aarnetalman

Copy link
Copy Markdown
Contributor Author

Another tiny PR that ensures users don't mess up deterministic inference by making bad choices.

I'd personally have combined with #44701, but I don't make the policies for PR scope.

Thanks! I kept them separate because #44489 is stacked on #44438, and folding the mixed-chunk change in would make it depend on that too; this way each can be reviewed and merged on its own. Happy to combine them if the maintainers prefer one PR.

@aarnetalman
aarnetalman marked this pull request as ready for review October 16, 2026 08:44
@aarnetalman
aarnetalman force-pushed the deterministic-validate-phase-attention-backends branch from 1d13627 to 022b79d Compare October 21, 2026 13:00
@github-actions

This comment has been minimized.

--prefill-attention-backend and --decode-attention-backend take priority
over --attention-backend for their phase, but deterministic inference only
checked --attention-backend against its allowlist. A non-deterministic
prefill or decode backend was therefore accepted without an error. Check
both against the same list.
@aarnetalman
aarnetalman force-pushed the deterministic-validate-phase-attention-backends branch from 022b79d to 14c9e46 Compare October 21, 2026 14:52
@1am9trash 1am9trash added the run-ci CI: run the baseline test suite on this PR label Oct 28, 2026
@github-actions

This comment has been minimized.

@aarnetalman

Copy link
Copy Markdown
Contributor Author

/rerun-failed-ci

1 similar comment
@aarnetalman

Copy link
Copy Markdown
Contributor Author

/rerun-failed-ci

@aarnetalman

Copy link
Copy Markdown
Contributor Author

/rerun-failed-ci

1 similar comment
@aarnetalman

Copy link
Copy Markdown
Contributor Author

/rerun-failed-ci

@1am9trash

Copy link
Copy Markdown
Collaborator

@1am9trash 1am9trash left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
Add a guard on separated backend.

@1am9trash
1am9trash merged commit 56a8d3a into sgl-project:main Oct 30, 2026
340 of 382 checks passed
Yuxingwang-intel pushed a commit to Yuxingwang-intel/sglang that referenced this pull request Nov 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-ci CI: run the baseline test suite on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants