refactor(VT-FP8-SHARED-SEAM): the FP8 W8A8 linear path becomes a seam a second model can reach (#940) - #972
Closed
localai-bot wants to merge 3 commits into
Closed
refactor(VT-FP8-SHARED-SEAM): the FP8 W8A8 linear path becomes a seam a second model can reach (#940)#972localai-bot wants to merge 3 commits into
localai-bot wants to merge 3 commits into
Conversation
… a second model can reach (#940) `ResidentFp8`, `DenseCublasLtFp8Enabled`, `MatmulFp8CutlassD` and `MatmulFp8CutlassPreQuantD` lived in the anonymous namespace of qwen3_5.cpp (`:1458`, `:1478`, `:1495`, `:1517` @ c7cb59f). A second model could reach them only by re-typing them, which is the hand-rolled parallel path AGENTS.md "Shared seams" forbids. NVFP4 already had both halves (`dense_nvfp4_gemm.h` + `compressed_tensors/schemes/nvfp4.h`); FP8 had neither. Moved verbatim to `include/vllm/model_executor/models/dense_fp8_gemm.h`, with the policy layer at `include/vllm/model_executor/layers/quantization/fp8.h` (`Fp8W8A8LinearMethod` + `MakeLinearMethod`, mirroring the NVFP4 pair). The CUDA-only guard travelled with the code it guards; no numerics, tolerance, guard or dispatch condition changed. The seam is templated on Dev/DBuf because qwen3_5.cpp keeps its own anonymous-namespace copies of those types (the KNOWN DUPLICATION dense_nvfp4_gemm.h:45-50 records). A non-template header could only have been COPIED into that file, and a seam that sits dead beside the production path makes "byte-identical" vacuously true. One definition, two instantiations instead: qwen3_5.cpp with its types, vllm::layers with the shared ones. Gate is Qwen3.5 byte-identity. Pre-existing suites unchanged at c7cb59f -> HEAD: test_qwen3_5_gdn_spec_routing 6/52, test_ops_fp8_cpu 4/56, test_qwen27_paged_forward 29/765, test_qwen27_dense_forward 9/583. test_linear_method 6/76 -> 8/88 (two new cases, declared additive). Full CPU gate: clean Ninja build 0 warnings under -Werror, ctest 485/485 passed. Found and deliberately NOT fixed, recorded in the spec: ResidentFp8 (and ResidentFp8Qkv/Qkvz) upload weight bytes without `load_stats::AddDeviceUpload` and without `AdoptDeviceBytesAsHost`, which every other resident-weight helper in the same file performs (`ResidentWeight` :1009,1016; `ResidentNvfp4` :1106,1111,1116,1121). Repairing it inside an extraction is exactly the behaviour change a byte-identity gate cannot see. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code]
Merged at a PINNED SHA, not the moving ref: origin/main advances under a shared checkout, and #841's preflight skips the trailer gate unless origin/main is an ancestor of HEAD, so the merge has to happen before the trailer result means anything. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code]
…new seam (#940) Anchors are load-bearing under the ground-every-impl rule, so a wrong one is a defect even in a comment. * `kFp8StaticTensorSym` is defined at `quantization/utils/quant_utils.py:124` and handed to `init_fp8_linear_kernel` at `modelopt.py:511-512`; the header cited `input_quant_fp8.py`, which does not contain it (verified by grep over the pinned tree at `555967922`). * The dequant-at-load fallback the factory comment points at ends at `qwen3_5_weights.cpp:405`, not `:414`. * The factory is `MakeLinearMethod`; the preamble called it `MakeFp8LinearMethod`. Comments only. `test_linear_method` re-run after: 8 cases / 88 assertions, SUCCESS!, unchanged. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code]
Collaborator
Author
|
Superseded by the branch Commit |
localai-bot
added a commit
that referenced
this pull request
Aug 16, 2026
… a second model can reach (#940) (#973) Closes #940. Spec: [`.agents/specs/vt-fp8-shared-seam.md`](https://github.com/mudler/vllm.cpp/blob/row/VT-FP8-SHARED-SEAM/.agents/specs/vt-fp8-shared-seam.md). Owning row: `MODEL-TEXT-nemotron-h-nemotron-hfor-causal-lm` (#517), which is what forces it. ## What was wrong AGENTS.md §"Shared seams": *"If a shared seam cannot represent the upstream behavior, extend it or record one exact tracked exception. Never hand-roll a parallel path."* NVFP4 honoured that. FP8 W8A8 did not: `Fp8Weight` was in a header, but everything that made it usable sat in the anonymous namespace of one `.cpp`. | Entry point | Was (`qwen3_5.cpp @ c7cb59f`) | Is | |---|---|---| | `ResidentFp8` | `:1458` | `models/dense_fp8_gemm.h` | | `DenseCublasLtFp8Enabled` | `:1478` | `models/dense_fp8_gemm.h` | | `MatmulFp8CutlassD` | `:1495` (guard `:1497-1498`) | `models/dense_fp8_gemm.h` | | `MatmulFp8CutlassPreQuantD` | `:1517` (guard `:1519-1520`) | `models/dense_fp8_gemm.h` | | policy layer | did not exist | `layers/quantization/fp8.h` (`Fp8W8A8LinearMethod`, `MakeLinearMethod`) | A second model had three options and the policy forbids two. #517 is that second model: 46 FP8 W8A8 mamba projections, 36.6% of decode bytes and 27.6% of GEMM FLOPs, and its `in_proj` produces the fused `zxbcdt` the conv and the SSD scan consume, so the block cannot be split and has no device path at all without this. ## Correspondence with `dense_nvfp4_gemm.h` | `dense_nvfp4_gemm.h` | `dense_fp8_gemm.h` | |---|---| | preamble: upstream chain, scope, known duplication | same shape, chain cites `compressed_tensors_w8a8_fp8.py:60,201-207`, `modelopt.py:444,531-537`, `fp8.py:267,446` | | `MarlinW4A16Enabled()` lever | `DenseCublasLtFp8Enabled()` lever (same spelling, same default) | | `ResidentNvfp4` `:294` | `ResidentFp8` | | `MatmulNvfp4MarlinD` `:506` | `MatmulFp8CutlassD` | | `MatmulNvfp4W4A16D` `:730` dispatcher | `MatmulFp8CutlassPreQuantD` (the `QuantizedActivation` overload) | | `schemes/nvfp4.h:35` `Nvfp4W4A16LinearMethod` | `quantization/fp8.h` `Fp8W8A8LinearMethod` | | `schemes/nvfp4.h:97` `MakeLinearMethod` | `quantization/fp8.h` `MakeLinearMethod` (overloaded on the weight type) | Two deliberate divergences, both argued in the spec: 1. **Path is `quantization/fp8.h`, not `quantization/schemes/fp8.h`.** #940 writes "schemes/nvfp4.h" for a file that lives at `compressed_tensors/schemes/nvfp4.h`, so it is shorthand; inventing a `quantization/schemes/` directory mirrors nothing upstream. `fp8.py` sits directly under `quantization/` in vLLM, and `layers/quantization/modelopt_mixed_precision.h` is the local precedent. 2. **The seam is templated on `Dev`/`DBuf`.** `qwen3_5.cpp` keeps its own anonymous-namespace copies of those types (the KNOWN DUPLICATION `dense_nvfp4_gemm.h:45-50` records). A non-template header could only have been *copied* into that file, which is the seam-dead-beside-the-path failure #940 exists to prevent. One definition, two instantiations instead. ## The call-site change `qwen3_5.cpp` loses 69 lines and gains a `using` plus two one-line adapters. Its ~14 call sites are unchanged text. ```c++ using dense_fp8::DenseCublasLtFp8Enabled; DBuf MatmulFp8CutlassD(Dev d, const Tensor& x, const Fp8Weight& w, DType out_dtype) { return dense_fp8::MatmulFp8CutlassD<DBuf>(d, x, w, out_dtype); } DBuf MatmulFp8CutlassPreQuantD(Dev d, const Tensor& a_fp8, const Fp8Weight& w, DType out_dtype) { return dense_fp8::MatmulFp8CutlassPreQuantD<DBuf>(d, a_fp8, w, out_dtype); } ``` No numerics, tolerance, guard or dispatch condition changed. The CUDA-only `VT_CHECK` travelled with the code it guards. ## Gate: Qwen3.5 byte-identity Baseline measured in a clean detached worktree at `c7cb59fbb`; after at `a0693813a`. Both on the CPU-only dev box, so `test_linear_method` reads without its two `VT_MARLIN_NVFP4` cases — the dgx shape is 10 / 97 and appears in the mutation table below. Identical counts for every pre-existing suite. | Suite | Before | After | |---|---|---| | `test_qwen3_5_gdn_spec_routing` | `test cases: 6 \| 6 passed`, `assertions: 52 \| 52 passed`, `SUCCESS!` | 6 / 52, `SUCCESS!` | | `test_ops_fp8_cpu` | 4 / 56, `SUCCESS!` | 4 / 56, `SUCCESS!` | | `test_qwen27_paged_forward` | 29 / 765, `SUCCESS!` | 29 / 765, `SUCCESS!` | | `test_qwen27_dense_forward` | 9 / 583, `SUCCESS!` | 9 / 583, `SUCCESS!` | | `test_linear_method` (CPU box) | 6 / 76, `SUCCESS!` | **8 / 88**, `SUCCESS!` (two new cases, declared additive) | Full CPU gate, run THREE times — once on the extraction commit and again after each pinned merge of `origin/main`, because a merge that brings in new source is a new binary and the earlier green does not cover it: | At | build | `ctest -j 4` | time | |---|---|---|---| | `a0693813a` (extraction) | exit 0, **0 warnings** under `-Werror` | **485/485 passed, 0 failed** | 828.96 s | | `018c9d1cb` (post-merge, `origin/main` @ `e5351776c`) | exit 0, **0 warnings** under `-Werror` | **485/485 passed, 0 failed** | 682.90 s | | `66c1e805c` (post-merge, `origin/main` @ `c90e3fc02`) | exit 0, **0 warnings** under `-Werror` | 487/488 passed, **1 failed** | 837.59 s | All three skip the same 2 absent-fixture tests (`test_modelopt_mixed_precision_checkpoint`, `test_voxtral_e2e`); the third run's 488 is 485 plus the 3 tests `c90e3fc02` brought in. **That one red is #294, and it was re-run before being called that.** `.agents/verification.md`: *"tests that starve under `ctest -j` are re-run serially before being called a regression."* #294 records `test_async_llm` reusing an aborted request id as racing the core abort at a **26% failure rate under contention, on main**. It PASSED in both earlier full runs of this same tree (0.14 s, 0.37 s) and failed in 0.31 s only in the third, which ran with the box at load average 29-35 from concurrent sessions. Re-run serially: **3/3 green, 15/15 assertions each**. Disk on the build host: 91% before, 99% at peak, 93% after, build tree removed afterwards. ## Mutation table: the seam is LIVE, not dead beside the path Every row rebuilt from source, `compile_exit` and `error:` count reported, binary sha distinct from baseline, case count non-zero. No `TEST_CASE` name contains a comma. **Every arm below ran on `dgx.casa`**, GB10 sm_121a, CUDA 13.0.88, image `vllmcpp-build:gb10`, `-j 4`, `$HOME/gpu.lock` held with `flock -n` for every test run. Both configure logs printed `CUDA feature cutlass-fp8: ENABLED for [121a]`, `cutlass-nvfp4: ENABLED for [121a]`, `fa2: ENABLED for [121a]`, `CUTLASS found at /cutlass` and `Triton AOT: ... sm_121a` — so no arm is voided. The seam header was restored byte-for-byte after every arm and verified against a pristine `sha256`, and each series' BASE re-run reproduced its own first binary sha exactly. CPU arms are `test_linear_method`; GPU arms are `test_qwen3_5_gdn_spec_routing`. **Each kind carries its own BASE control measured in the same tree**, because a mutation row on a box with pre-existing failures cannot be read without one. Two trees, stated per row. **T1 = `a0693813a`** (the extraction) carries CONTROL, BASE, M3, M4, M5, M7, M8. **T2 = `32980afda`** (this branch's reviewed head, transferred with `git archive`, clean configure + full build, `build rc=0`, 0 `error:`, 0 `warning:`) carries GDN_BASE, LM_BASE, M1, M2 and M9. Binary shas are comparable WITHIN a tree, never across the two. | Arm | Tree | Perturbation | `compile_exit` | `error:` | binary sha16 | `[doctest] test cases:` | `[doctest] assertions:` | `Status:` / exit | |---|---|---|---|---|---|---|---|---| | **CONTROL** (GPU) | T1 | none, and `qwen3_5.cpp` replaced by its `c7cb59fbb` (pre-extraction) content in the SAME tree, SAME flags | 0 | 0 | `8b740f86eeb7da5d` | `12 \| 11 passed \| 1 failed` | `123 \| 119 passed \| 4 failed` | `FAILURE!` / 1 | | **BASE** (GPU) | T1 | none | 0 | 0 | `090bc6e47a478cb2` | `12 \| 11 passed \| 1 failed` | `123 \| 119 passed \| 4 failed` | `FAILURE!` / 1 | | M3 (GPU) | T1 | drop the `input_scale`: `QuantFp8Static(..., w.input_scale)` → `..., 1.0F` | 0 | 0 | `91cfeeb337fec5a3` | `12 \| 11 passed \| 1 failed` | `123 \| 107 passed \| 16 failed` | `FAILURE!` / 1 | | M4 (GPU) | T1 | change the alpha fold: `w.alpha` → `w.alpha * 2.0F` in `MatmulFp8CutlassD` | 0 | 0 | `6a7fb2f31de34576` | `12 \| 11 passed \| 1 failed` | `123 \| 107 passed \| 16 failed` | `FAILURE!` / 1 | | M5 (GPU) | T1 | same alpha fold in `MatmulFp8CutlassPreQuantD` | 0 | 0 | `ecebc93903e7d801` | `12 \| 11 passed \| 1 failed` | `123 \| 119 passed \| 4 failed` | **UNCHANGED vs BASE — negative result** | | M7 (GPU) | T1 | `VT_CHECK(false, ...)` as the first statement of `MatmulFp8CutlassPreQuantD` | 0 | 0 | `84fd9f9d2d7386f7` | `12 \| 9 passed \| 3 failed` | `103 \| 99 passed \| 4 failed` | `FAILURE!` / 1 | | M8 (GPU) | T1 | `VT_CHECK(false, ...)` as the first statement of `MatmulFp8CutlassD` | 0 | 0 | `dc8bcfc1e97fafac` | `12 \| 11 passed \| 1 failed` | `91 \| 91 passed \| 0 failed` | `FAILURE!` / 1 | | **GDN_BASE** (GPU) | T2 | none | 0 | 0 | `47a9960ac92d4b66` | `12 \| 11 passed \| 1 failed` | `123 \| 119 passed \| 4 failed` | `FAILURE!` / 1 | | **LM_BASE** (CPU) | T2 | none | 0 | 0 | `f8f5b2d3a0116980` | `10 \| 9 passed \| 1 failed` | `97 \| 95 passed \| 2 failed` | `FAILURE!` / 1 | | M1 (CPU) | T2 | delete the CUDA guard in `MatmulFp8CutlassD` | 0 | 0 | `2a3a1a8e5df413fd` | `10 \| 8 passed \| 2 failed` | `97 \| 94 passed \| 3 failed` | `FAILURE!` / 1 | | M2 (CPU) | T2 | delete the CUDA guard in `MatmulFp8CutlassPreQuantD` | 0 | 0 | `15d702dd127b1c35` | `10 \| 8 passed \| 2 failed` | `97 \| 94 passed \| 3 failed` | `FAILURE!` / 1 | | M9 (GPU) | T2 | zero the alpha fold: `w.alpha` → `0.0F` in `MatmulFp8CutlassPreQuantD` (both GEMM arms; anchor count asserted `== 2`) | 0 | 0 | `6c9335e8b6a53228` | `12 \| 9 passed \| 3 failed` | `123 \| 115 passed \| 8 failed` | `FAILURE!` / 1 | | **GDN_BASE re-run** (GPU) | T2 | none, after every T2 mutation was reverted | 0 | 0 | `47a9960ac92d4b66` (identical to GDN_BASE) | `12 \| 11 passed \| 1 failed` | `123 \| 119 passed \| 4 failed` | `FAILURE!` / 1 | **Reading it.** *The BASE red is inherited, not introduced.* `test_qwen3_5_gdn_spec_routing` reads 119/123 on GB10 at `main`, which is exactly what #907 already records for this box. The CONTROL row proves it in the same tree rather than by citation: pre-extraction `qwen3_5.cpp`, same build directory, same flags, distinct binary, and the result is identical down to the individual mismatch counts (`30504`, `48756`, `30504`, `48756`) and the same four `H=5120 / T=3` combinations. **That equality is the byte-identity evidence** — the extraction reproduces the production numerics including a pre-existing defect. *The seam is on the live Qwen3.5 path.* M3 and M4 each take the failures from 4 to **16** (every combination in the case), from a source change inside `dense_fp8_gemm.h`, with a clean compile and a distinct binary. The split arm of `ProjectGdnFp8QkvzForTest` (`qwen3_5.cpp:6693-6694`) calls the extracted `MatmulFp8CutlassD` while the merged arm does not, so perturbing the seam breaks their bitwise equality. A seam sitting dead beside the model cannot do that. *M5 is a negative result and is reported as one.* Doubling alpha in the pre-quantized arm changed nothing. M7 shows the arm is nevertheless REACHED: forcing that function to throw turns **two additional cases** red — `GDN gate POLARITY on the FP8 tail: GdnBlockPaged (CUDA)` and `... the MIXED spec batch (CUDA)`. *M9 is why nothing SAW the doubling, and it corrects the first reading of M5.* The mechanism is **scale invariance of each predicate, not cancellation between the two arms** — those two cases compare their arms against nothing at all. Each arm has its own predicate: `CHECK(silu_nonzero == 0)` (`tests/vllm/models/test_qwen3_5_gdn_spec_routing.cpp:895`), invariant under ANY non-zero rescale, and `CHECK(max_sigmoid > 0.0)` (`:904`), invariant under any POSITIVE one. `w.alpha * 2.0F` is positive and non-zero, so M5 could not have moved either. M9 sets that same alpha to `0.0F` — a factor COMMON to both arms, which the cancellation reading predicts would stay green — and it turns those two cases RED. The failure detail is what settles it: the four NEW assertion failures are all `:904 CHECK( max_sigmoid > 0.0 )` with `values: CHECK( 0 > 0 )` and `max_sigmoid := 0` (two dims each in both fp8 polarity cases), while `CHECK(silu_nonzero == 0)` at `:895` stays green with `silu_nonzero := 0`. The pre-existing four at `:525` (`30504`, `48756`, `30504`, `48756`) are untouched, which is how `123 | 119 passed | 4 failed` becomes `123 | 115 passed | 8 failed`. What the suite therefore owes is a gate sensitive to any POSITIVE rescale of that arm — see §Owed. *M8 is also the reason to read cases and not just assertions.* It prints `assertions: 91 | 91 passed | 0 failed` — and `Status: FAILURE!` with exit 1, because the throw aborted the case before its `CHECK`s ran. An assertion-only reading of that line would have called a fully-blocked GEMM a pass. *M1 and M2, corrected.* As first recorded these two rows were attributed to `dgx.casa` and read `8 | 7 passed | 1 failed` / `88 | 87 passed | 1 failed`. That is the CPU-ONLY box's shape, not this host's: `test_linear_method` puts 2 cases and 9 assertions behind `VT_MARLIN_NVFP4` (`tests/vllm/model_executor/layers/test_linear_method.cpp:105-272`), so a CUDA build here reads 10 cases / 97 assertions, and the 2 pre-existing failures sit inside one of those Marlin cases — `linear_method: MXFP4 fused gate_up ~= split`, both at `:247 CHECK( after == before + 1 )` with `values: CHECK( 0 == 1 )`. Those two are #907's `test_linear_method 83 of 85` plus this row's 12 additive assertions, all passing (85 + 12 = 97, 83 + 12 = 95). The rows above are RE-RUNS on `dgx.casa` against the `LM_BASE` control measured in the same tree, and the conclusion is unchanged: each guard deletion turns exactly ONE further case red, `linear_method: the fp8 w8a8 method reaches the shared seam in both arms`, at the `CHECK_THROWS_WITH_AS` that pins that guard's message (`:540` for M1, `:550` for M2) — the deleted guard lets the call reach `vt::MatmulFp8CublasLt`, which throws a different message. *M6 is unused, and no arm is missing.* The labels run M1-M5 and M7-M9. Nothing was measured under an M6 label and nothing was withheld: the number was skipped when the GPU arms were planned, and the gap went unexplained until this round. It stays unused rather than renumbered, because `73d67f9d2` and this row's fresh review both cite M7 and M8 by those labels, and renumbering would silently break every citation. M9 continues the sequence. ## Found and deliberately NOT fixed `ResidentFp8` (and its siblings `ResidentFp8Qkv` / `ResidentFp8Qkvz`) `Alloc` + `Copy` the fp8 weight bytes to the device **without** `vllm::load_stats::AddDeviceUpload` and **without** the post-upload `AdoptDeviceBytesAsHost`. Every other resident-weight helper in the same file does both: `ResidentWeight` (`qwen3_5.cpp:1008,1015 @ c7cb59f`) and `ResidentNvfp4` (`:1105,1110,1115,1120`), and `dense_nvfp4_gemm.h:294-328` carries the comment explaining why the pair is mandatory (ENG-LOAD-DIRECT-UPLOAD, #150). So the 35B fp8 tower's upload is missing from load accounting and its pages are never re-tagged, which is the shape of the GB10 weight-residency ATS penalty. The accounting half is BOUNDED and #974 says so: `load_stats` has exactly one non-test consumer, `PrintLoadBytes` (`src/vllm/entrypoints/model_loader.cpp:133`), a stderr diagnostic behind `LoadStatsEnabled()`, so the under-report costs a diagnostic and not a decision. The residency half carries no such bound. Carried across unchanged: repairing it inside an extraction is exactly the behaviour change a byte-identity gate cannot see. Recorded in the spec under §Found, not fixed; filed as **#974**, which also names extending `scripts/check-fp4-resident-consistency.py` — today the same invariant for `ResidentNvfp4` alone — as the gate the fix should carry. Separately, **not a new find**: the CUDA-only refusal keys on `kMatmulFp8CublasLt` while the ops it would run (`kQuantFp8Static`, `kMatmulFp8Cutlass`) have CPU reference arms since #468/#842. That is already recorded in `vt-fp8-w8a8-cpu-arm.md` §Residual gap and pinned at `tests/vt/test_ops_fp8_cpu.cpp:445-453`. Carried unchanged; the new CPU case re-pins it at the model tier. ## Protocol deviation: the spec was not committed before the implementation AGENTS.md §"Spec before code" requires the spec to be committed first. `.agents/specs/vt-fp8-shared-seam.md` was added in `a0693813a`, the SAME commit as the extraction, and the superseded PR #972 had the same shape. Commit order is what proves the spec came first when one PR carries both, and here it proves the opposite. It is recorded rather than repaired because it cannot be repaired: neither `main` nor a branch under review is force-pushed, so one commit cannot be rewritten into two, and back-dating a spec commit that never existed would be worse than the ordering it hides. The spec now carries a `## Protocol deviation` section stating what the deviation cost — the scope, the gate and the two divergences were argued after the code existed, so none of them constrained the implementation — and what it did not cost: the mutation table was designed and run by sessions that did not write the code, and reproduced independently by the fresh review. This is visible debt, not success. A reviewer who does not accept the argument does not merge it. ## Not in scope Wiring NemotronH to the seam (A2-Q under #517), `ResidentFp8Qkv`/`Qkvz` and the merged-QKV(z) path, and any widening of the device predicate. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code] --------- Co-authored-by: Ettore Di Giacinto <mudler@localai.io>
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.
Closes #940.
Spec:
.agents/specs/vt-fp8-shared-seam.md.Owning row:
MODEL-TEXT-nemotron-h-nemotron-hfor-causal-lm(#517), which is what forces it.What was wrong
AGENTS.md §"Shared seams": "If a shared seam cannot represent the upstream behavior, extend it or record one exact tracked exception. Never hand-roll a parallel path."
NVFP4 honoured that. FP8 W8A8 did not:
Fp8Weightwas in a header, but everything that made it usable sat in the anonymous namespace of one.cpp.qwen3_5.cpp @ c7cb59fbb)ResidentFp8:1458models/dense_fp8_gemm.hDenseCublasLtFp8Enabled:1478models/dense_fp8_gemm.hMatmulFp8CutlassD:1495(guard:1497-1498)models/dense_fp8_gemm.hMatmulFp8CutlassPreQuantD:1517(guard:1519-1520)models/dense_fp8_gemm.hlayers/quantization/fp8.h(Fp8W8A8LinearMethod,MakeLinearMethod)A second model had three options and the policy forbids two. #517 is that second model: 46 FP8 W8A8 mamba projections, 36.6% of decode bytes and 27.6% of GEMM FLOPs, and its
in_projproduces the fusedzxbcdtthe conv and the SSD scan consume, so the block cannot be split and has no device path at all without this.Correspondence with
dense_nvfp4_gemm.hdense_nvfp4_gemm.hdense_fp8_gemm.hcompressed_tensors_w8a8_fp8.py:60,201-207,modelopt.py:444,531-537,fp8.py:267,446MarlinW4A16Enabled()leverDenseCublasLtFp8Enabled()lever (same spelling, same default)ResidentNvfp4:294ResidentFp8MatmulNvfp4MarlinD:506MatmulFp8CutlassDMatmulNvfp4W4A16D:730dispatcherMatmulFp8CutlassPreQuantD(theQuantizedActivationoverload)schemes/nvfp4.h:35Nvfp4W4A16LinearMethodquantization/fp8.hFp8W8A8LinearMethodschemes/nvfp4.h:97MakeLinearMethodquantization/fp8.hMakeLinearMethod(overloaded on the weight type)Two deliberate divergences, both argued in the spec:
quantization/fp8.h, notquantization/schemes/fp8.h. The FP8 W8A8 linear path is not a shared seam: residency and GEMM entry points live inside qwen3_5.cpp, so a second model cannot reach them without hand-rolling a parallel path #940 writes "schemes/nvfp4.h" for a file that lives atcompressed_tensors/schemes/nvfp4.h, so it is shorthand; inventing aquantization/schemes/directory mirrors nothing upstream.fp8.pysits directly underquantization/in vLLM, andlayers/quantization/modelopt_mixed_precision.his the local precedent.Dev/DBuf.qwen3_5.cppkeeps its own anonymous-namespace copies of those types (the KNOWN DUPLICATIONdense_nvfp4_gemm.h:45-50records). A non-template header could only have been copied into that file, which is the seam-dead-beside-the-path failure The FP8 W8A8 linear path is not a shared seam: residency and GEMM entry points live inside qwen3_5.cpp, so a second model cannot reach them without hand-rolling a parallel path #940 exists to prevent. One definition, two instantiations instead.The call-site change
qwen3_5.cpploses 69 lines and gains ausingplus two one-line adapters. Its ~14 call sites are unchanged text.No numerics, tolerance, guard or dispatch condition changed. The CUDA-only
VT_CHECKtravelled with the code it guards.Gate: Qwen3.5 byte-identity
Baseline measured in a clean detached worktree at
c7cb59fbb; after ata0693813a. Identical counts for every pre-existing suite.test_qwen3_5_gdn_spec_routingtest cases: 6 | 6 passed,assertions: 52 | 52 passed,SUCCESS!SUCCESS!test_ops_fp8_cpuSUCCESS!SUCCESS!test_qwen27_paged_forwardSUCCESS!SUCCESS!test_qwen27_dense_forwardSUCCESS!SUCCESS!test_linear_methodSUCCESS!SUCCESS!(two new cases, declared additive)Full CPU gate at
a0693813a: clean Ninja configure +cmake --build build -j 12, exit 0, 0 warnings under-Werror;ctest -j 4485/485 passed, 0 failed in 828.96 s (2 skipped:test_modelopt_mixed_precision_checkpoint,test_voxtral_e2e, both absent-fixture skips). Disk 91% before, 99% at peak, 91% after teardown.Mutation table: the seam is LIVE, not dead beside the path
Every row rebuilt from source,
compile_exitanderror:count reported, binary sha distinct from baseline, case count non-zero. NoTEST_CASEname contains a comma.GPU arm in progress on dgx (GB10 sm_121a,
cutlass-fp8: ENABLED); this section is filled before the PR leaves draft.Found and deliberately NOT fixed
ResidentFp8(and its siblingsResidentFp8Qkv/ResidentFp8Qkvz)Alloc+Copythe fp8 weight bytes to the device withoutvllm::load_stats::AddDeviceUploadand without the post-uploadAdoptDeviceBytesAsHost. Every other resident-weight helper in the same file does both:ResidentWeight(qwen3_5.cpp:1009,1016 @ c7cb59fbb) andResidentNvfp4(:1106,1111,1116,1121), anddense_nvfp4_gemm.h:294-328carries the comment explaining why the pair is mandatory (ENG-LOAD-DIRECT-UPLOAD, #150). So the 35B fp8 tower's upload is missing from load accounting and its pages are never re-tagged, which is the shape of the GB10 weight-residency ATS penalty. Carried across unchanged: repairing it inside an extraction is exactly the behaviour change a byte-identity gate cannot see. Recorded in the spec under §Found, not fixed; filed separately.Separately, not a new find: the CUDA-only refusal keys on
kMatmulFp8CublasLtwhile the ops it would run (kQuantFp8Static,kMatmulFp8Cutlass) have CPU reference arms since #468/#842. That is already recorded invt-fp8-w8a8-cpu-arm.md§Residual gap and pinned attests/vt/test_ops_fp8_cpu.cpp:445-453. Carried unchanged; the new CPU case re-pins it at the model tier.Not in scope
Wiring NemotronH to the seam (A2-Q under #517),
ResidentFp8Qkv/Qkvzand the merged-QKV(z) path, and any widening of the device predicate.FOLLOWING_AGENTS_PROTOCOL
Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:claude-opus-5 [claude-code]