perf(GDN-MOE-BF16-OUT): the GDN recurrence output and the z gate are bf16 on MoE too, as both references keep them - #1223
Merged
Conversation
…ow the model dtype on EVERY arm, not only the dense one (#1168, #1169) `GdnOutDType(bool dense_model)` resolved bf16 for a dense checkpoint and f32 for a MoE one, and all three call sites passed `cfg.num_experts == 0`. So on every MoE checkpoint the GDN recurrence output `dcore` [T,Hv,Dv], the `z` output gate, and the gated-RMSNorm weight that has to match them were held at double width: each of `dcore` and `z` is stored once and loaded once, so f32 doubled four passes over the two largest per-layer GDN activations. vLLM does not branch on model shape anywhere on this path. It resolves ONE model dtype and every layer inherits it: `core_attn_out` is allocated at `hidden_states.dtype` (qwen_gdn_linear_attn.py:870-873 @ 5559679), `z` is a split of the bf16 `in_proj_qkvz` output (:843, :859-860), `RMSNormGated` is built with no dtype override (:459-465), and `Qwen3_5ForCausalLMBase` is the shared base of the dense and MoE causal-LM arms (models/qwen3_5.py:280-297). SGLang agrees, so ours was a deviation from the PRIMARY oracle rather than a disagreement between references. No gate could see it, and that is the point: a token gate cannot detect a dtype that is too wide. `test_qwen36_paged_engine` is 315/315 BECAUSE f32 is the more precise deviation. The tree derived this whole finding at qwen3_5.cpp:3172-3189 and deferred it, naming its own successor campaign; this is that campaign. The `bool dense_model` parameter is REMOVED rather than defaulted. A signature that accepts a model shape makes the default unreadable at the definition and lets a new call site reintroduce the split silently, and vLLM has no such parameter because it has no such decision. `VT_GDN_OUT_BF16=0` is unchanged as the same-binary f32 rollback, and it is now the rollback for both arms rather than for the dense one alone. The env decision moves into the pure `detail::GdnOutBf16FlagIsOn`, mirroring `PackedGdnDecodeFp8TowerFlagIsOn`, because the resolver caches its `getenv` and one process can only observe one value of it. `detail::ShouldUsePackedGdnDecode`'s `e.dense_model` term goes with it, in that order and not before. It entered at f344dec as one of that change's "real-model safety gates", was never revisited, and neither reference has an equivalent (`VLLM_ENABLE_FLA_PACKED_RECURRENT_DECODE` defaults True with no shape term; SGLang keys `supports_packed_decode` on the platform alone). Removing it FIRST would have been inert and therefore unobservable through any production entry point, since `GdnPackedDecodeDTypesCompatible` already pins `core_out` to BF16 and `core_out` is `outdt`. That is why the two edits are one change. Removing it does NOT reach packed GDN decode on a MoE checkpoint, and the pull request must not claim otherwise. `has_packed_ba` needs `in_proj_ba`, written at exactly one site in the tree — the dense loader, qwen3_5_dense_weights.cpp:436. That is #1169 and it is owed. `ShouldUseMergedGdnQkvz`'s `uniform_dtype` term flips true on a MoE model for the same reason and is inert for the same one: no MoE or GGUF loader builds the merged `in_proj_qkvz` owner either. Red first, in the order `## Tests` sets. `test_qwen35_paged_forward`'s new case enters through `ModelRegistry::Forward` on a MoE config and observed `core_out == F32`, `z_gate == F32` before the change; a test that called `GdnOutDType` by hand would have proved the predicate answers and nothing about what the model runs, so the dtypes are read off `dcore`'s own tensor and off the projected gate through a `detail::` probe. `test_qwen27_paged_forward`'s new eligibility case names no model-shape field, so it says the same thing before and after the field exists, and it returned false before Edit 2. The `rejects` case that pinned `dense_model = false` is replaced by the argument for the removal, not deleted to reach green. Three in-tree comments stated the model-shape reason for confining `VT_GDN_FP8_IN_BF16` to the 27B and are now wrong; #521 asked for exactly this correction. They are repaired here because Edit 1 is what makes them wrong: the bound is the DEFAULT-OFF toggle now, not the model shape. Nothing moves by default in either merge order. Gated on the CPU tier only. `test_qwen35_paged_forward` 5/5 cases 13/13 assertions, `test_qwen27_paged_forward` 31/31 cases 770/770 assertions, plus `test_ops_gdn`, `test_qwen27_dense_forward`, `test_qwen3_5_gdn_spec_routing`, `test_model_registry`, `test_runner` and the three expert-stream suites green; `scripts/agent-preflight.sh` exit 0. The 235/235 and 315/315 engine gates, the 35B correctness gate, the `VT_GDN_OUT_BF16=0|1` same-binary A/B and the `nsys` memory-format confirmation are all OWED to the operator on a GPU host and are recorded PENDING in the spec's `## Now`; the implementer's host has no CUDA toolchain, no GPU, and 6 GB of free disk. Nothing here is measured. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5-1m [claude-code]
`origin/main` advanced to 1f4878f while this row was being implemented, so the commit-style and commit-trailer gates skipped with "branch is behind" and reported nothing about this tree. The merge is taken so they run. What arrived touches only `.agents/` and `scripts/check-agent-record.py`; it does not overlap this row's files, and the focused CPU gate is unchanged by it. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5-1m [claude-code]
… in one direction and manufactured a red in the other (#1168, #521) Fresh review 1 confirmed both edits correct and bound by tests, and found the lever itself ungated at both ends. `GdnOutDType()` mutated to `return DType::kBF16;`, severed from `GdnOutBf16FlagIsOn` and from the variable entirely, left `test_qwen35_paged_forward` 5/5 13/13 and `test_qwen27_paged_forward` 31/31 770/770 GREEN: the CPU tier pinned the pure parser's truth table and nothing pinned that the production resolver reads it. On the same unmutated binary `VT_GDN_OUT_BF16=0` FAILED 4/5 cases, 11/13 assertions, because the new case asserted bf16 unconditionally and the resolver's function-local static caches its getenv, so the case cannot neutralise the variable in-process. That variable is the denominator of this row's whole same-binary A/B, so the first defect would have invalidated the measurement that decides the row and the second hands an operator following the documented rollback a red that is not a regression. Both halves are one repair. `GdnOutDType` gets a `detail::` declaration beside its parser, so a gate can call the thing the model calls; the model case and a new resolver case assert against `VT_GDN_OUT_BF16` as the test file reads it DIRECTLY, never through the parser under test, because re-using either would make exactly the mutation being hunted self-consistent; and `tests/CMakeLists.txt` registers the same binary a second time with the lever forced to 0. That is the shape this tree already uses for a read-once lever (`_glue_fuse_off`, `test_dense_gateup_fused_marlin_off_*`), not a new pattern. The rollback arm now INVERTS instead of failing, and it is the arm in which a hardwired BF16 reads BF16 where F32 was ordered. Two in-tree statements that Edit 1 falsified survived the first pass and are repaired for the same reason the other three were: `outdt == BF16` is unconditional now, so the default-OFF `VT_GDN_FP8_IN_BF16` toggle is the only remaining bound on that arm, not the model shape. `test_qwen27_paged_forward:534-535` sat verbatim above the case that pins `GdnFp8MergedMixedQkvDType`'s three terms, and `cuda_gdn.cu:1975` still called the 35B the f32 arm. `cuda_gdn.cu` cannot be compiled here (no nvcc), so that edit is comment-only and every touched line was verified to begin with `//`. The dtype instrument's contract overstated its coverage three ways and the contract is narrowed rather than the recording extended, because a second call site no test enters is the shape reachability.md warns about. `GdnBlockPagedMixedSpec` is a paged GDN layer at the same `outdt` and records nothing, so a mixed step leaves `observed` false or stale, which is the ambiguity `observed` was added to remove. It resembles `GdnFp8InProjDebugStats` in shape only: that sibling is default-off behind an enable flag, this one stores unconditionally on every paged GDN layer of every forward. And it records at CUDA-graph CAPTURE, not replay. `ShouldUsePackedGdnDecode`'s comment implied the `dense_model` removal became observable in production. It did not, in either order: `has_packed_ba` still gates it and `in_proj_ba` has exactly one writer, the dense loader at qwen3_5_dense_weights.cpp:431 (the comment said 436). The removal reaches packed decode on NO checkpoint; it removes a contradiction with both references. The row had no lifecycle surface at all, which came from the spec pull request rather than the implementation: `GDN-MOE-BF16-OUT` existed only in `.agents/issue-index.md` and its own spec. It is recorded in `.agents/kernel-matrix.md` beside `KERNEL-GDN-AOT-BF16` at `GATING`, NOT `DONE`, since every GPU gate is owed, and that sibling's "Every 35B path stays f32" is corrected in the same edit because Edit 1 is what makes it false. `docs/STATUS.md` and `docs/BENCHMARKS.md` follow, as `scripts/check-doc-checkpoint.py` requires of a lifecycle claim; both say no number, none claimed, every axis owed to a GPU host. The spec's Risks item "first bf16 exposure of the fp8-fused gated norm" was assigned to Test 3 and Test 3 cannot reach it: `vt::RmsNormGatedQuantFp8` is guarded by `!w.out_proj_fp8.Empty() && supports_fp8()`, the synthetic CPU model has no fp8 `out_proj`, and the CPU platform reports no fp8. It is now recorded under `## Now` and `## Owed` as owed to the 35B GPU gate instead of standing as if satisfied. Red first for both halves of the lever repair. Verbatim before the repair: the severing mutation green in both suites, and `VT_GDN_OUT_BF16=0` failing at :520 and :521 with `CHECK( 0 == 2 )`. After it, the same mutation is RED in the `=0` arm at 3 assertions across 2 cases (`CHECK( 2 == 0 )`, `lever := false`), and the tree is 6/6 15/15 in BOTH arms, `test_qwen27_paged_forward` 31/31 770/770, `test_ops_gdn` 62/1825, `test_qwen27_dense_forward` 9/583, `test_qwen3_5_gdn_spec_routing` 6/52, `test_model_registry` 24/924, and `ctest -R '^test_qwen35_paged_forward'` 2/2. `scripts/agent-preflight.sh --staged` exits 0 with the two trailer gates SKIPPED because this branch is behind `origin/main`. Nothing here is measured and no GPU gate has run; the host has no CUDA toolchain and no GPU. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5-1m [claude-code]
…ranch that #1168 removed, and one of them named a reachable pair as impossible (#1168) The behavioural repair is closed; these are the documentary findings from the scoped review of `fcb3cb9`. Each one is a sentence a future implementer would read before narrowing something, and each was false about this tree. `cuda_gdn.cu` told the reader that the gated-RMSNorm `out` "is bf16 under GlueFuse and otherwise follows the input". It does not follow the input. All three non-GlueFuse call sites allocate `DBuf dgated(d, DType::kF32, ...)` whatever the input dtype (`qwen3_5.cpp:3965`, `:4435`, `:4886`), so this row's own `VT_GDN_OUT_BF16=1` default plus the documented, ctest-gated `VT_GLUE_FUSE=0` rollback makes bf16-in/f32-out reachable -- exactly the pair the sentence said could not occur. `TryLaunchRmsNormGatedFast` already tests `out.dtype` independently of `x.dtype` and dispatches all four `(Tin,Tout)` instantiations, so nothing is functionally wrong; the comment is what would justify narrowing that dispatch, which is why it has to be right. The previous wording was wrong in a different way ("35B ... else f32 out"), so the last edit traded one inaccuracy for another. `qwen3_5.cpp:124-125` carried a sixth surviving copy of the falsified claim: `outdt` "confines the narrowing to the dense 27B". `GdnOutDType()` no longer takes a shape argument, so `outdt` is BF16 on both arms at the default and confines nothing; the default-OFF `VT_GDN_FP8_IN_BF16` toggle is the only remaining bound. That definition now agrees with its own declaration comment in `qwen3_5_internal.h`, which was corrected already. The instrument contract headed "THREE LIMITS" reads as exhaustive and is not: the non-paged `GdnBlock` allocates at the same `outdt` and records nothing either. "Paged" excludes it technically, and a list a reader takes for complete has to say so, so it is limit four. `kernel-matrix.md` pointed both `KERNEL-GDN-PACKED-DECODE` and `KERNEL-GDN-AOT-BF16` at `qwen3_5.cpp#L1555`, which is today `MatmulFp8CutlassD`, an unrelated fp8 GEMM wrapper; the AOT row's label repeated the falsified 27B-only claim on top of that. Both anchors and the label now name something that exists. The spec understated the fp8-fused gated-norm guard as two terms when it has four (`GdnOutFp8FuseEnabled` and `GlueFuseEnabled` are also required), and its `## Owed` anchor landed on a comment line nine lines above the guard. The conclusion is unaffected: Test 3 still cannot reach that call on the CPU tier. Comment- and record-only. Every added and removed line in the three source files begins with `//`, verified mechanically, because this host has no `nvcc` and `cuda_gdn.cu` cannot be compiled here. Focused ctest all green and unchanged: `test_qwen35_paged_forward` 15/15 and its `VT_GDN_OUT_BF16=0` arm 15/15, `test_qwen27_paged_forward` 770/770, `test_ops_gdn` 1825/1825, `test_qwen27_dense_forward` 583/583, `test_qwen3_5_gdn_spec_routing` 52/52, `test_model_registry` 924/924. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code]
Taken so the branch gates stop skipping and so the focused suites rebuild against main's vt/ops.h change before the operator reruns them. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: Claude:Opus-5 [Claude Code]
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The GDN recurrence output
dcoreand thezgate were held in f32 on every MoEcheckpoint while both vLLM and SGLang keep them at the bf16 model dtype. That is
the dtype-polarity case the protocol names: a token gate cannot see it, because
the tokens match and the goldens pass while the path moves twice the bytes on the
largest per-layer GDN activation, twice over -- written by the recurrence, read
by the gated RMSNorm.
GdnOutDType()now takes no model-shape argument, so bf16 is the default on botharms and
VT_GDN_OUT_BF16=0is the rollback. The redundantdense_modelterm isgone from the packed-decode eligibility, which neither reference engine gates on
model shape either.
Removing that term does NOT make packed decode reachable on MoE, and the change
says so rather than implying otherwise.
has_packed_bastill gates it, andin_proj_bahas exactly one writer, the dense loader. That is #1169, filed andowed, not fixed here.
Two review rounds, both adversarial and both productive. The first found the
advertised lever simultaneously unpinned and self-reddening: severing
GdnOutDType()from its env parser left every suite green, and running thedocumented
VT_GDN_OUT_BF16=0rollback red the focused gate, so the rollbacklooked like a regression. The repair moved the resolver into
detail::, gave thetest its own independent env reader so a drift in the product parser reds instead
of silently agreeing, and registered a second ctest arm under the opposite
environment. The second round found the first repair had exchanged one inaccurate
comment for another, and a sixth surviving copy of the model-shape claim this row
falsifies.
Both rounds are mutation-proven. Severing the resolver reds the f32 arm; forcing
f32 reds the default arm; inverting the parser reds both arms plus the
independence assertion; deleting the production call site reds both arms on
reachability. The two ctest registrations run the same binary and return opposite
verdicts, which is what proves the environment property actually arrives.
Every GPU gate is OWED, not passed, and the row lands at
GATINGrather thanDONE. There is nonvccand no GPU on the build host, so nothing here ismeasured: the 27B 235/235 dense inertness control, the 35B correctness gate, the
VT_GDN_OUT_BF160-versus-1 A/B per leaf, and thensysmemory-formatconfirmation are all recorded
PENDINGon a GPU host.docs/BENCHMARKS.mdsays"No number, none claimed". The one control that would prove the dense arm
byte-identical,
test_qwen27_paged_engine, was run and SKIPPED withassertions: 0-- a skip wearing a pass -- and no result rests on it.Refs #1168, #1169, #1170, #521.
FOLLOWING_AGENTS_PROTOCOL
Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: Claude:Opus-5 [Claude Code]