Skip to content

Skip the expert-parallel sentinel masking when expert parallelism is off - #48201

Merged
qgallouedec merged 21 commits into
mainfrom
moe-skip-ep-sentinels-v2
Sep 10, 2026
Merged

qgallouedec merged 21 commits into
mainfrom
moe-skip-ep-sentinels-v2

Conversation

@qgallouedec

@qgallouedec qgallouedec commented Aug 21, 2026

Copy link
Copy Markdown
Member

CPU CI GPU run-slow

grouped_mm_experts_forward runs the expert-parallel sentinel dance unconditionally:

sentinel_mask = (expert_ids_g >= self.num_experts).unsqueeze(-1)
expert_ids_g.clamp_(max=self.num_experts - 1)

selected_hidden_states_g.masked_fill_(sentinel_mask, 0.0)

weighted_out.masked_fill_(sentinel_mask, 0.0)

Sentinel ids are produced only by RouterParallel._prepare_output_fn, i.e. only under expert parallelism. Without it the router's topk cannot return an index >= num_experts, so sentinel_mask is all-False and the two masked_fill_ are pure memory traffic, on tensors the size of the expert activations, twice in the forward and twice again in the backward.

This skips the whole thing when expert parallelism is off. The flag defaults to False where the experts module is built, and is set to True in MoEParamShard.shard_param when it shards the expert dimension, i.e. on the grouped_gemm plan entry that every expert-parallel plan carries. That is also where num_experts is rewritten to the per-rank count the sentinel mask compares against, so the two cannot disagree. A module-level plan entry would not do: llama4's expert-parallel plan has ep_router and grouped_gemm but no moe_tp_experts. The expert-parallel path is unchanged.

Numbers

One Qwen3MoeSparseMoeBlock from Qwen/Qwen3-30B-A3B (128 experts, top-8, hidden=2048), seq 4096, bf16, fwd+bwd, single H100, grouped_mm:

fwd+bwd peak memory
main 6.18 ms 3.46 GB
this PR 5.22 ms (1.18×) 3.46 GB
bench_sentinel.py — the script that produced the table
# bench_sentinel.py — cost of the expert-parallel sentinel masking when EP is off.
import statistics
import time

import torch
from transformers import AutoConfig
from transformers.models.qwen3_moe.modeling_qwen3_moe import Qwen3MoeSparseMoeBlock

MODEL, SEQ, ITERS = "Qwen/Qwen3-30B-A3B", 4096, 30

cfg = AutoConfig.from_pretrained(MODEL)
cfg._experts_implementation = "grouped_mm"
block = Qwen3MoeSparseMoeBlock(cfg).cuda().to(torch.bfloat16)
x = torch.randn(1, SEQ, cfg.hidden_size, device="cuda", dtype=torch.bfloat16, requires_grad=True)


def run():
    torch.cuda.synchronize()
    t = time.perf_counter()
    out = block(x)
    (out[0] if isinstance(out, tuple) else out).sum().backward()
    torch.cuda.synchronize()
    return time.perf_counter() - t


for _ in range(5):
    run()
times = [run() for _ in range(ITERS)]
torch.cuda.reset_peak_memory_stats()
run()
print(f"is_expert_parallel={getattr(block.experts, 'is_expert_parallel', 'n/a')}  "
      f"{statistics.median(times) * 1e3:6.2f} ms/iter   {torch.cuda.max_memory_allocated() / 2**30:5.2f} GB")

End to end on 2×H100 with FSDP2 (Qwen/Qwen3-30B-A3B, seq 4096, per-device batch 2, LoRA r=16 on attention, chunked_nll):

step tokens/s/GPU peak
main 1.927 s 4252 41.3 GB
this PR 1.798 s 4555 (1.07×) 41.3 GB

The same 1.07× was measured before #47579 refactored this code, on an 8×H100 run of the same model, and 1.05× on Qwen3.6-35B-A3B.
In an 8×H100 profile of that model, aten::masked_fill_ was 2.0% of all GPU kernel time at per-device batch 1 and 3.8% at batch 4, entirely from these two calls.

`grouped_mm_experts_forward` builds a sentinel mask and runs two `masked_fill_`
on tensors the size of the expert activations, on every forward and again on the
backward. Sentinel ids only ever come from `RouterParallel`, i.e. only under
expert parallelism; without it the mask is all-False and the work is pure memory
traffic. Default the flag off where the experts module is built and set it where
`MoeExpertsParallel` installs its forward.
@HuggingFaceDocBuilderDev

Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@qgallouedec

qgallouedec commented Aug 27, 2026

Copy link
Copy Markdown
Member Author

CI was right, this was broken. The gate is never True.

install_forward has a single call site and it doesn't pass is_expert_parallel, so it always takes the False default. The masking was skipped unconditionally, including under EP, which is exactly when it's needed.

Repro, 2 GPUs, torchrun --nproc_per_node=2 repro.py:

# torchrun --nproc_per_node=2 repro.py
import os
import tempfile

import torch
import torch.distributed as dist

from transformers import AutoModelForCausalLM, Qwen3MoeConfig
from transformers.distributed import DistributedConfig

