feat: fmt=8 (fp8-e4m3) decode on the kv_b absorb path, CPU and CUDA - #1102
Open
monotophic wants to merge 6 commits into
Open
feat: fmt=8 (fp8-e4m3) decode on the kv_b absorb path, CPU and CUDA#1102monotophic wants to merge 6 commits into
monotophic wants to merge 6 commits into
Conversation
Both functions used to refuse fmt=8 with exit(1) (the SIGSEGV-prevention guard from the earlier fix round). This adds the real decode branch instead, reusing quant.h's e4m3_decode LUT and FP8_BLOCK/fp8_nblk per-128x128-block scale geometry -- the same plumbing matmul_fp8 already exercises on the dense/expert path (colibri.c:964). Mirrors matmul_fp8's block-scale indexing exactly: blkO=row/FP8_BLOCK selects the scale row, one f32 scale per FP8_BLOCK-wide slice of I. Every other unsupported fmt (6, and anything not explicitly listed) still refuses loudly by name, unchanged. Per FP8_CAPABILITY_MATRIX_2026-08-18.md and D-FP8-1 (colibri_lab/knowledge/decisions/2026-08-16-fp8-faithful-container.md): this closes the "absorb has no fp8 decoder" gap, the sole remaining hole in fmt=8 CPU support (every other family/path already dispatches fmt=8 through matmul_qt_ex/matmul_fp8). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replaces the two fmt=8 refusal checks (call_addrow_fmt8/call_matvec_fmt8 -- fmt=8 no longer refuses) with real correctness coverage: - Exact-dequant check: fmt=8 fixtures (a small single-block shape and a kv_b-shaped O=192,I=512 multi-block shape, I=512=kv_lora_rank per the 2026-08-16 fp8-faithful-container decision record) run through the existing check_addrow_identity/check_matvec_identity harness against a new, independently-coded fmt=8 branch in qt_dequant_row_ref (a flat per-element loop, deliberately not the block-batched shape the functions under test use). - Parity check: qt_matvec_rows' fmt=8 output against quant.h's matmul_fp8 -- the proven non-absorb fp8 reference -- on the same kv_b-shaped tensor, tolerance 1e-5 relative (both share the exact same double-accumulate- per-128-block order, so this is a tight guard against reassociation noise, not a loose bound). fmt=6 (no absorb decoder, still refuses) keeps its fork+pipe+waitpid refusal coverage unchanged. Built and run locally (plain make, no GPU/model involved): make tests/test_qt_addrow && ./tests/test_qt_addrow -> "qt_addrow/qt_matvec_rows tests: ok" (exit 0) Full CPU suite (make test-c, 62 binaries) also reruns clean after this change: 0 failures. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
weight_at gains an fmt==8 branch: decode the raw e4m3 byte through the shared c_e4m3 LUT (the same table quant_matmul's fmt==8 branch reads, published once by coli_cuda_fp8_set_lut), unscaled -- absorb_scale applies the per-128x128-block scale, same division of labor every other quantized fmt already uses through this pair of functions. absorb_scale gains an fmt==8 branch: blkO=row>>7, colBlk=k>>7, scale index blkO*ng+colBlk -- exactly quant_matmul's own fmt==8 indexing (scales[(o>>7)*ceil(I/128) + i>>7]) and matmul_fp8's CPU reference (quant.h). `ng` here is coli_cuda_tensor_upload's t->ng, already set to ceil(I/128) specifically for fmt=8 uploads (backend_cuda.cu, pre-existing); `gs` is unused for fmt=8 (always 0) since the block edge is the fixed FP8_BLOCK=128 constant, not a caller-supplied group size. absorb_fmt_ok widened from `fmt<=4` to `fmt<=4 || fmt==8`. All 7 absorb host wrappers funnel through this one predicate, so none needed a per-wrapper change -- confirms the capability matrix's minimal-change estimate. Per FP8_CAPABILITY_MATRIX_2026-08-18.md and D-FP8-1: closes the CUDA half of the "absorb has no fp8 decoder" gap. Before this, an fp8 kv_b_proj absorb call refused before any kernel launch (falls back to CPU, which until the companion CPU commit also refused) -- never silently wrong, per the matrix's headline finding, just unsupported. Adjacent finding (NOT fixed here, out of this branch's scope): coli_cuda_tensor_free's byte-accounting formula for fmt=8 (backend_cuda.cu:2315-2316, `(size_t)tensor->O * ng * sizeof(float)`) does not match coli_cuda_tensor_upload's own charge (backend_cuda.cu:1361-1363, `ceil(O/128) * ng`) -- free overcounts, the `tensor_bytes >= bytes` guard silently skips the subtraction, and the diagnostic VRAM counter (coli_cuda_stats, printed by colibri.c's cuda_stats_print) sticks non-zero after freeing ANY fmt=8 tensor. This predates this PR (coli_cuda_tensor_free untouched by it) and is orthogonal to absorb -- it would already reproduce today via the dense/expert fmt=8 path. Diagnostic-only: the counter feeds no real budget/eviction decision, and cudaFree itself still runs unconditionally, so no bytes are actually leaked on the device. See this PR's build report for the reproduction. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds test_fmt8_absorb(), called from main() alongside test_fmt6() with the same self-contained upload/free lifecycle. Publishes a synthetic-but- arithmetically-correct e4m3 LUT via coli_cuda_fp8_set_lut, uploads a kv_b-shaped fmt=8 weight (H=2,Q=40,V=40,R=2,K=140 -> O=160: nblkO=2 with a partial row-block tail, nblkI=2 with a partial column-block tail -- the exact geometry weight_at/absorb_scale's new branches must index correctly), runs it through the real coli_cuda_attention_absorb, and compares against a CPU reference built the same way the pre-existing fmt=0 absorb block in main() is (independent score/softmax/context accumulation; only the weight lookup changes to an e4m3 block-scale dequant, decoded arithmetically -- sign/exp/mant -- not via the engine's own LUT, so it cross-checks the uploaded table rather than assuming it). Tolerance 1e-3 relative. Adjacent finding, recorded not fixed (see the companion engine commit and this PR's build report): coli_cuda_tensor_free's byte accounting for fmt=8 predates this PR and drifts on every fmt=8 tensor free. This test's post-free stats check therefore asserts tensor_count==0 only (proves no tensor was actually leaked -- that decrement is unconditional) and not bytes==0 (which trips the pre-existing, orthogonal, diagnostic-only bug). Built and run on spark1 (idle, COLI_GPU=0, GB10/sm_121): make cuda-test -> MAKE_CUDA_TEST_EXIT=0 backend_cuda_test's final line: "cuda backend: q8/q4/q2/f32/e8 correctness ok on 1 device(s)" with no mismatch/leaked/rejected lines anywhere in the run (ragged_attention_test, fp8_warp_test, mxfp4_cuda_test also ran clean as part of the same cuda-test recipe). Full log: colibri_lab/reports/ (this PR's build report) references the copy kept at spark1:~/f8absorb_build/cuda_test_run.log. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rd_kvb The head-shard uploader admitted fmt=8 (fp8-e4m3-b128) kv_b tensors and only failed safe by accident: fmt=8 keeps its e4m3 bytes in q8, so the function selected a NULL q4 weight pointer with an int2 row stride and the upload's !weights check happened to reject it -- silent, and one refactor away from misreading the per-128x128-block scale array with per-row geometry. Allowlist the formats the stride/scale arithmetic is actually written for (1/2/3/4) and refuse anything else by name BEFORE any pointer/stride use, stating what serves fmt=8 instead (the absorb path on the layer home device; COLI_CUDA_ATTN_SHARD is a no-op for it). Notice only, bounded once per process per fmt: sharding is an opt-in optimization and skipping it is the correct behavior. tests/test_shard_kvb_refuse.c pins the refusal (fires for fmt=8 and fmt=6, no shard state minted, bounded, fmt=1 not refused); built against the pre-guard code its message assertions fail, proving the guard bites. CPU builds SKIP loudly (the function only exists under COLI_CUDA); no GPU is needed on CUDA builds -- every probed path returns before any device context exists.
…_addrow The fmt=8 identity cases never exercised nblkI>=2 WITH a partial column tail (the kv_b-shaped case uses I=512, exact column blocks), so the block-scale stride/bi-index math was only checked at shapes where flooring it is invisible. Add O=130 x I=200 (2-row row-block tail, 72-wide column-block tail); mutation-checked: nblkI=I/FP8_BLOCK (floor) passes every pre-existing fmt=8 case and fails exactly this one, through both qt_addrow and qt_matvec_rows. Also pin the fmt=8 NaN policy on the absorb path (quant.h: a 0x7F/0xFF byte decodes to IEEE NaN and propagates, never scrubbed): the identity checks exclude NaN bytes and would pass NaN lanes silently anyway (NaN > eps is false), so assert explicitly against the independent reference -- qt_addrow poisons exactly the lanes whose reference dequant is NaN (both byte codes, including the tail-block corner), qt_matvec_rows poisons the whole dot product of a NaN-bearing row, and clean rows/lanes of the same tensor stay NaN-free and tolerance-identical.
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
1 participant
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.
Authored by Fable 5 in Claude Code, analysis in partnership with @monotophic
Checkpoint-faithful FP8 containers stamp
kv_b_projas fmt=8 (raw e4m3bytes + one f32 scale per 128×128 block). The absorb decode path had no
fmt=8 branch: CPU
qt_addrow/qt_matvec_rowsrefused loudly, and theCUDA absorb gate refused before any kernel ran — an FP8 container could
load but not decode attention. This adds the fmt=8 branch to both
backends, mirroring
matmul_fp8's block-scale indexing exactly, plus anexplicit named skip (with the absorb path called out) where kv_b GPU
sharding legitimately cannot serve fmt=8.
Metal builds are covered too, without new Metal code: the existing
per-family format gate routes fmt=8 kv_b to this PR's CPU absorb branch
(with its named notice), so after this change all three backends serve
fmt=8 kv_b. Verified end-to-end with a Metal-enabled build decoding a
real FP8 container (correct output; the absorb branch's execution
directly observed via temporary instrumentation, since removed).
Behavioral contract
backends; every other fmt's behavior is unchanged.
by exactly {8}).
layer_cuda_shard_kvbrefuses un-shardable kv_b formats BY NAME(notice + skip; the absorb path serves them) instead of proceeding on a
NULL pointer by accident.
Scope note @JustVugg — observability deliberately left out. During verification
we used a temporary log line to prove the absorb branch executes on the
serve path (the evidence below), then removed it to keep this change
minimal. A permanent, env-gated absorb-path debug line is a reasonable
follow-on if you'd find it useful — happy to add it to this PR or a
follow-up at your request.
Capstone matrix
Fuller matrix and review record
Two-reviewer roster + fix round. Fix-round changes: (1) explicit allowlist refusal in layer_cuda_shard_kvb (was accidental-safe on NULL weights + wrong stride); notice+skip polarity follows the repo's existing per-family gate-notice convention — the absorb path is the correct server for fmt=8 kv_b, so killing the load would refuse a legitimate container; (2) partial-column-tail CPU case (I=200/O=130, nblkI≥2); (3) NaN propagation pinned for both NaN byte codes incl. the tail-block corner. Interaction disclosed: fmt=8 tensors' VRAM byte accounting is corrected by the separate accounting fix PR (posting order: that one first); without it the diagnostic counter over-reports (pre-existing there, not introduced here). NaN policy: identical CPU vs CUDA (auditor-verified); absorb NaN propagation now test-pinned.Durable vs current-state: decode branches and refusal are durable;
mutation-failure counts and GB10/sm_121 figures are current-state
(2026-08-18, base ad79236).