Skip to content

feat: fmt=8 (fp8-e4m3) decode on the kv_b absorb path, CPU and CUDA - #1102

Open
monotophic wants to merge 4 commits into
JustVugg:devfrom
monotophic:f8/absorb-fmt8
Open

monotophic wants to merge 4 commits into
JustVugg:devfrom
monotophic:f8/absorb-fmt8

Conversation

@monotophic

@monotophic monotophic commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Authored by Fable 5 in Claude Code (original, 2026-08-18) and re-derived onto current dev by Fable 5.1 with a Sonnet/Opus review roster in Claude Code (2026-09-03), analysis in partnership with @monotophic

Context. This PR is one of four independent contributions derived from a single locally-verified working tree (fp8 container support, scoring-evidence tooling, server API hardening, and batched group scoring). This one carries the fp8 container line: the fmt=8 absorb decode path, the mint tool's tests, and their end-to-end test. It stands alone — nothing else in the set needs to land for this to be complete, and merging or declining the others does not affect it. The others will be proposed separately, each with its own evidence.

Checkpoint-faithful FP8 containers stamp kv_b_proj as fmt=8 (raw e4m3 bytes + one f32 scale per 128×128 block). The absorb decode path had no fmt=8 branch: CPU qt_addrow/qt_matvec_rows refused loudly, and the CUDA 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 an explicit named skip (with the absorb path called out) where kv_b GPU sharding legitimately cannot serve fmt=8. The block geometry (FP8_BLOCK, fp8_nblk) moves to a shared header, fp8_format.h, so the CPU branch, the CUDA kernels, and the tests read one definition site.

The dense fmt=8 matmul path already exists on dev; this PR's own new content is the kv_b/absorb decode arms (CPU and CUDA), the shared block-geometry header, and their tests. Platform coverage at this head: CPU is native for both dense and absorb fmt=8. CUDA is native for both and is device-proven end-to-end on GB10: the two-slot batched-serve CUDA-absorb arm passed under CUDA_DENSE=1 + COLI_CUDA_ATTN=1, with the engine's own boot line [CUDA] mode: routed experts + resident dense tensors captured as a positive witness (kv_b CUDA-eligible, absorb path selected); the same test fails, on the same hardware, when that boot line reads resident dense on CPU. Metal has a real, unit-tested fmt=8 decode kernel that no live dispatch path calls, so every fmt=8 matmul on a Metal build continues to fall back to CPU by existing dispatch design — unchanged by this PR; Vulkan has no fmt=8 arm and continues to refuse it at upload, also unchanged. Two other engines in this tree (glm53.c, kimi_k3.c) share the identical kv_b_proj-absorb attention shape and are attachable to the same feature but are not wired to it — a separate follow-on, not a gap here.

Behavioral contract

  • fmt=8 kv_b decodes correctly against the quant.h reference on both backends: on CPU, qt_addrow bit-exact and qt_matvec_rows within float-accumulation-order tolerance; on CUDA, within 1e-3 relative on the GB10 oracle, with the absorb kernels' per-element scaling documented in backend_cuda.cu as the accepted RDNA2 (gfx1030) field report: AMD backend works; greedy decode not token-stable across mixed CPU/GPU expert tiers #510 divergence class. Every other fmt's behavior is unchanged.
  • Unsupported fmts still refuse loudly on both backends (admission widened by exactly {8}).
  • layer_cuda_shard_kvb refuses un-shardable kv_b formats BY NAME (notice + skip; the absorb path serves them) instead of proceeding on a NULL pointer by accident — and the same allowlist closes two pre-existing silent misreads (fmt=5 and fmt=6 computed a wrong row-byte stride with a non-NULL q4).
  • The CUDA fmt=8 LUT-ready flag is cleared on coli_cuda_init as well as shutdown, closing a pre-existing re-init hole (process-wide flag vs per-device table); a lifecycle test pins it.
  • Everything that worked before still works: the full gate set is green at this head, and the branch passes make check standing alone.

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, then removed it to keep this change minimal; the end-to-end test now captures the engine's own boot-mode line instead. 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 (evidence re-run at the pushed head 77d8939 unless marked)