dist.init_process_group(backend="nccl")
rank, world = dist.get_rank(), dist.get_world_size()
torch.cuda.set_device(rank)

path = os.path.join(tempfile.gettempdir(), "tiny_qwen3_moe")
if rank == 0:
    config = Qwen3MoeConfig(
        vocab_size=256, hidden_size=128, intermediate_size=256, moe_intermediate_size=64,
        num_hidden_layers=2, num_attention_heads=4, num_key_value_heads=2,
        num_experts=8, num_experts_per_tok=2, decoder_sparse_step=1, tie_word_embeddings=False,
    )
    torch.manual_seed(0)
    AutoModelForCausalLM.from_config(config, dtype=torch.bfloat16).save_pretrained(
        path, save_original_format=True
    )
dist.barrier()

model_ep = AutoModelForCausalLM.from_pretrained(
    path, dtype=torch.bfloat16,
    distributed_config=DistributedConfig(tp_size=world, enable_expert_parallel=True),
)
dist.barrier()
model_ref = AutoModelForCausalLM.from_pretrained(path, dtype=torch.bfloat16).to(model_ep.device)

torch.manual_seed(0)
input_ids = torch.randint(0, 256, (2, 64), device=model_ep.device)
with torch.no_grad():
    logits_ref = model_ref(input_ids).logits
    logits_ep = model_ep(input_ids).logits

if rank == 0:
    experts = model_ep.model.layers[0].mlp.experts
    print(f"experts implementation: {model_ep.config._experts_implementation}")
    print(f"is_expert_parallel:     {getattr(experts, 'is_expert_parallel', '<absent>')}")
    print(f"NaN in EP logits:       {bool(torch.isnan(logits_ep).any())}")
    print(f"max abs diff vs non-EP: {(logits_ref.float() - logits_ep.float()).abs().max().item()}")

dist.destroy_process_group()

On this branch:

experts implementation: grouped_mm
is_expert_parallel:     False
NaN in EP logits:       True
max abs diff vs non-EP: nan

With the flag actually plumbed through:

experts implementation: grouped_mm
is_expert_parallel:     True
NaN in EP logits:       False
max abs diff vs non-EP: 0.0

On main (no gate, masking unconditional) the same script gives no NaN and 0.0.

Fix is to pass the flag from apply_tensor_parallelism, where model.config.distributed_config.enable_expert_parallel is already set, plus an is_expert_parallel=False kwarg on the base TensorParallelLayer.install_forward and the two overrides that share that call site. 6 lines.

Worth flagging separately: the EP mixin tests pass on CPU even without the fix, so the CPU suite doesn't cover this path. Only the GPU run above surfaces it.

Comment thread src/transformers/integrations/moe.py
@IlyasMoutawwakil

Copy link
Copy Markdown
Member

thanks for the optimization ! i remember measuring it for inference only and seeing only noise 🥲

@IlyasMoutawwakil

Copy link
Copy Markdown
Member

not sure but i think we can probably know if EP is enabled by checking for DTensor + sharding dim (0) of gate_up/down proj

@IlyasMoutawwakil

Copy link
Copy Markdown
Member

btw there are other implementations that do the same, in finegrained-fp8 and deepgemm

Comment thread src/transformers/integrations/moe.py Outdated
@qgallouedec

Copy link
Copy Markdown
Member Author

@IlyasMoutawwakil I tried the DTensor+Shard(0) derivation and it can't work: _use_local_dtensor_params wraps the experts forward under EP, so inside the forward the weights are already plain local tensors and the check reads False when EP is on.

(to verify: an in-forward probe shows type(gate_up_proj) == Tensor under EP, and the skipped clamp makes batched_mm crash with an OOB gather.)

Kept the explicit attribute instead, applied your other two points: the batched_mm clamp is now gated too, and the same skip is applied in finegrained-fp8 and deepgemm.

qgallouedec and others added 3 commits September 4, 2026 23:11
The finegrained-fp8 and deepgemm forwards now read `self.is_expert_parallel`,
which real experts modules get from `use_experts_implementation.__init__`. The
kernel tests build their stand-ins as a `SimpleNamespace` and bypass the
decorator, so all nine of them raised `AttributeError`.

`_build_experts` carries the flag like the decorator does, and the two tests
that exercise sentinels set it to `True`, since sentinels only exist under
expert parallelism.
@qgallouedec

qgallouedec commented Sep 7, 2026

Copy link
Copy Markdown
Member Author

CI was red on nine kernel tests, all the same cause: the tests build their experts stand-in as a SimpleNamespace, so it never goes through use_experts_implementation.__init__
Fixed in eb9dafc, full tests/kernels/ run should be green now.

@VI-Arthur VI-Arthur 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.

LGTM, much needed indeed cc @IlyasMoutawwakil

Comment thread src/transformers/distributed/tensor_parallel.py Outdated
Comment thread src/transformers/distributed/tensor_parallel.py Outdated
qgallouedec and others added 4 commits September 8, 2026 16:38
…as is

