feat(models): add native GPT-OSS implementation - #3424
Conversation
4480252 to
501d435
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4097a00. Configure here.
| """Route GPT-OSS BF16 expert and sink weights through vLLM loaders. | ||
|
|
||
| Remove this patch when the pinned vLLM version uses weight loaders for these | ||
| prepared tensors in ``GptOssModel._load_weights_other``. |
There was a problem hiding this comment.
What exactly was the problem here? The change in dtype was causing issues?
|
|
||
| def hf_to_prime(self, state_dict: dict[str, Tensor]) -> None: | ||
| gate_up_name = f"{self.prefix}.gate_up_proj" | ||
| if gate_up_name not in state_dict: |
There was a problem hiding this comment.
why is this early return and the one below correct? Not obvious, to me.
garrett361
left a comment
There was a problem hiding this comment.
LGTM. Checked the CP sink handling for ulysses, in particular.
Just had some minor questions.
#3424 removed the gpt-oss optional-dependency group from pyproject.toml, but Dockerfile.cuda still passed --extra gpt-oss to uv sync, which fails on undefined extras and broke every image build since. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Build-only Dockerfile alignment with pyproject extras; no application or dependency logic changes beyond fixing the sync command. > > **Overview** > Repairs **CUDA Docker image builds** that broke after the `gpt-oss` optional extra was removed from `pyproject.toml` (#3424). > > Both `uv sync` steps in `Dockerfile.cuda` (the cached third-party layer and the full workspace sync after sources are copied) no longer pass `--extra gpt-oss`. **`uv` was failing** on an undefined extra; the remaining extras (`gpu`, flash-attn variants, `disagg`, `quack`, `kernels`, `mamba-ssm`, etc.) are unchanged. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit b340617. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: Johannes Hagemann <johannes@Johanness-MacBook-Pro.local> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>

Summary
Replace the Transformers/hub-kernel GPT-OSS runtime with a Prime-owned model, FA4 attention with learned sinks, and canonical grouped experts. BF16 GPT-OSS checkpoints load through lossless expert-layout conversion; inference policy updates route prepared expert and sink tensors through vLLM's registered parameter loaders.
This branch targets
mainand supports runtime QKV and gate/up fusions, canonical checkpointing, and the CP capability interface. Main's dependency pins are retained; the obsoletegpt-osshub-kernel extra is removed.Supported configuration
model.impl = "auto"selects native GPT-OSS. Validated checkpoint:unsloth/gpt-oss-20b-BF16. Original MXFP4 checkpoint loading is outside this change.(output, lse); FA2/FA3 default calls return a tensor.GPT-OSS and shared FA4 attention no longer use
torch._dynamo.disable. There is no forwarding wrapper around the native FA4 function. Native configuration/input guards and HF GPT-OSS execution-comparison tests are removed; checkpoint conversion and CP checks remain. There are no example or skill changes relative to main.Context-parallel attention
Every CP helper replaces an attention compute method. GPT-OSS's ring adapter is separate because the shared ring custom operators have no learned-sink input or sink-gradient output. It uses differentiable K/V gathers and FA4 to preserve those gradients. Its Ulysses adapter passes the model's FA4 function, scale, and sinks into the shared sink-aware Ulysses implementation. That thin adapter is an organizational choice; directly assigning the existing generic closure would omit these arguments.
Validation
torch.compile(fullgraph=False)forward/backward and eager-output agreement passed for GPT-OSS and shared FA2/FA3/FA4 attention.Final 20-step H200 RL run
After all code changes,
uv run rl @ config.tomlcompleted through the standard Slurm launcher on eight H200 GPUs: four trainer and four inference GPUs,unsloth/gpt-oss-20b-BF16, reverse-text, batch 64, group 8, sequence length 1024, completion budget 512, EP=4, QKV/gate-up fusions, compilation, selective activation checkpointing, and NCCL policy reloads. Rollout concurrency starts at 32 and is capped at 64.Slurm job 326 completed with exit code 0, in 24m42s. All 20 optimizer steps completed and inference acknowledged 21 successful reloads (startup plus 20 updates). Rollout errors were 0%. The short completion budget truncates most reasoning; this validates numerical consistency and pipeline operation.
The table reports
mismatch_kl/all/meanon commitc3fe30c29.Completed steps: 20
Mean of step means: 0.0014423477
Min: 0.0007825278
Max: 0.0027635854
Note
Medium Risk
Touches core training attention/MoE and vLLM weight loading for a new architecture; mistakes could affect numerics or checkpoint load, though conversion and CP tests mitigate this.
Overview
Replaces the Transformers / hub-kernel GPT-OSS path with a Prime-owned
gpt_ossstack: custom config, packed-batch forward (seq_lens), grouped MoE, and FlashAttention 4 attention with learnable sink parameters wired through ring and Ulysses context parallelism (including sink gradients).Training/runtime: Drops Hopper-only checks and explicit hub-kernel enablement; hub kernels stay disabled globally. Shared FA4 call sites now handle
(output, lse)returns and droptorch._dynamo.disablewrappers. Ulysses varlen attention gains optionallearnable_sinksharding.Inference: Registers a vLLM startup patch to load prepared BF16 MoE expert and
.sinkstensors via weight loaders until upstream vLLM catches up.Packaging/docs: Removes the
gpt-ossoptional extra (kernelsdependency); docs point at BF16 checkpoints (e.g.unsloth/gpt-oss-20b-BF16) and SM90/SM100+ for FA4 sinks. Tests add 2-GPU ring/Ulysses CP checks and drop HF MoE parity.Reviewed by Cursor Bugbot for commit c3fe30c. Bugbot is set up for automated code reviews on this repo. Configure here.