Skip to content

Restore GPT-OSS's router-masked expert-parallel plan - #49431

Closed
IlyasMoutawwakil wants to merge 2 commits into
mainfrom
gpt-oss-ep-router-plan
Closed

IlyasMoutawwakil wants to merge 2 commits into
mainfrom
gpt-oss-ep-router-plan

Conversation

@IlyasMoutawwakil

@IlyasMoutawwakil IlyasMoutawwakil commented Oct 8, 2026 •

Copy link
Copy Markdown
Member

CPU CI GPU run-slow

The MXFP4 experts route their tokens themselves: they take the routing data routing_torch_dist builds (already restricted to the rank's experts) instead of expert ids, which token dispatch cannot exchange. With the plan on ep_dispatch_experts, MXFP4 GPT-OSS under expert parallelism failed on its first forward (ep_forward() got an unexpected keyword argument 'scatter_idx'). The router-masked plan from before #49160 leaves them to their own routing.

What does this PR do?

Fixes # (issue)

Code Agent Policy

The Transformers repo is currently being overwhelmed by a large number of PRs and issue comments written by
code agents. These often are low-quality, or fix extremely minor issues that occur rarely or never in practice.
As a result, we're instituting a rule that first-time contributors should not use code agents to submit PRs or issues.
We'd also ask autonomous "OpenClaw"-like agents not to open any PRs or issues.

Issues/PRs from first-time contributors that violate this rule will probably just be closed without review, and we
might block you, especially if you open more than one or appear to be deliberately ignoring this. We especially do not
want new contributors to jump in on random issues to contribute an agent-written fix. This creates lots of noise
for reviewers and other users and will almost certainly get you blocked.

For more information, please read CONTRIBUTING.md.

  • (First-time contributors only): I confirm that this PR description and code is not written by an LLM or code agent

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline and the
    Pull Request checks?
  • Was this discussed/approved via a Github issue or the forum? Please add a link
    to it if that's the case.
  • Did you make sure to update the documentation with your changes according to the guidelines?
  • Did you write any new necessary tests?

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

The MXFP4 experts route their tokens themselves: they take the routing data `routing_torch_dist` builds (already
restricted to the rank's experts) instead of expert ids, which token dispatch cannot exchange. With the plan on
`ep_dispatch_experts`, MXFP4 GPT-OSS under expert parallelism failed on its first forward
(`ep_forward() got an unexpected keyword argument 'scatter_idx'`). The router-masked plan from before #49160 leaves
them to their own routing.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@IlyasMoutawwakil
IlyasMoutawwakil marked this pull request as ready for review October 8, 2026 13:24
@github-actions
github-actions Bot requested a review from ArthurZucker October 8, 2026 13:25
@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.

It inherits GPT-OSS's config through its modular file, but its experts take expert ids like every other experts
class, so it keeps the `ep_dispatch_experts` plan rather than following GPT-OSS back to router masking.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

[For maintainers] Suggested jobs to run (before merge)

run-slow: gpt_oss, openai_privacy_filter

@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

CI recap

Dashboard: View test results in Grafana
Latest run: 37783949793:1
Result: success | Jobs: 8 | Tests: 493 | Failures: 1 | Duration: 2m 1s

IlyasMoutawwakil added a commit that referenced this pull request Oct 8, 2026
MXFP4 GPT-OSS experts take routing data instead of expert ids; GPT-OSS keeps the router-masked plan for them
(#49431), so masked EP only handles expert ids, and token dispatch points such experts to that plan.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
"layers": (["hidden_states", "attention_mask"], ["hidden_states"]),
"norm": (["hidden_states"], ["hidden_states"]),
}
# MXFP4 experts route their tokens themselves, which only the router-masked plan leaves them to

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.

mmm this should not be related. MXFP4 should be acting on the plan AFAIK

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

in an ideal world yes, but the current mxfp4 integration is based on kernels that aren't modular enough by default to do that. i can adapt this mxfp4 (which we will get rid of once we get to finegrained kernels) but it has to import some non-surfaced ops in the gpt oss mxfp4 kernels which turns iut to be much uglier.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

that's why i said this is temporary, until fg kernels is merged which does not have the limitations and rigidity of "triton-kernels", lmk what u prefer

@IlyasMoutawwakil

Copy link
Copy Markdown
Member Author

closing in favor of the EP strategies stack: GPT-OSS (MXFP4 included) now runs on ep_dispatch_experts in #49613, so the router-masked plan isn't needed anymore.

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.

3 participants