claim decisive evidence
CPU decode matches the reference validator-written, tree-independent OCP-E4M3FN decoder, 6 disjoint shapes at block boundaries and tails: qt_addrow bit-exact; qt_matvec_rows ≈6e-8 relative (float accumulation order)
CUDA decode matches the reference within the documented tolerance. make cuda-test on GB10: the fmt=8 absorb-kernel oracle, the fmt trap test, and the LUT-lifecycle pin all pass (oa-lane/results/spark1/cells/cuda_test/)
CPU/CUDA indexing agree static host-side sizing probe, 324 shapes × 7 invariants + a 4,097-ordinal row_bytes sweep, with a negative control (wrong ng formula fails as expected)
old head could not serve fmt=8 old binary vs new test on both a CPU host and a CUDA host: BITE CONFIRMED: qt_addrow: unsupported fmt=8 … (oa-lane/results/{strix1,spark1}/cells/cell_old/)
the tests would catch index corruption old test binary vs new colibri.c: 2 FAILED at exactly the fmt=8 refusal pins, fmt=6 pins unchanged; the CUDA sizing probe's negative control fails on a wrong block-count formula
the shard-path hole is closed CUDA=1 shard-refuse guard run on GB10: layer_cuda_shard_kvb refusal probe: ok (oa-lane/results/spark1/cells/shard_kvb_refuse/)
fmt=8 absorb serves end-to-end on CUDA two-slot batched serve on GB10, both slots 200, engine survives, boot witness routed experts + resident dense tensors captured; the negative twin (CPU-dense boot line) fails the same test on the same box (oa-lane/results/spark1/cells/cell_new_cuda_absorb/, …/negative_polarity_ce501dd/). The witness proves CUDA eligibility and path selection; kernel execution follows from the source-verified dispatch chain (no silent-fallback branch between selection and launch) rather than a separate artifact
fmt=8 LUT survives re-init lifecycle pin executed on GB10: "shutdown AND init both force a republish"
HIP lane sees no new warning HIP syntax gate: warning identity 1090 = 1090 vs the dev baseline (run at ce501dd; the delta to 77d8939 is Python tests and docs only)
Metal non-regression make METAL=1: zero warnings at every commit in the series; metal-test ok under both COLI_METAL_RESSET states
Fuller matrix, review record, and origin accounting

