Skip to content

[MoonEP] Fix BF16 physical row semantics and unsupported config checks - #36133

Open
charliechenye wants to merge 3 commits into
sgl-project:mainfrom
charliechenye:work/moonep-correctness-35783
Open

charliechenye wants to merge 3 commits into
sgl-project:mainfrom
charliechenye:work/moonep-correctness-35783

Conversation

@charliechenye

@charliechenye charliechenye commented Aug 24, 2026

Copy link
Copy Markdown

Motivation

Part of #35783.

The MoonEP BF16 reference path added in #33249 currently conflates physical MoonEP VM groups with logical expert identities in the expert runner. For prefetched experts, MoonEP copies a logical source expert into a physical prefetch slot E + b, so the BF16 runner should execute that physical slot directly rather than remapping it back to the logical source row.

This PR tightens that correctness boundary and makes unsupported configurations fail early. It also removes the silent rank 0 fallback when resolving logical expert metadata.

This is intentionally a narrow correctness and support-contract change. It does not add MXFP4/DeepGEMM support, asynchronous overlap, CUDA graph support, lifecycle handling, or observability.

Modifications

  • Separate physical VM group rows from logical expert identities:

  • Remove the silent rank 0 fallback:

    • logical expert metadata now uses dist.get_rank(group=self.group) directly
    • rank lookup failures are no longer silently replaced with rank 0
  • Reject unsupported expert bias:

    • MoonEP now fails early when FusedMoE is constructed with with_bias=True
    • the check is attached to the MoonEP backend rather than the current BF16 weight-storage policy
  • Reject unsupported overlap modes:

    • single-batch overlap is rejected for MoonEP
    • two-batch overlap is rejected for MoonEP
    • the existing behavior that disables prefill and decode CUDA graphs for MoonEP is preserved
  • Add focused unit coverage for:

    • BF16 compute using physical VM group rows
    • logical expert metadata for normal and prefetched groups
    • unsupported expert bias
    • unsupported single-batch and two-batch overlap

This PR intentionally does not change padded-route accounting. Excluding padded rows only from tokens_per_expert while still passing the static padded top-k tensor would make MoonEP's planner inputs inconsistent. A correct solution requires an explicit MoonEP planner/API contract for inactive static rows.

Accuracy Tests

Focused Linux unit tests pass:

python -m pytest -v \
  test/registered/unit/layers/moe/test_moonep_buffer.py \
  test/registered/unit/server_args/test_server_args.py

Result:

154 passed

The physical VM-row regression specifically covers a prefetched physical group whose logical expert ID differs from its physical weight row, and verifies that the BF16 runner uses the physical row.

Additional checks passed:

python scripts/ci/check_registered_tests.py

pre-commit run \
  --from-ref upstream/moonep \
  --to-ref HEAD

git diff --check upstream/moonep..HEAD

The BF16 integration in #33249 was previously validated on 8x H100 SXM with zero numerical error.

Distributed MoonEP validation has not yet been rerun after the physical-row correction in this PR. In particular, the stronger dispatch -> remote prefetch -> physical-slot compute -> combine validation for this change is still pending.

Speed Tests and Profiling

Not run.

This PR is a correctness and support-contract change and does not intentionally change the MoonEP communication algorithm or add a new optimized kernel path. No performance improvement is claimed.

MoonEP performance benchmarking against DeepEP is tracked separately in #35783.

Checklist

Review and Merge Process

  1. Ping Merge Oncalls to start the process. See the PR Merge Process.
  2. Get approvals from https://github.com/sgl-project/sglang/blob/main/.github/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.
    (https://docs.sglang.io/developer_guide/contribution_guide.html#write-documentations).

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): ❌ Missing run-ci label -- add it to run CI tests.
Latest PR Test (Extra): ❌ Blocked -- run-ci is required first.
Latest PR Test (AMD ROCm 7.2): ➖ No AMD PR run found for this commit.

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.

2 participants