Wiring is_expert_parallel through the TP styles reached further than
intended. It is read at tensor_parallel.py:601, where a true value skips the
_AllReduceBackward on top_k_weights, so the router gradients would stop
being summed across the group under expert parallelism. Nothing else sums
them. That path now behaves exactly as on main.

The flag also has to reach the experts module, and a module-level plan entry
does not: llama4's expert-parallel plan has ep_router and grouped_gemm but
no moe_tp_experts, so the experts would never have been marked. Set it in
MoEParamShard.shard_param under shards_expert_dim, which is the grouped_gemm
entry every expert-parallel plan carries, and which already rewrites the
num_experts the sentinel mask compares against.

The masking now tests the mask rather than the flag, so the two halves,
forty lines apart in finegrained_fp8, cannot disagree.
@qgallouedec

Copy link
Copy Markdown
Member Author

@VI-Arthur needs another look, the version you approved had a bug.

Passing the flag through install_forward also reached

if not is_expert_parallel:
top_k_weights = _AllReduceBackward.apply(top_k_weights, tp_group)

where a true value skips the _AllReduceBackward on top_k_weights: router grads would stop being summed under EP. That path is back to main.

Flag is now set in MoEParamShard.shard_param under shards_expert_dim, next to the num_experts rewrite the mask compares against. A module-level entry doesn't work, llama4's ep_plan has no moe_tp_experts. That's the bit I'd like your eyes on.

Also dropped the parameter from the three overrides that ignored it, per your comment. tensor_parallel.py is now main + 2 lines.

@IlyasMoutawwakil IlyasMoutawwakil left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm ! just make sure the new flag will stay relevant in the a2a PR where there are no sentinels iiuc

Comment thread src/transformers/distributed/tensor_parallel.py Outdated
qgallouedec and others added 2 commits September 10, 2026 11:26
The experts forwards read it at call time so it has to live on the module, but nothing outside them
needs it, so it is `_is_expert_parallel` now.
@github-actions

Copy link
Copy Markdown
Contributor

CI recap

Dashboard: View test results in Grafana
Latest run: 34508019559:1
Result: failure | Jobs: 16 | Tests: 187,513 | Failures: 1 | Duration: 16h 24m

@qgallouedec
qgallouedec added this pull request to the merge queue Sep 10, 2026
Merged via the queue into main with commit 606e6e8 Sep 10, 2026
215 of 217 checks passed
@qgallouedec
qgallouedec deleted the moe-skip-ep-sentinels-v2 branch September 10, 2026 18:09
@qgallouedec
qgallouedec restored the moe-skip-ep-sentinels-v2 branch September 10, 2026 19:12
@qgallouedec
qgallouedec deleted the moe-skip-ep-sentinels-v2 branch September 10, 2026 19:15
sbucaille pushed a commit to sbucaille/transformers that referenced this pull request Sep 16, 2026
…off (huggingface#48201)

* Skip the expert-parallel sentinel masking when expert parallelism is off

`grouped_mm_experts_forward` builds a sentinel mask and runs two `masked_fill_`
on tensors the size of the expert activations, on every forward and again on the
backward. Sentinel ids only ever come from `RouterParallel`, i.e. only under
expert parallelism; without it the mask is all-False and the work is pure memory
traffic. Default the flag off where the experts module is built and set it where
`MoeExpertsParallel` installs its forward.

* Drop the explanatory comments; the reasoning is in the PR

* Pass is_expert_parallel through to the experts TP style

* Gate the batched_mm sentinel clamp and extend the EP-off skip to finegrained-fp8 and deepgemm

* Apply ruff format to the deepgemm call sites

* Carry is_expert_parallel on the test experts stand-ins

The finegrained-fp8 and deepgemm forwards now read `self.is_expert_parallel`,
which real experts modules get from `use_experts_implementation.__init__`. The
kernel tests build their stand-ins as a `SimpleNamespace` and bypass the
decorator, so all nine of them raised `AttributeError`.

`_build_experts` carries the flag like the decorator does, and the two tests
that exercise sentinels set it to `True`, since sentinels only exist under
expert parallelism.

* Set is_expert_parallel at the call site instead of threading a parameter

* Set the flag where the experts are sharded, and keep the router path as is

Wiring is_expert_parallel through the TP styles reached further than
intended. It is read at tensor_parallel.py:601, where a true value skips the
_AllReduceBackward on top_k_weights, so the router gradients would stop
being summed across the group under expert parallelism. Nothing else sums
them. That path now behaves exactly as on main.

The flag also has to reach the experts module, and a module-level plan entry
does not: llama4's expert-parallel plan has ep_router and grouped_gemm but
no moe_tp_experts, so the experts would never have been marked. Set it in
MoEParamShard.shard_param under shards_expert_dim, which is the grouped_gemm
entry every expert-parallel plan carries, and which already rewrites the
num_experts the sentinel mask compares against.

The masking now tests the mask rather than the flag, so the two halves,
forty lines apart in finegrained_fp8, cannot disagree.

* Drop the dead default on _dispatch_routed_input

Both call sites pass it.

* Keep the expert-parallel flag off the public module surface

The experts forwards read it at call time so it has to live on the module, but nothing outside them
needs it, so it is `_is_expert_parallel` now.
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.

4 participants