Review record: four bisectable commits (shared header → CPU engine → CUDA engine → tests), each built from make clean and passing the full suite standing alone; each commit reviewed by an independent blind validator and a deep auditor, with fix rounds closed at primary source; the assembled program then passed a program-head deep audit and a final pre-push verification. One review finding is worth naming: an earlier version of the CUDA-absorb end-to-end test went green without ever reaching the CUDA absorb path (the eligibility knob was stripped by the test's own env hygiene). The test now injects the knob, asserts the engine's boot-mode line, and echoes what it observed on pass and fail, so that class of vacuous green cannot recur.

Origin accounting for a re-derive onto current dev: the decode branches and CUDA arms are byte-identical or offset-only carries of the previously reviewed content; three pre-existing defects found during re-derivation are fixed here and disclosed above (the fmt=5/6 shard-allowlist misreads, the LUT re-init hole, and a missing fp8_format.h dependency in Makefile.deepseek-v4 that left five DeepSeek-V4 objects stale against a block-size edit); a few stale line-number anchors in docs/FORMATS.md and the repack tool's docstring were corrected while those files were open. The end-to-end test and the refusal canary diverge from the earlier hardened lineage by exactly the reviewed hardening described above.

The mint→load regression test is armed only where torch/numpy/safetensors are installed (it names its skip otherwise); upstream CI does not install torch, so it SKIPs there and runs on our fleet. Compiled-binary digests for the CUDA build varied run-to-run on identical sources (nvcc non-determinism; the gcc build was byte-stable) — disclosed as a reproducibility note, not a behavior.

The fmt=8/fmt=6 scale-byte VRAM accounting interaction disclosed in the original submission was fixed separately in #1100 (merged 2026-08-19), which is in this PR's base; the diagnostic counters are correct here without further change.

Style: changed lines were held to the file's measured local idiom via a diff-scoped consistency check; no surrounding code was reformatted.

Durable vs current-state: decode branches, refusals, and the LUT gate are durable; GB10/sm_121 timings, skip counts, and warning counts are current-state (2026-09-03, base 387653f; dev tip 53c57f1 at push, no file overlap).

@monotophic

Copy link
Copy Markdown
Contributor Author

Rebased on 0282193.

More coming to build out support for the FP8 checkpoint-faithful container and full logprobs instrumentation support when using the OpenAI API.

@monotophic

Copy link
Copy Markdown
Contributor Author

Rebased onto dev @ 12b0fa1 (30 commits past the previous base 387653f); head 77d89390282193, force-with-lease.

What changed, by origin

  • Dev-motion-caused: c/Makefile only — upstream's header-prerequisite work (build: list the headers each engine includes as Makefile prerequisites #1284) and later rules expanded the
    prerequisite lists of colibri$(EXE), cuda-dll, hip-dll and backend_cuda.o; this branch's own addition
    (fp8_format.h in each of those four lists) is re-applied on top of the expanded lists. c/backend_metal.mm merged
    automatically against fix(metal): bit-exact fp8-e4m3 decode via ieee754 exponent bitcast #1346 (bit-exact fp8-e4m3 decode); this branch's one-line comment fix there is unchanged.
  • Pre-existing defects found by re-review: none.
  • Adjacent improvements: none.
  • Contribution content: byte-identical to 77d8939 on every file except the Makefile lines above (diff-of-diffs 0 lines
    over the branch's other 19 files).

Re-verified on 0282193

check result
make check (serial) exit 0 — 743 Python tests OK (43 skipped), all C test binaries OK
c/tests/test_makefile_deps.py (new upstream gate) 2 passed, 7 subtests passed
lint on changed lines, per commit (4 commits) no findings
preflight (preflight_push.sh) against a fresh fetch GO (base unmoved)
CI on the pushed head all passed (operator report)

Preflight's advisory about POSIX-only test constructs refers to c/tests/test_qt_addrow.c (fork/pipe/waitpid at
lines 359–370); that block sits inside #ifndef _WIN32 … #else … #endif (lines 358–390), so the Windows CI lane skips
it by construction — consistent with the green Windows run.

Authored by Claude Fable 5.1 in Claude Code, analysis in partnership with @monotophic

@monotophic

monotophic commented Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

Authored by Claude Fable 5.1 in Claude Code, analysis in partnership with @monotophic

Rebased onto current dev (87c54a93). No functional change on this branch's side — the 4-commit chain is preserved unsquashed with the same messages.

Files needing conflict resolution, all caused by dev motion since the last rebase:

  • c/quant.h: dev added an AVX2 e4m3/bf16 decode block right where this PR removes FP8_BLOCK/fp8_nblk. Resolved by keeping the whole AVX2 block and moving only the two definition lines out to fp8_format.h, as before. matmul_fp8 and the AVX2 helpers are untouched; FP8_BLOCK/fp8_nblk are each defined exactly once.
  • c/Makefile: dev expanded the qwen38, glm53 and cuda-test prerequisite lists, added four rules that name quant.h and did not exist when this PR was written (deepseek_v41, tests/test_e4m3_vector, tests/test_qwen38_tier_engine, tests/bench_router_select), and (941d5fe) added omp_tune.h to the deepseek_v41, qwen38, and glm53 rules. Resolved by keeping dev's full lists and pairing fp8_format.h right after quant.h wherever a rule names quant.h — since quant.h now includes fp8_format.h, a rule without the pairing is exactly the stale-binary case this PR's Makefile change exists to prevent. tests/test_makefile_deps.py passes on the result.
    Everything else auto-merged with this PR's hunks verbatim (diff against the new base identical to the diff against the old base on the other 19 files).

Re-verified on 619d51fa: make check exit 0 (983 tests, no failures), every one of the 4 commits builds, METAL=1 build with zero warnings, metal-test under both COLI_METAL_RESSET states, the HIP syntax lane replica (rocm 6.2 container), and the branch's fp8 test modules. Lint on changed lines: no findings.

@JustVugg: Checking in on the viability of this PR. Is there interest in merging the fmt=8 absorb decode path? The format itself is already in via the mint tooling; this PR closes the one path that still refuses it. I have a personal interest in having the option to preserve the published checkpoint quality for the resident spine for GLM 5.2/5.3 and also a full checkpoint quality container for a quant degradation benchmark. The quant performance/cost experiments are also the driver for my associated PRs that build out proper instrumentation for the OpenAI API endpoint.

I've got everything I need integrated and working at my end so my container experiments are not blocked by this stuff sitting unmerged, but I would prefer not to be investing in an isolated fork and rebasing regularly to avoid drifting too far from what you are doing here. Steering input is welcome, my hope is that these contributions would be generally useful for anyone wanting to use Colibri for serious work, including model tuning/development and research.

@monotophic

Copy link
Copy Markdown
Contributor Author

Authored by Claude Fable 5.1 in Claude Code, analysis in partnership with @monotophic

Field evidence for the CPU fmt=8 absorb path in this PR, measured on real fmt=8 containers.

Identity gate (2026-09-07, this PR's build 0282193e; spark GB10 class, CPU only, temperature 0, MTP off): direct decode with the absorb path on (ABSORB=1) versus off (ABSORB=0), same binary, same five raw prompts, 64 new tokens each.

container format identical extracted bytes per prompt (on / off)
we-v1 fp8 e4m3 experts, int3 g64 5/5 325/325 · 377/377 · 345/345 · 312/312 · 297/297
wq-v1 fp8 e4m3 experts, int4 g64 5/5 370/370 · 379/379 · 353/353 · 301/301 · 314/314

Control: the dev binary of the day (e1efc687) under the identical ABSORB=1 command on we-v1 exits 1 with qt_addrow: unsupported fmt=8 for the per-row-scale absorb path — so the ABSORB=1 arm reaches the absorb surface, and this PR is what makes it pass. Two-slot serving through openai_server.py on both containers completed 5/5 requests with zero refusals.

Repeated 2026-09-15 with this PR integrated alongside #1353 and #1355#1357 on dev edd4f48d: we-v1 on aarch64 (spark GB10) 5/5 identical with the same byte counts as above; wq-v1 on x86_64 (Strix Halo, AVX-512 absorb specialisation) 5/5 identical; dev control refused on both. A serve battery on the same integrated build (two KV slots, so the absorb path is forced) returned text identical to this PR's standalone build on the default completion, chat and Anthropic-messages requests.

Not measured: CUDA runtime behaviour of the fmt=8 absorb kernels (no CUDA host in these runs; the kernels are verified by compilation and the HIP syntax lane only). Serving rate on the CPU absorb path was ~0.31 tok/s on a GB10 for we-v1, so these are identity measurements, not throughput ones.

Records: F-5 gate colibri_lab/dispatch/2026-09-07-f5-gate/results/ and integration cells colibri_lab/dispatch/2026-09-15-integ-cells/results/ (available on request).

The fmt=8 (fp8-e4m3-b128) 128-wide scale-block edge lived in quant.h,
which is heavy to include from a translation unit that only needs the
block geometry (LUTs, OpenMP pragmas, _Thread_local -- the last is
what nvcc's C++ front end rejects, per the Makefile's own comment);
the CUDA backend today restates 128 / >>7 as bare literals for
exactly that reason.

Carve the two definitions out into a deliberately tiny plain-C header
that any translation unit can include, and include it from quant.h in
their old place. Zero behavior change. Moving the constant first,
alone, keeps any follow-up that converts a literal user to the named
constant down to a diff a reviewer can check by eye.

Every prerequisite list naming quant.h in both build files (Makefile,
Makefile.deepseek-v4) gains fp8_format.h beside it, so an edit to the
new header rebuilds everything that transitively includes it. Prose
pointers to the old location (docs/FORMATS.md's fmt=8 entry, comments
in colibri.c and backend_metal.mm) now name fp8_format.h; FORMATS.md's
fmt=8 line numbers had rotted and are corrected while there.
qt_addrow and qt_matvec_rows gain explicit fmt=8 (fp8-e4m3-b128)
branches: one f32 scale per 128x128 block (block-row-major,
ceil(O/128)*ceil(I/128) entries), raw e4m3 bytes read from t->q8 (t->q4
is NULL for this format), block-scale indexing identical to quant.h's
matmul_fp8, and qt_matvec_rows double-accumulates across blocks with
matmul_fp8's own widen-then-multiply rounding (a+=(double)acc*sc, the
same convention as this function's grouped fmt=4 arm). Before these
branches fmt=8 hit the functions' fmt guards and refused; the guard
comments and messages now list fmt 0/4/5/8 as handled above, and
test_qt_addrow's refusal pins for fmt=8 become conformance tests of the
new arms against an element-order-independent reference (flat
per-element loop vs the arms' block-by-block walk; the reference reuses
the declared geometry constants on purpose -- they ARE the format under
test -- so fp8_format.h's not-independent list gains this file).

This is the engine half of a two-sided integration whose tool half
already shipped: tools/repack_fp8_passthrough.py mints kv_b_proj (kind
"kvb") as byte-preserved fmt=8, and attention_rows' absorb gate cannot
be bypassed on the batched serving path, so these arms are the decode
support that container needs. The tool's docstring and the test header
are updated to say so (both previously described the pre-landing world;
the CUDA absorb kernels still refuse fmt=8 and the docstring now says
exactly that).

layer_cuda_shard_kvb gains an explicit shard-format allowlist (fmt
1/2/3/4, the only strides its rb/weights/scale arithmetic computes).
fmt=8 kv_b previously survived here only by accident -- q4==NULL made
the upload's !weights check reject it downstream -- while fmt=5 and
fmt=6 kv_b computed a WRONG row-byte stride (rb=(I+3)/4 against
i3_rowbytes' 24B-per-64-group layout) with a non-NULL q4 and uploaded
garbage silently; the allowlist closes both live misreads. Refuse by
name instead, before any pointer/stride use, notice-only (sharding is
an opt-in optimization; the un-sharded kv_b staying whole on its home
device is the correct behavior), bounded once per process per fmt.
The guard's own pin (tests/test_shard_kvb_refuse.c + its Makefile
rule, CUDA-gated) is deferred to the A-series test rung by plan.
weight_at gains an fmt=8 branch (raw e4m3 bytes through the shared
c_e4m3 LUT, the same table quant_matmul's dense fmt=8 branch reads) and
absorb_scale gains the per-128x128-block scale indexing
([ceil(O/128), ceil(I/128)], the same geometry as quant_matmul and the
CPU arms), so the absorb host wrappers decode fmt=8 kv_b on-device.
coli_cuda_weight_at_supported admits fmt=8 -- one predicate for the
launch-site gates and the device backstop, so they cannot drift apart;
uploads stay gated on the published e4m3 LUT, and coli_cuda_shutdown
clears that flag so a shutdown/re-init span republishes before any
fmt=8 tensor exists. The CUDA-side pins move with the behavior:
test_cuda_fmt_guard's fmt=8 expectation flips to supported,
test_cuda_fmt_trap grows fmt=8 trap/decode coverage, and
test_backend_cuda exercises the absorb decode end to end.

The CUDA translation unit now includes fp8_format.h (the block edge is
the named constant, not restated literals) and pins it with a
static_assert: on-disk fmt=8 containers carry ceil(dim/128)-edged scale
grids, so FP8_BLOCK is container format, not a tunable. The six
CUDA-family Makefile rules gain the header as a prerequisite;
fp8_format.h's own header comment now truthfully names both backends,
and the shard-refusal comment in colibri.c returns to describing the
CUDA absorb kernels as a live fmt=8 path.

tests/test_shard_kvb_refuse joins the suite with its Makefile rule (a
loud SKIP without CUDA; with CUDA=1 it pins the layer_cuda_shard_kvb
refusals added alongside the CPU arms -- no GPU needed, every probed
path returns before any device context). The repack tool's docstring
drops its last pending-work caveat: both engine halves now exist.
…sion

test_fp8_serve_batch_e2e.py pins the fmt=8 kv_b batched-serve decode end
to end against a real container (named SKIP when none is present). Its
child environment is allowlist-scrubbed: ambient COLI_* and the bare
engine knobs are stripped; read-only model-LOCATION config passes
(COLI_MODEL_DIRS/COLI_MODEL_MIRROR/COLI_MMAP/COLI_DISK_WEIGHTS -- where
the weights ARE, not what the engine does with them); documented legacy
aliases are stripped alongside their primaries (SNAP_MIRROR, and TEMP
only when fully numeric per temp_from_env's own test); and ABSORB=0 for
the ratified non-absorb arm passes value-restricted. By default every
lane exercises the CPU absorb arms; the docstring states the lane
semantics exactly.

The CUDA fmt=8 absorb decode is opt-in only: a lane sets
COLI_FP8_E2E_CUDA_ABSORB=1 and the test INJECTS the exact engine bundle
the path needs -- COLI_CUDA_ATTN=1 (selects the CUDA absorb dispatch)
and CUDA_DENSE=1 (makes kv_b/o cuda_eligible; qt_load grants eligibility
only under g_cuda_dense, with no VRAM/budget gate, so CUDA_DENSE=1
reaching the child is sufficient on a fitting device). Injection, not
ambient passthrough: the fresh-env allowlist only keeps already-set
vars, so a bare CUDA_DENSE the lane had not exported would never reach
the engine -- the opt-in flag is the single source of the bundle, and
both knobs are recorded in the invocation-of-record kept-knobs line.
Under the opt-in the test also WITNESSES the engine's boot report,
requiring the routed+resident-dense line and failing outright on the
resident-dense-on-CPU line, so a misconfigured lane can never bank a
vacuous green (this witness caught a first attempt that value-allowed
the bundle through the scrub instead of injecting it).

test_fp8_refusal_canary.py runs in CI without a container: it pins that
each absorb function keeps AT LEAST ONE refusal matching the e2e bite
matcher (existential on purpose -- an additional differently-worded
guard is new coverage, not drift), and the matcher's family selectivity
against both a synthetic near-miss and the real in-tree sibling
(layer_cuda_shard_kvb's shard refusal).

test_e8x4g64_mint_load.py + test_e8x4g64_loader.c pin the wq-v0
container class through the REAL tools/convert_fp8_to_int4.py CLI and
the REAL C loader at toy scale (mint half of the mint->load->run
regression bar), plus the D-2 duplicate-tensor-name refusal on a
tool-produced container. The Python driver compiles the C half itself,
mirroring the Makefile's CFLAGS; the three Python files join the suite
through the tests/test_*.py unittest glob (the C harness is built by
its driver, not by any Makefile rule).

Also trued-up while here: the repack tool's external-files line table
and docs/FORMATS.md's routed-expert/stamp-ingest anchors had rotted
(pre-existing; the series' own insertions moved three of them further)
and now match this head, as do the fmt-dispatch anchors
(qt_alloc/qt_bytes/qt_scale_bytes/FMT_NAMES/qt_resolve_fmt) and the
block's provenance line, which now states head-relative numbering.

The e2e and canary deliberately DIVERGE from the hardened D-I2 lineage
(6c29f81) by exactly the reviewed hardening -- the injected/witnessed
CUDA-absorb opt-in with the alias/location env policy, and the
existential matcher pin with its real in-tree near-miss -- so this
program tree, not the D-I2 branch, is the version of record for both.

Positive coverage is echoed, not inferred: the CUDA-absorb boot-line
witness prints the exact observed "[CUDA] mode: ..." line (pass and
fail), and the concurrency witness prints each slot's admission
queue-wait, so a green run leaves a captured artifact of what actually
ran rather than only a succeeded assertion. This is permanent test
behavior: upstream CI logs then show the routed+dense residency and the
non-serialized mux verbatim. (The bite lane already echoes its matched
refusal line; generated-text quality stays with the numeric battery, not
echoed here.)
@monotophic

Copy link
Copy Markdown
Contributor Author

Authored by Claude Fable 5.1 in Claude Code, analysis in partnership with @monotophic

Rebased onto current dev (b5968eb6). No functional change on this branch's side — the 4-commit chain is preserved unsquashed with the same messages.

One file needed conflict resolution, caused by dev motion since the last rebase: c/Makefile, where #1558 added kv_prefix.h to the deepseek_v41 prerequisite list on the same line this PR adds fp8_format.h. Resolved by keeping both. Every other file merged with this PR's hunks verbatim — the diff against the new base is identical to the diff against the old base except for that one line.

Re-verified on f25ac455: make check exit 0 (1002 tests, no failures), every one of the 4 commits builds, METAL=1 build with zero warnings, metal-test under both COLI_METAL_RESSET states, the HIP syntax lane replica (rocm 6.2 container), the branch's fp8 test modules, and tests/test_makefile_deps.py. Lint on changed lines: no findings.

@JustVugg

Copy link
Copy Markdown
Owner

Reviewed, and this is close. The CPU arm mirrors matmul_fp8 exactly, same block index, same scale stride, same partial-tail handling, and test_qt_addrow is the standout of the set: an independent per-element reference decoder rather than the block-batched shape, a partial-column tail with a stated negative control, direct parity against the reference, a NaN pin, and byte-identity re-checks for the other six formats so the guard's placement is pinned too. That is real coverage.

One change before I merge, and it is wording rather than code. The behavioural-contract bullet says fmt=8 kv_b decodes bit-consistently with the reference on both backends. The CUDA side does not, and your own comment in backend_cuda.cu says so plainly: the fp8 accumulation convention is not carried into the absorb kernels and the divergence is an accepted documented class. The capstone matrix is honest about this; the one-line summary is not. Please make the summary match the comment.

Worth knowing for sequencing rather than for you to fix: the roughly seventy lines of Makefile dependency churn will conflict with most other open PRs touching that file, so this wants to land either before them or well after.

@monotophic

Copy link
Copy Markdown
Contributor Author

Authored by Claude Fable 5.1 in Claude Code, analysis in partnership with @monotophic

Agreed, and fixed in the PR body — wording only, no push.

  • The behavioral-contract bullet now states the two backends separately: CPU qt_addrow bit-exact and qt_matvec_rows within float-accumulation-order tolerance; CUDA within 1e-3 relative on the GB10 oracle, with the absorb kernels' per-element scaling named as the accepted RDNA2 (gfx1030) field report: AMD backend works; greedy decode not token-stable across mixed CPU/GPU expert tiers #510 divergence class, matching the backend_cuda.cu comment.
  • While there, the two capstone labels that said "exact" ("CPU decode is exact", "CUDA decode is exact") were the same summary-vs-evidence gap one tier down, so they now read "matches the reference" / "matches the reference within the documented tolerance". The evidence cells are unchanged.

Head is still f25ac455; nothing in the branch changed.

@JustVugg: Understood on the Makefile sequencing. Knowing that this is getting review is sufficient to keep me focused on making sure this is ready to merge when it fits your broader priorities. Thanks!

@monotophic

Copy link
Copy Markdown
Contributor Author

Heads-up on an interaction with #1356, which is in review now: #1356 and this PR both add entries to c/Makefile and will conflict there — only there. We merged them in both orders in a scratch worktree to check: the one conflicted path is c/Makefile in each direction, and c/colibri.c merges cleanly in both orders even though both PRs modify it. So it is the ordinary consequence of two PRs each adding build targets, not an engine-level collision.

Whichever lands second, we re-merge dev and push the resolution within a day. Nothing is needed from you here.

One related note, if this PR lands first, quant.h gains fp8_format.h, and #1356's two new Makefile rules should list it too, exactly as their sibling rules do — a one-line follow-up in whichever order the two land.

Authored by Opus 5 in Claude Code, analysis in partnership with @Monotophic

This branch has not been deployed

No deployments
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.

2 participants