formats: self-describing container stamp (TRUST-VERIFY-REFUSE) + FORMATS.md registry (#524)#529
formats: self-describing container stamp (TRUST-VERIFY-REFUSE) + FORMATS.md registry (#524)#529monotophic wants to merge 19 commits into
Conversation
|
Working on the Windows / MinGW fail. Will update with a fix shortly. |
902a5ea to
a7851fc
Compare
|
Fixed and ready for review |
|
Reviewed alongside #528. Answering the question you asked in #524 first, because it's the one that decides whether this convention is safe: Does the stamp ever override byte-arithmetic inference? No — verified at every call site. Each stamp-consulting branch is gated on a candidate that is already byte-consistent: the fmt=6 block sits inside Four notes, none fatal:
Test coverage is genuinely there and tests what it claims — the fork-based stderr assertions cover both stamp directions, the unrelated-stamp case and the UE8M0-stays-refused-when-stamped cases. The one false claim I found is shared with #528: the comment asserting Verdict: good to merge on its own merits, but it's stacked on #528, so it goes in after the collision policy there is inverted. If you'd rather land the registry sooner, rebasing this onto |
…tVugg#528 restack Maintainer review, JustVugg#529 (shared with JustVugg#528's fix): FORMATS.md cites specific c/colibri.c line numbers and branch/commit SHAs for every format row and for the metadata-stamp mechanism. Restacking fmt7/stamp-registry onto the revised fmt7/fp8-passthrough-rev1 (the JustVugg#528 INVERSION/qt_wire_split/stale-comment commits) shifted every colibri.c line number after qt_bytes(): qt_scale_bytes 216->223, qt_alloc 893->910, qt_resolve_fmt (and its SECOND DESIGN LANDMINE comment) 1144->1161, the FMT_NAMES table 1104->1121. Verified against the actual restacked file (grep for each cited function's definition line), not recomputed by offset arithmetic. The "Known formats" intro and "Sources for all rows" preamble also named the pre-restack branch/commit (fmt7/stamp-registry @ base 9a1690e) -- updated to fmt7/stamp-registry-rev1 @ base 696ce96, restacked on fmt7/fp8-passthrough-rev1 @ a77421d, with a note that these line numbers need re-verifying again if the branch is rebased further. No code changes, no test changes -- doc-only.
…qs scope Maintainer review, JustVugg#529 (four non-fatal notes): 1. Non-retroactivity: FORMATS.md gains an explicit section stating the stamp is a forward convention only -- it describes containers a stamping tool writes going forward, never retroactively. Every pre-convention container (including every container this repo's own tooling has ever produced) is simply unstamped, and byte-arithmetic inference alone decides for it, exactly as before this feature existed. The registry is not a migration path: there is no in-place upgrade for an existing container, only re-producing it through a stamping tool. 2. Stamp-map scan bound (orchestrator decision: CAP, not hash): st.h's st_fmt_stamp_ingest now caps the number of __metadata__["colibri.fmt"] entries it will ingest across a container's shards at ST_FMT_STAMP_MAX (4096) and refuses (exit(1)) past it, naming the bound in the error. Stamps are a resident-tensor convention -- a handful to a few hundred entries per model -- never the tens of thousands of routed-expert tensors a large MoE checkpoint carries; a container exceeding the cap is malformed by construction. tests/test_fp8_load.c tests both sides of the boundary: exactly ST_FMT_STAMP_MAX entries loads fine (no off-by-one false refusal), one more refuses (fork+waitpid, matching this suite's existing exit(1)-testing convention). 3. Discovery-time abort surface: st_fmt_stamp_ingest's exit(1) calls (this commit's cap included) fire from st_init_multi's shard-header-parse loop -- container DISCOVERY time, before any tensor is resolved against the model or a single weight byte is read. Documented at the call site (st.h) and in FORMATS.md: this is coarser-grained and earlier than qt_resolve_fmt/qt_verify_fmt_stamp's own per-tensor refusals, and the two are distinguishable at a glance -- this surface's messages name a shard FILE, the per-tensor surface names a TENSOR. 4. Non-.qs stamps (orchestrator decision: scope the claim, don't widen verification): confirmed at the source that qt_from_disk only ever calls st_fmt_stamp inside its `.qs`-sidecar branch -- a colibri.fmt entry naming any other tensor (raw f32/bf16, norm, router, embed/ lm_head) is ingested and stored like any other entry but never looked up, so it has no effect. FORMATS.md and st_fmt_stamp's own comment now state this scope explicitly rather than leaving it implicit. RAN (this commit): make clean && make portable -> clean rebuild, zero warnings. make test-c -> full C suite green, exit 0 (includes the two new stamp-map-cap cases). make test-python -> 155 tests, 10 skipped, 0 failures, exit 0. make glm METAL=1 -> clean rebuild, zero warnings. make metal-test under COLI_METAL_RESSET=0 AND =1: both full suite green, exit 0.
a7851fc to
fe9ac93
Compare
|
Authored by Fable 5 in Claude Code, analysis in partnership with @monotophic All four notes taken; restacked on the revised #528. Head
Also corrected here: two FORMATS.md passages still described the On your unstacking offer — keeping it stacked, deliberately: after the Verification rode the same upgraded process as #528: the spec-blind |
Ports fmt6/fp8-passthrough-r2@150f15b (the CPU read path -- quant.h's E4M3_LUT/e4m3_decode/matmul_fp8, colibri.c's qt_bytes/qt_scale_bytes/ qt_from_disk/qt_resolve_fmt fmt=1-vs-fp8 disambiguation) onto dev 9baae9b, renumbered fmt=100 -> fmt=7 throughout (code, tests, comments): the maintainer assigned fmt=7 as this format's public ordinal on JustVugg#524, so this branch stops using the private-block number the format developed under. Mechanical rename per the PRIVATE ORDINAL BLOCK convention's own "find-and-replace, zero on-disk impact" promise -- qt_resolve_fmt still infers format from byte arithmetic alone; nothing on disk ever encoded the number 100 or the number 7. Two structural differences from the source commit, both scoped by the maintainer's JustVugg#524 reply (PR 1 = CPU+repack+collision/refusal+Metal; registry+stamp move to a separate PR): - qt_resolve_fmt has NO stamped_name parameter and never will in this branch -- its signature is the plain 6-argument form every call site already uses (name,O,I,nb,ns,gs). A self-describing container stamp that could resolve a genuine byte-collision instead of refusing it is a follow-up proposal, called out as such in this function's own header comment and inside THE DESIGN LANDMINE's comment, not implemented here. - fmt=7's scale ENCODING is documented as a declared property of the format, not a hardcoded constant (QT struct comment + qt_resolve_fmt's new "SCALE ENCODING IS A DECLARED PROPERTY" comment). f32 (4 bytes/ block) is what this build implements. UE8M0 (1 byte/block, power-of-two exponent) is a REAL, distinct encoding the same weight geometry ships with -- DeepSeek-V4, per the maintainer's JustVugg#524 finding -- recognized by its own byte signature (ns==ceil(O/128)*ceil(I/128)) inside the fmt=1-vs-fmt=7 landmine and refused BY NAME rather than silently misread as truncated/corrupt f32 or matched to the wrong candidate. Checked for collision against fmt=1's and fmt=7-f32's own ns arithmetic: realistically distinct for GLM-sized shapes, but not categorically -- the same small-O regime that already makes the f32 landmine possible also makes a ue8m0 collision possible (documented in-code, new test case, still refuses either way). tests/test_fp8_load.c's disambiguation suite (Part A) carries forward unchanged in substance (renumbered), plus a new Part A3 (test_ue8m0_scale_refusal) covering both the clean UE8M0 signature and the small-O ue8m0-vs-fmt=1 collision. Part C (check_fp8_bytes) carries forward: it exercises qt_bytes()/qt_scale_bytes(), CPU-read-path functionality added in this same commit, not the metadata stamp it was originally bundled with upstream in the source branch's history. Does NOT yet include: the fmt=6-vs-fmt=7 "SECOND DESIGN LANDMINE" collision reconciliation (source branch's final commit) -- qt_resolve_fmt here still has dev's unconditional fmt=6 early-return, same gap the source branch's first port commit deliberately left open. Follow-up commit on this branch closes it. Does NOT yet include the Metal kernel, the repack tool, or the end-to-end loader test -- following commits on this branch. FIX ROUND (Windows CI): test_fp8_load.c's expect_refuse() forked a child to catch qt_resolve_fmt's exit(1) refusals -- MinGW has no fork()/sys/wait.h. Gated <unistd.h>/<sys/wait.h> and expect_refuse's body behind #ifndef _WIN32, mirroring tests/test_st_pread.c's own established Windows arm for exactly this problem (fork-based exit(1) checks): the Windows arm prints an explicit "skipped on Windows (no fork): <tag>" line per skipped case (visible, never silent) and returns pass-through (1) rather than failing the build; every non-refusing case (expect_fmt and its callers) still runs and asserts for real on Windows. No product code touched by this fix -- test-file-only, per the CI failure's own diagnosis (fatal error on tests/test_fp8_load.c:51, compile-time only). RAN (this commit): make glm METAL=1 (clean rebuild) -- zero warnings. ./tests/test_fp8_passthrough -> "fp8 passthrough CPU tests: ok". ./tests/test_fp8_load -> "fp8 loader-seam tests: ok" (Part A + A3 + B + C, including both new ue8m0 refusal cases and the renumbered fmt=1-vs-fmt=7 suite; POSIX refusal path confirmed still running, not skipped, on this platform -- zero "skipped on Windows" lines in the output). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Ports fmt6/fp8-passthrough-r2@7baaada onto this branch's fmt=7 renumbering (previous commit): mm_gemv shader's fmt==7 branch (in-kernel e4m3 bit decode, OCP E4M3-FN NaN policy matching quant.h's e4m3_decode exactly; block-scale indexed scale[(o/128)*nblkI+i/128] and folded into acc), fmt_bytes()/fmt_scale_bytes() fmt==7 cases, coli_metal_matmul's explicit allow-list entry (fmt>3 && fmt!=7, since 7 is no longer adjacent to the public 0-3 range this file otherwise supports), all renumbered from the source commit's fmt=100. coli_metal_gemm/bind_gemv unchanged: fmt=7 was never in scope for either (matches matmul_qt_ex's CPU-dispatch exclusion from the previous commit). tests/test_backend_metal.mm: the fmt=7 GPU test group (own bit- manipulation CPU reference decode, magnitude-relative block-scale oracle, 7 shapes incl. block-edge/degenerate/non-square-stride-audit cases, an exhaustive 256-code LUT-exactness check run through the actual GPU kernel, and a gate-off check confirming coli_metal_gemm refuses fmt=7/returns 0), renumbered. Also carries forward run_fp8_moe_gate (originally landed alongside the source branch's metadata-stamp commit, but content-wise it is pure Metal-gate regression coverage with no stamp dependency: proves BY TEST, with deliberately-invalid 0xdeadbeef weight/ scale pointers that must never be dereferenced, that coli_metal_moe_block/moe_submit's existing `fmt != 1 && fmt != 2` gate already refuses fmt=7 before MB_BUILD's pointer-selection ternary could submit the wrong pointer -- in scope for this PR's "Metal kernel + all its tests", independent of the registry/stamp PR). RAN (this commit): make glm METAL=1 (clean rebuild) -- zero warnings. make metal-test under COLI_METAL_RESSET=0: full suite green, exit 0, including all 11 fp8 GPU lines (LUT 0/256 mismatches; 7 shape cases worst_rel in [3e-8, 7e-8]; GEMM gate rc=0; MB_BUILD/moe_submit gate rc=0) and every pre-existing case (int8/int4/int2/f32, moe_block, gemm S=64, fused attention, top-8 select) unaffected. make metal-test under COLI_METAL_RESSET=1: same, plus "[METAL] residency-set: on (macOS 15+, moe_submit skips per-buffer useResource:)" confirmed active. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Ports fmt6/fp8-passthrough-r2@b927911 (repack tool folded with its kv_b_proj self-review fix, as the source branch shipped it), renumbered fmt=100 -> fmt=7. This is the tool's PRE-metadata-stamp shape: it writes no __metadata__["colibri.fmt"] stamp, matching this PR's scope (stamp writer + reader + registry are a separate, follow-up PR per the maintainer's JustVugg#524 reply). The module docstring says so explicitly now, and documents the scale-encoding property this tool implements (f32 block scales only -- Z.ai's GLM-5.2-FP8 checkpoints ship f32 weight_scale_inv, so there is no UE8M0 source data this tool would ever need to convert). tools/repack_fp8_passthrough.py: unlike convert_fp8_to_int4.py (dequant -> requant to a lossy format), copies fp8 weight bytes AS-IS and only renames/reshapes the _scale_inv sidecar to the engine's .qs convention (flat F32, ceil(O/128)*ceil(I/128) elements). Selection reuses convert_fp8_to_int4's classify()/check_or_record_params to target resident kinds only: shared expert, o_proj, other attention projections, dense-MLP first layers, and the generic resident fallback. Routed experts stay on the existing int4-g64 path; kv_b_proj is excluded because the CPU MLA-absorb path (qt_addrow/qt_matvec_rows) and the CUDA absorb kernels have no fmt=7 case and would silently misread it as int2-packed data. _check_geometry refuses (ValueError) rather than silently repack a shard whose _scale_inv shape doesn't match ceil(O/128)xceil(I/128) -- the write-side twin of qt_resolve_fmt's read-side "THE DESIGN LANDMINE" refusal. tests/test_fp8_repack.py: synthetic-fixture-only suite (glm_fp8_emit's exact real-checkpoint FP8 layout, no real Z.ai shard read or written) covering selection (resident kinds byte-preserved; routed experts/io/ f32/kv_b_proj excluded), byte-for-byte weight preservation + .qs rename, the write-side geometry-refusal path, --dry-run writing nothing, and the JustVugg#383-class resume/params-guard idiom. Renumbered from the source's fmt=100. tools/README.md: one-line entry for the tool, renumbered. RAN: python3 -m unittest tests.test_fp8_repack -v -- 9/9 tests pass (this commit, torch + safetensors present in this environment). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Ports fmt6/fp8-passthrough-r2@b2aea6c, renumbered fmt=100 -> fmt=7 and
adapted to drop the metadata-stamp check the source commit's version
carried: this branch's repack tool (previous commit) writes no
__metadata__ stamp, so there is nothing to sanity-check on that axis.
Closes the last coverage gap: neither test_fp8_load.c (hand-authored
wire-format fixtures -- proves the LOADER's own logic in isolation) nor
test_fp8_repack.py (proves the WRITER's Python-side output shape, never
invokes the C loader) exercises the real round trip.
tests/test_fp8_e2e_loader.c: minimal harness (`#define main
coli_glm_main_unused; #include "../colibri.c"`, same pattern as every
other colibri.c-embedding test in this suite). Takes a container
directory plus (name,O,I) triples on argv, calls the REAL qt_from_disk
for each, and asserts fmt==7 resolved, q8/s both non-NULL, and every
dequantized value finite.
tests/test_fp8_e2e_repack_load.py: builds a synthetic FP8 checkpoint via
tools/glm_fp8_emit.py, repacks it with the REAL
tools/repack_fp8_passthrough.py via subprocess (the actual CLI entry
point, not an imported-function reimplementation), sanity-checks the
real tool's own selection by tensor presence before ever invoking the C
side (so a failure localizes to tool vs. loader) -- and, since this PR's
tool writes no container metadata stamp, additionally asserts
__metadata__ is ABSENT from the repacked output (a regression guard that
this PR's tool really stays out of the stamp PR's scope, not just an
omitted assertion) -- compiles test_fp8_e2e_loader.c with
production-mirroring flags (asserted zero warnings), and runs it against
the real repacked output directory. Includes a routed-expert tensor and
an f32 norm as negative controls, proving the real tool's selection
logic held on an actual round trip, not just in test_fp8_repack.py's own
synthetic-fixture suite.
This closes out Branch A / PR 1's content: CPU read path + matmul_fp8,
Metal kernel, repack tool (no stamp), collision/refusal logic
(fmt=1-vs-fmt=7 THE DESIGN LANDMINE + fmt=6-vs-fmt=7 SECOND DESIGN
LANDMINE, both refuse-unconditionally, no stamped_name parameter
anywhere), the UE8M0 scale-encoding recognition seam, and now this
round-trip test. The metadata stamp itself (writer, qt_verify_fmt_stamp,
stamped_name plumbing, docs/FORMATS.md) is Branch B / PR 2, stacked on
top of this branch.
RAN (this commit): python3 -m unittest tests.test_fp8_e2e_repack_load -v
-> 1/1 pass ("ok"). make clean && make glm METAL=1 -> clean rebuild,
zero warnings. make test-c -> full C suite green, exit 0 (includes
test_fp8_passthrough, test_fp8_load, test_int3/test_int3_load, and every
other pre-existing case). make test-python -> 152 tests, 10 skipped, 0
failures, exit 0 (includes this branch's test_fp8_repack and
test_fp8_e2e_repack_load, plus dev's own suite). make metal-test under
COLI_METAL_RESSET=0 AND =1: both full suite green, exit 0, all fp8 GPU
lines ok, RESSET=1 confirmed active via "[METAL] residency-set: on".
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Maintainer review, JustVugg#528 (the load-bearing blocker): self_attn.o_proj.weight loads as [D,H*v_head] = [6144,16384] on GLM-5.2 -- nblkO=ceil(6144/128)=48, nblkI=ceil(16384/128)=128, product=6144==O, so a genuine, valid, pre-existing int8-row o_proj tensor satisfies BOTH the per-row (fmt=1) and per-128x128-block (fmt=7) scale-byte-count conventions at once. qt_resolve_fmt's prior behavior was to refuse this shape unconditionally (exit(1)) -- a real regression on an ordinary, already-shipping model, strictly worse than the misread it guarded against. Confirmed at shard-header level on this repo's own glm52_v1_e8x4g64 container: o_proj weight U8 100,663,296 B (==6144*16384), .qs scale blob 24,576 B (==6144*4==48*128*4, satisfying both formulas at once). colibri.c: qt_resolve_fmt's is_row&&is_blk branch no longer exits -- it falls through to fmt=1 (no explicit assignment needed: fmt is already 1 entering this branch, and the is_blk&&!is_row check further down evaluates false whenever is_row is true, per the comment left in its place). Sound specifically because the WRITER now refuses to ever emit an fmt=7 container at a shape satisfying this same predicate (see repack_fp8_passthrough.py below): an unstamped ambiguous tensor reaching this function is never a genuine fmt=7 candidate, so resolving to the incumbent format isn't a guess. The SECOND DESIGN LANDMINE (fmt=6 vs fmt=7 collision at I=98) and the UE8M0 recognized-not-implemented refusals are unrelated collision predicates and are unchanged -- still unconditional refusals. tools/repack_fp8_passthrough.py: _check_geometry() now also refuses to repack any [O,I] where nblkO*nblkI==O -- the exact shape the reader's collision predicate describes -- making the module docstring's existing promise ("never anything that could coincide with O*4 by accident") true rather than merely asserted. tools/fp8_collision_census.py (new): enumerates every resident-role tensor shape reachable from a container's config.json + safetensors headers (JSON header parse only, no tensor data, no model load) and evaluates the collision predicate over all of them -- run against this repo's own two GLM-5.2 containers (glm52_v1_e8x4g64, glm52_i4), the complete family is self_attn.o_proj at [6144,16384] (79 instances per container, one per layer including the MTP head layer) and nothing else among gate/up/down, q_a/q_b/kv_a/kv_b_proj, or routed/shared experts. tests/test_fp8_load.c: the three shapes the review named explicitly ([128,16384],[256,16384],[384,16384]) plus every other degenerate case in the same sweep (they all hit the identical is_row&&is_blk branch) flip from expect_refuse to expect_fmt(...,1,...); new explicit regression for the real o_proj shape [6144,16384]. tests/test_fp8_repack.py: new writer-refusal test for a well-formed-but-ambiguous [2,256] fixture (the smallest instance of the same family, distinct from the pre-existing malformed-shape refusal test). RAN (this commit): make clean && make portable -> clean rebuild, zero warnings. make test-c -> full C suite green, exit 0. make test-python -> 153 tests, 10 skipped, 0 failures, exit 0. make glm METAL=1 -> clean rebuild, zero warnings. make metal-test under COLI_METAL_RESSET=0 AND =1: both full suite green, exit 0.
Maintainer review, JustVugg#528: qt_scale_bytes() already existed and had the right fmt=7 branch (per-128x128-block scale count), but neither qt_wire_mmap nor qt_unwire_mmap actually called it -- both independently hardcoded scale_b=(int64_t)t->O*4 (per-row) inline, so the function compiled but was never referenced by its intended call sites. It only compiled clean because -Wno-unused-function is in CFLAGS: the claimed fmt=4/fmt=5/fmt=7 expert-pinning fix was never applied, and an fmt=7 tensor reaching qt_wire_mmap would get weight_b overshooting the allocation whenever nblk>O (mlocking/munlocking the wrong byte ranges on both halves). colibri.c: new qt_wire_split(t,&weight_b,&scale_b) -- the ONE place both call sites now get their weight/scale byte split from, calling qt_scale_bytes() internally. qt_wire_mmap and qt_unwire_mmap both replace their inline scale_b/weight_b computation with a call to it. qt_scale_bytes() is consequently no longer an orphan: it is exercised both directly (existing qt_bytes/qt_scale_bytes arithmetic tests) and through the real call sites via qt_wire_split. tests/test_fp8_load.c: check_wire_split() calls qt_wire_split() directly (no mlock syscall -- mem_wire's actual RLIMIT_MEMLOCK behavior is environment-dependent and not what changed) across fmt=1 (unaffected, per-row), fmt=4/fmt=5 (grouped formats qt_scale_bytes' own comment already named as previously broken too), and fmt=7 at two nblk>O shapes -- each asserting the split matches qt_scale_bytes()/qt_bytes() AND that the fmt=7 cases have actually moved off the old O*4 constant, not just coincidentally matched it. RAN (this commit): make clean && make portable -> clean rebuild, zero warnings. make test-c -> full C suite green, exit 0. make test-python -> 153 tests, 10 skipped, 0 failures, exit 0. make glm METAL=1 -> clean rebuild, zero warnings. make metal-test under COLI_METAL_RESSET=0 AND =1: both full suite green, exit 0. One-shot build with -Wno-unused-function removed from CFLAGS: qt_scale_bytes and qt_wire_split are both silent (no longer orphans); 8 unrelated, pre-existing orphan functions remain (st_init, st_prefetch, st_read_slice_f32, tier_pick_swap, stops_arm, cmp_fdesc, attention, and qt_unwire_mmap itself -- the last one only because this translation unit was built without -DCOLI_CUDA, its sole caller's #ifdef guard), all out of this PR's scope -- see the report.
Self-review while responding to the JustVugg#528 maintainer round: attention_rows' and layer_forward_rows' fmt!=7 guard comments claimed "coli_metal_attn_decode's WP_() macro and the mm_gemv shader it dispatches through (bind_gemv) have no fmt=7 case in this build" -- stale since this PR's own Metal kernel commit (96baf22, "Metal mm_gemv kernel for fmt=7 native FP8-e4m3 passthrough") added exactly that: mm_gemv's shader body has a real, working fmt==7 branch (fp8-e4m3 decode + per-128x128-block scale lookup, backend_metal.mm). The shader is not the gap. The actual reason these two fused-kernel paths still fail closed for fmt=7 is the WP_() macro immediately below each guard: `(q).fmt==1?(const void*)(q).q8:(const void*)(q).q4` picks q8 only for fmt==1 and q4 otherwise -- and q4 is NULL/unallocated for fmt=7 (same convention as fmt=1, see the QT struct comment), so an fmt=7 tensor reaching either fused path through WP_() would hand the kernel a NULL weight pointer, not a shader that misreads bytes as f32. Both guard comments now name the real blocker and note that fixing WP_() and threading fmt=7 through bind_gemv is a deferred follow-up (this PR's own GPU-path note), not attempted in this round. No code changes: the guards (fmt!=7 exclusions) and their behavior are unchanged, only the comments explaining WHY they exist. RAN (this commit): make clean && make portable -> clean rebuild, zero warnings. make test-c -> full C suite green, exit 0. make glm METAL=1 -> clean rebuild, zero warnings.
FIX ROUND, validator finding (mutation-proven gap): check_wire_split()
exercises qt_wire_split() directly, so it does NOT notice a mutation that
reverts ONLY qt_wire_mmap's and qt_unwire_mmap's call sites back to the old
inline scale_b=(int64_t)t->O*4 hardcode, leaving qt_wire_split() itself
untouched -- the two sites would simply stop calling the now-orphaned-again
helper, and nothing existing would catch it.
tests/test_fp8_load.c: new observer seam (#define mlock/munlock to
per-file names before the colibri.c #include, then define those names as
self-contained recorders after it -- both are external POSIX library
functions with no body in this translation unit, so there is nothing to
accidentally shadow-out-of-existence, unlike an attempted mem_wire()-level
shadow which was tried first and found NOT to work: mem_wire is a real,
internally-defined function, so renaming it renames its call sites too,
and qt_wire_mmap ends up calling the renamed-but-still-real function
directly, bypassing anything defined under the freed-up old name --
caught by inspecting the preprocessed output before it could hide a
broken test, see the comment left in its place). test_wire_site_regression()
calls the real qt_wire_mmap()/qt_unwire_mmap() and asserts, through that
seam, that the (addr,len) each site actually passes down to the platform
lock call matches qt_scale_bytes()/qt_bytes() -- observing the call sites'
own behavior, not a reimplementation of it. POSIX-only (mlock/munlock are
only called on the __APPLE__/__linux__/__FreeBSD__ arm; Windows uses
compat_mlock/compat_munlock, untouched here, matching this file's existing
POSIX-only test-seam convention).
PROVEN TO BITE (RAN): applied the exact mutation this test exists to
catch -- reverted only the two call sites to `int64_t scale_b=(int64_t)
t->O*4; int64_t weight_b=qt_bytes(t)-scale_b;`, leaving qt_wire_split()
itself byte-for-byte untouched (diffed against the pre-mutation file to
confirm) -- rebuilt, and got:
FAIL wire-site regression: qt_wire_mmap's SCALE mlock() call got len=8 want=512 (this is exactly what a reverted scale_b=O*4 hardcode breaks)
FAIL wire-site regression: qt_unwire_mmap's SCALE munlock() call got len=8 want=512 (this is exactly what a reverted scale_b=O*4 hardcode breaks)
fp8 loader-seam tests: 4 FAILED
(4 distinct CHECK() failures -- weight+scale on each of wire/unwire;
each line appears repeated in the raw run due to a pre-existing,
unrelated stdio-buffering artifact where this file's LATER fork()-based
stamp-refusal tests each flush a copy of the still-buffered output when
their child calls exit(1) -- confirmed by counting: exactly as many
repeats as later fork+exit(1) children, same total whether piped or
redirected to a file, and the "N FAILED" summary counts the real 4
distinct failures once each). Reverted the mutation (diffed back to
byte-identical with the pre-mutation file) and reran: "fp8 loader-seam
tests: ok", exit 0.
RAN (this commit): make clean && make portable -> clean rebuild, zero
warnings. make test-c -> full C suite green, exit 0. make test-python ->
155 tests, 10 skipped, 0 failures, exit 0. make glm METAL=1 -> clean
rebuild, zero warnings. make metal-test under COLI_METAL_RESSET=0 AND =1:
both full suite green, exit 0.
FIX ROUND, audit finding (SHOULD-FIX): qt_scale_bytes() had no explicit fmt=6 branch and fell back to the per-row O*4 formula. Verified at source before implementing (per instructions, not trusting the audit's own derivation): qt_from_disk's fmt==6 branch allocates t->s via qsalloc(1) -- ONE float, 4 bytes total, independent of O -- and its st_read_f32_cap call right below validates the SAME cardinality (`fmt==6 ? (int64_t)1 : ...`). qt_bytes()'s own fmt==6 branch already treats the scale as a fixed `+4` literal, never O*4. All three sites agree: fmt=6's scale allocation is a fixed 4 bytes, confirming the audit's claim rather than contradicting it. This function's own header comment used to ALSO claim fmt=6 tensors "never reach qt_wire_mmap/qt_unwire_mmap's mlock path in this build" because they "stay CPU-side" -- that conflated two unrelated mechanisms: qt_cuda_upload decides GPU-VRAM upload eligibility (fmt=5/6 genuinely excluded there, no CUDA kernel for either), while qt_wire_mmap/qt_unwire_mmap mlock HOST RAM pages under COLI_MMAP, an entirely CPU-side concern -- staying CPU-side is exactly what makes a tensor a wiring CANDIDATE, not exempt from it. expert_load_impl assigns fmt=6 to ESlot g/u/d exactly like fmt=4/5 (same qt_resolve_fmt call site), and pin_wire calls qt_wire_mmap/qt_unwire_mmap on every pinned ESlot's g/u/d unconditionally, with no format filter -- so a pinned E8/IQ3 expert under COLI_MMAP + mem_should_wire() does reach here. Before this fix, that path's O*4 fallback would return a scale_b wildly larger than the tensor's real 4-byte allocation, then mlock/munlock that many bytes starting at t->s -- past the end of a 4-byte allocation. This is a strict correction, not scope creep introduced by wiring the helper: pre-existing behavior at the old hardcoded call sites (scale_b=(int64_t)t->O*4, inline, before qt_wire_split existed) was identically wrong for fmt=6 already -- this closes the same gap the JustVugg#528 fix closed for fmt=4/5/7, for the one format that fix's own review round didn't reach. colibri.c: qt_scale_bytes() gains `if(t->fmt==6) return 4;`, and the header comment's stale reachability claim is corrected. tests/test_fp8_load.c: check_wire_split(6, ...) at a realistic expert shape (verifying the value moved off the old O*4 fallback) and at the O=1 degenerate shape (where O*4 coincidentally also equals 4, so only the new direct `want_scale==4` assertion -- not the generic "moved off O*4" guard -- catches a regression there; both are asserted). PROVEN TO BITE (RAN): removed the `if(t->fmt==6) return 4;` line only, rebuilt, ran the suite: FAIL tests/test_fp8_load.c:574: want_scale == 4 fp8 loader-seam tests: N FAILED Reverted (diffed back to byte-identical with the pre-mutation file) and reran clean: "fp8 loader-seam tests: ok", exit 0. RAN (this commit): make clean && make portable -> clean rebuild, zero warnings. make test-c -> full C suite green, exit 0. make test-python -> 155 tests, 10 skipped, 0 failures, exit 0. make glm METAL=1 -> clean rebuild, zero warnings. make metal-test under COLI_METAL_RESSET=0 AND =1: both full suite green, exit 0. One-shot -Wno-unused-function-stripped build: same 8 pre-existing, unrelated orphans as before this round, no new ones.
FIX ROUND 2, engine defect (clean-room conformance trial finding, the headline item): qt_addrow and qt_matvec_rows (colibri.c) serve the kv_b MLA-absorption CPU path. Both dispatch fmt 0/4/5 explicitly, then fall through assuming a PER-ROW scale (t->s[row]) followed by fmt=1/2/3 (qt_addrow) or fmt=0/1/2/3/4/5 via an if/else-if chain ending in a bare `else` (qt_matvec_rows) -- nothing stopped any OTHER fmt from reaching that fall-through. Reproduced (trial-verified, re-confirmed here): a fmt=7 QT (t->s holds ceil(O/128)*ceil(I/128) per-block floats, not O -- a [130,130] tensor has nblk=4, so t->s[row] overreads for every row past 3) reaches the fall-through's `t->q4+(int64_t)row*((I+3)/4)` -- t->q4 is NULL for fmt=7 (raw bytes live in t->q8 instead, same convention as fmt=1) -- and dereferences NULL-plus-offset: SIGSEGV. A fmt=6 QT (t->s is a FIXED 4-byte tag, qsalloc(1), not O floats) overreads t->s[row] for row>0 and then silently misreads the real E8/IQ3 lattice bytes in t->q4 as int2-packed data -- same bug SHAPE as JustVugg#298's CUDA absorb-kernel fix (this file's own fmt=4/5 branches exist for exactly this reason; fmt=6 was simply missed). Both functions now refuse loudly (stderr naming the function and the fmt, exit(1)) for any fmt they don't explicitly handle: qt_addrow gets an explicit `fmt!=1 && fmt!=2 && fmt!=3` guard before the per-row-scale read (fmt 0/4/5 already returned above); qt_matvec_rows' final bare `else` becomes `else if(fmt==3)` plus a new refusing `else`. Reachability note (context, not a scope excuse): both functions serve ONLY the kv_b absorb path, and tools/repack_fp8_passthrough.py deliberately excludes kv_b_proj from fmt=7 repacking -- so this fires only via a hand-slotted or ambiguous-collision container, never this repo's own tooling's output. Crash-instead-of-refuse is still a real defect (loud failure, every refusal names its condition), and the fmt=7 QT surface these functions can now be handed is one this same PR pair created. tests/tests/test_qt_addrow.c (new): fork+pipe+waitpid refusal tests (this suite's house pattern) for fmt=6 and fmt=7 through BOTH functions, using the coordinator's own [130,130] partial-block repro shape for fmt=7; the harness also detects a raw SIGSEGV (WIFSIGNALED) and reports it as the specific failure it is, rather than hanging or mis-reporting. Byte- identity checks for every format both functions still handle (0/1/2/3/4/5) against an independently-written reference dequantizer (not copy-pasted from either function -- restructured as a single per-element loop per format), epsilon-compared (float multiplication reassociation, e.g. qt_addrow's precomputed c=coef*scale vs the reference's coef*(scale*w[i]), is not bit-exact even though mathematically identical -- confirmed by inspecting actual failures at strict equality before relaxing, all last-ULP-only, no shape/decode errors). PROVEN TO BITE (RAN, full transcript in the report): reverted the guard in qt_addrow only, rebuilt, ran the new test -- reproduced the exact SIGSEGV (signal 11) for fmt=7 and the silent non-refusal for fmt=6, caught cleanly by the test's own signal-aware harness. Reverted (diffed back to byte-identical with the pre-mutation file), reran clean. Repeated independently for qt_matvec_rows' guard alone (qt_addrow's guard left intact) -- same SIGSEGV/silent-non-refusal pair reproduced and reverted. RAN (this commit): make clean && make portable -> clean rebuild, zero warnings. make test-c -> full C suite green, exit 0, includes the new tests/test_qt_addrow binary. make test-python -> 153 tests, 10 skipped, 0 failures, exit 0. make glm METAL=1 -> clean rebuild, zero warnings. make metal-test under COLI_METAL_RESSET=0 AND =1: both full suite green, exit 0. One-shot -Wno-unused-function-stripped build: same 8 pre-existing, unrelated orphans as before this round, no new ones.
FIX ROUND 2, item 3 (clean-room conformance trial finding, spec I6 -- loud failure, every refusal names its condition): a real (non-dry-run) run of tools/repack_fp8_passthrough.py whose --indir contains FP8 tensors but none matching a repack-target kind used to exit 0 having emitted nothing -- trial-verified with a non-resident-named FP8 tensor selecting silently. That is a silent trap: an empty "container" (just the resume/params sidecars, no actual .safetensors output) nobody asked for, with no caller-side check positioned to catch it. main() now refuses (nonzero exit, stderr naming --indir and the shard count checked) when every shard -- this run's AND any prior resumed run's, via the `done` progress dict, one entry per shard -- has NEVER produced an output. --dry-run is deliberately NOT covered: printing "0 tensor(s) selected" already IS the loud, honest answer dry-run exists to give, not a silent no-op, so turning it into a refusal would be wrong, not thorough (this judgment call implemented directly, per the coordinator's own "or escalate the disagreement" allowance -- no disagreement here, both readings agree once dry-run's actual purpose is considered). tests/test_fp8_repack.py: new ZeroTargetRefusalTest -- a fixture shard with FP8 tensors but none matching a repack-target kind (routed expert + f32 norm, mirroring the trial's own scenario) must refuse on a real run and still exit 0 on --dry-run. Also updates the module's kv_b_proj-exclusion comment (repack tool docstring), which the JustVugg#528+FIX-ROUND-2 qt_addrow/qt_matvec_rows fix made stale: it claimed an fmt=7 kv_b_proj tensor "would silently misread fp8 bytes as int2-packed data" -- true only for fmt=6; fmt=7 actually SIGSEGV'd (t->q4 is NULL for fmt=7). Both now refuse loudly at the absorb call site instead -- corrected to describe the actual (fixed) behavior, while keeping the underlying exclusion decision itself unchanged (refusing loudly there is still not the same as supporting fmt=7 for kv_b_proj). PROVEN TO BITE (RAN): removed the new zero-target check only, ran the new test -- `test_zero_targets_refuses_loudly` failed exactly as expected (returncode 0 instead of nonzero); `test_zero_targets_dry_run_unaffected` still passed (confirming the dry-run path was never touched by the mutation). Restored (diffed back to byte-identical with the pre-mutation file), reran: both new tests green. RAN (this commit): python3 -m unittest tests.test_fp8_repack -v -> 14/14 pass. make clean && make portable -> clean rebuild, zero warnings. make test-c -> full C suite green, exit 0. make test-python -> 157 tests, 10 skipped, 0 failures, exit 0 (2 more than before this round: the new ZeroTargetRefusalTest cases).
A single read() on the capture pipe can return short on Linux (glibc's unbuffered stderr arrives in multiple chunks), truncating the refusal message before its marker -- the linux CI job caught expect_refuse's [64,98] fmt=6/fmt=7 collision case failing exactly this way while the refusal itself fired correctly. Loop the read to EOF; same fix in test_qt_addrow.c's identical helper. macOS coalesced the chunks, which is why local runs never saw it.
Stacked on fmt7/fp8-passthrough (PR 1). Ports fmt6/fp8-passthrough-r2's metadata-stamp feature (originally ff4b403, folded here against this branch's fmt=7 renumbering and already-reconciled qt_resolve_fmt) as the first commit of PR 2 (registry + metadata stamp), split out per the maintainer's JustVugg#524 scoping. This commit adds the stamp infrastructure and a POST-HOC verify step; it does NOT yet let a stamp resolve a genuine byte-collision inside qt_resolve_fmt itself -- that is the next commit, mirroring the source branch's own bisectable two-commit shape (stamp scaffolding, then the resolve-power threaded in). st.h: new FORMAT METADATA STAMP fields on `shards` (fmt_name/fmt_val/fmt_n/ fmt_cap) + st_fmt_stamp_ingest (parses each shard's __metadata__["colibri.fmt"] -- itself JSON text, a {tensor_name: format_name} map, since safetensors __metadata__ values are always plain strings) + st_fmt_stamp (lookup). Wired into st_init_multi's real header-parse loop. Absent __metadata__, or absent colibri.fmt key, is NOT an error (unstamped container, byte-arithmetic decides as before); a colibri.fmt key present but malformed IS refused (same untrusted-container discipline as the rest of the loader). Unchanged from the source commit -- this file never touched fmt numbering. colibri.c: FMT_NAMES table (name string <-> internal fmt int, renumbered fp8-e4m3-b128 -> 7) + qt_fmt_by_name/qt_name_by_fmt + qt_verify_fmt_stamp (TRUST-VERIFY-REFUSE: agreeing stamp is a silent no-op; disagreeing or unrecognized-name stamp refuses loudly; absent stamp is a no-op, zero behavior change for unstamped containers). Table includes a placeholder "e8-iq3-lattice" entry for dev's own fmt=6 (dev has no stamp feature of its own). Called from qt_from_disk right after qt_resolve_fmt -- deliberately NOT threaded into the routed-expert loader paths (repack_fp8_passthrough.py never stamps routed experts). tools/repack_fp8_passthrough.py: writer side. FORMAT_NAME="fp8-e4m3-b128" / METADATA_KEY="colibri.fmt" constants; repack_shard() now also returns fmt_map ({weight_name: FORMAT_NAME}, keyed by weight only, never the .qs sidecar); main() JSON-encodes it (sort_keys, deterministic) into the output shard's __metadata__ via safetensors.save_file(metadata=...). tests/test_fp8_repack.py: two new cases (fmt_map covers exactly the selected weight names; end-to-end __metadata__.colibri.fmt present and correct through the real CLI) plus the repack_shard() call sites updated for its new 3-tuple return. tests/test_fp8_e2e_repack_load.py and tests/test_fp8_e2e_loader.c: the stamp-agreement checks this branch's earlier commit (PR 1) deliberately omitted -- since PR 1's tool never wrote a stamp, there was nothing to check -- are restored now that the tool stamps again. Does NOT yet include: qt_resolve_fmt's stamped_name parameter (next commit), test_fp8_load.c's Part D stamp-outcome suite (next commit, since it needs stamped containers exercised through the full qt_from_disk path -- this commit's plumbing is sufficient for that, but it lands with the parameter-threading commit to keep the two reviewable as one coherent "stamp becomes load-bearing" change, matching the source branch's own split). RAN (this commit): make clean && make glm METAL=1 -- clean rebuild, zero warnings. make test-c -> full C suite green, exit 0 (test_fp8_load unaffected -- no stamped fixtures in its current suite, byte-arithmetic inference alone still decides every case). make metal-test under COLI_METAL_RESSET=0: full suite green, exit 0, all fp8 GPU lines ok (unaffected -- Metal-side code never touches the stamp). python3 -m unittest tests.test_fp8_repack -v -> 11/11 pass. python3 -m unittest tests.test_fp8_e2e_repack_load -v -> 1/1 pass (the real repack tool's stamp checked end-to-end against the real C loader, which now verifies it transparently via qt_verify_fmt_stamp). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Stacked on the previous commit. Ports fmt6/fp8-passthrough-r2's qt_resolve_fmt stamp-threading (originally fb3b45d), rebuilt against this branch's already-reconciled qt_resolve_fmt (both THE DESIGN LANDMINE and the SECOND DESIGN LANDMINE, plus the UE8M0 scale-encoding seam added on fmt7/fp8-passthrough) rather than cherry-picked -- the source commit predates all three of those. Closes the gap the previous commit deliberately left open: qt_verify_fmt_stamp only ran AFTER qt_resolve_fmt, so a genuinely ambiguous tensor was refused unconditionally regardless of any stamp. colibri.c: qt_resolve_fmt gains a `stamped_name` parameter (NULL for every routed-expert caller -- repack_fp8_passthrough.py never stamps routed experts). Both collision points now consult it: - THE DESIGN LANDMINE (fmt=1 vs fmt=7-f32): a stamp naming exactly one of "int8-row"/"fp8-e4m3-b128" resolves directly; anything else falls through to the existing refusal. - SECOND DESIGN LANDMINE (fmt=6 vs fmt=7 at I=98): a stamp naming "e8-iq3-lattice"/"int8-row"/"fp8-e4m3-b128" resolves the matching candidate; anything else refuses as before. Neither collision point lets a stamp grant a decoder this build doesn't have: a "fp8-e4m3-b128" stamp on a tensor whose scale sidecar is UE8M0-shaped (not f32-shaped) still refuses in both places -- the stamp confirms the WEIGHT format, not the scale ENCODING, and this build only implements f32 block scales. This is new relative to the source project's original stamp-resolve commit, which predates the UE8M0 seam entirely; it's the direct consequence of keeping "declared property, not guaranteed decodable" honest under a stamp. qt_verify_fmt_stamp's signature is unchanged (name, stamped, fmt) -- qt_from_disk now looks the stamp up via st_fmt_stamp ONCE and passes it to BOTH qt_resolve_fmt (to resolve) and qt_verify_fmt_stamp (to re-confirm, trivially, in the resolved case). The three routed-expert call sites (2x expert_load_impl, uring_finalize_load) gain the new trailing NULL argument with a one-line comment explaining why. tests/test_int3.c (pre-existing, predates the fp8 work entirely) calls qt_resolve_fmt directly in 5 places and would not have compiled against the new 7-argument signature -- fixed by threading stamped_name=NULL through all 5 calls (this suite is orthogonal to the fp8 metadata-stamp feature; none of its calls touch the collisions the stamp parameter exists for). tests/test_fp8_load.c: expect_fmt/expect_refuse split into stamped variants (expect_fmt_stamped/expect_refuse_stamped, taking the new stamped_name parameter) with the original two names becoming thin NULL-stamped wrappers -- every existing call site is therefore unchanged in behavior. test_fmt6_fp8_collision (Part A2) and test_ue8m0_scale_refusal (Part A3) both gain stamped cases: successful resolution where the stamp names a real, decodable candidate; STILL refuses where the only stamped candidate is the ue8m0-scaled one (no decoder); an "unrelated stamp doesn't resolve" case. New Part D (test_stamp_agreeing/mismatching/unrecognized_name/absent) covers qt_verify_fmt_stamp's four outcomes on an otherwise-unambiguous tensor, fork+waitpid for the two refusing cases. RAN (this commit): make clean && make glm METAL=1 -- clean rebuild, zero warnings. ./tests/test_fp8_load -> "fp8 loader-seam tests: ok" (Parts A through D, including every new stamped/collision/ue8m0 case). make test-c -> full C suite green, exit 0 (test_int3/test_int3_load unaffected by the signature change). make test-python -> 154 tests, 10 skipped, 0 failures, exit 0. make metal-test under COLI_METAL_RESSET=0 AND =1: both full suite green, exit 0, all fp8 GPU lines ok, RESSET=1 confirmed active. This completes Branch B / PR 2's colibri.c-side content. docs/FORMATS.md (the registry document) is a separate, following commit. FIX ROUND (Windows CI, rebased onto the fixed fmt7/fp8-passthrough): this commit both renames expect_refuse -> expect_refuse_stamped AND introduces a second fork-based helper, expect_stamp_refuse (Part D) -- both need the same #ifndef _WIN32 gate the previous branch's fix applied to expect_refuse's original body. Gated both, same pattern: POSIX arm unchanged (fork/pipe/waitpid, exit(1) capture), Windows arm prints "skipped on Windows (no fork): <tag>" per case and returns pass-through. colibri.c and test_int3.c carried forward byte-identical to the pre-fix f400fa4 (this cherry-pick's only real conflict was the expect_refuse -> expect_refuse_stamped rename colliding textually with the sibling branch's #ifndef insertion at the same line; resolved by hand, diff-verified against both parents). RAN (fix round, this commit): make glm METAL=1 -- clean rebuild, zero warnings. ./tests/test_fp8_load -> "fp8 loader-seam tests: ok", zero "skipped on Windows" lines (POSIX fork path confirmed still exercised on this platform, not silently bypassed). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nvention) Stacked on the previous two commits, closing out Branch B / PR 2's content. Adapts upstream_contribution/FORMATS_registry_draft.md (the Feature Request draft already posted, referenced verbatim on JustVugg#524) into an in-repo reference doc: renumbered fmt=100 -> fmt=7 throughout, line citations re-verified against this branch's actual current colibri.c/quant.h (substantially rewritten from the draft's source tree by the PR-1/PR-2 restructuring), and three content updates specific to this PR pair: - fmt=7's table row status is "this PR pair" (was "proposed"), since the ordinal is no longer private and the implementation is the thing under review. - A new "Scale encoding is a declared property (fmt=7)" section documents both values explicitly: f32 (implemented, what Z.ai's GLM-5.2-FP8 checkpoints ship and what this PR pair's CPU/Metal/repack code reads and writes) and UE8M0 (recognized by its distinct byte signature, refused by name, not implemented -- the DeepSeek-V4 finding the maintainer surfaced on JustVugg#524, credited here). Documents the exact refusal message, the collision check against fmt=1's own ns arithmetic, and that a stamp naming "fp8-e4m3-b128" cannot grant this build a decoder it doesn't have. - "Open questions for maintainer review" gains a second resolved question (scale-encoding-as-declared-property, decided on JustVugg#524) alongside the already-resolved JustVugg#465 sequencing question; the still-open questions (100+ convention acceptability, colibri.fmt key naming) carry forward unchanged. The PRIVATE ORDINAL BLOCK convention section is kept intact (per the task's explicit instruction), updated only to cite fmt=7 as the convention's own worked example of a private ordinal graduating to a public one. "How to add a format" gains one bullet: a format's Feature Request should declare which scale/secondary-array ENCODING(s) it implements, not just its byte layout, matching the precedent this PR pair sets. Read-only source: upstream_contribution/FORMATS_registry_draft.md (untracked, main checkout only) -- consulted for content, not modified; this commit's docs/FORMATS.md is a new, independently-committed file. No code changes in this commit; make glm METAL=1 / make test-c re-run as a sanity check (unaffected by a docs-only change) -- clean rebuild, zero warnings, full C suite green, exit 0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tVugg#528 restack Maintainer review, JustVugg#529 (shared with JustVugg#528's fix): FORMATS.md cites specific c/colibri.c line numbers and branch/commit SHAs for every format row and for the metadata-stamp mechanism. Restacking fmt7/stamp-registry onto the revised fmt7/fp8-passthrough-rev1 (the JustVugg#528 INVERSION/qt_wire_split/stale-comment commits) shifted every colibri.c line number after qt_bytes(): qt_scale_bytes 216->223, qt_alloc 893->910, qt_resolve_fmt (and its SECOND DESIGN LANDMINE comment) 1144->1161, the FMT_NAMES table 1104->1121. Verified against the actual restacked file (grep for each cited function's definition line), not recomputed by offset arithmetic. The "Known formats" intro and "Sources for all rows" preamble also named the pre-restack branch/commit (fmt7/stamp-registry @ base 9a1690e) -- updated to fmt7/stamp-registry-rev1 @ base 696ce96, restacked on fmt7/fp8-passthrough-rev1 @ a77421d, with a note that these line numbers need re-verifying again if the branch is rebased further. No code changes, no test changes -- doc-only.
…qs scope Maintainer review, JustVugg#529 (four non-fatal notes): 1. Non-retroactivity: FORMATS.md gains an explicit section stating the stamp is a forward convention only -- it describes containers a stamping tool writes going forward, never retroactively. Every pre-convention container (including every container this repo's own tooling has ever produced) is simply unstamped, and byte-arithmetic inference alone decides for it, exactly as before this feature existed. The registry is not a migration path: there is no in-place upgrade for an existing container, only re-producing it through a stamping tool. 2. Stamp-map scan bound (orchestrator decision: CAP, not hash): st.h's st_fmt_stamp_ingest now caps the number of __metadata__["colibri.fmt"] entries it will ingest across a container's shards at ST_FMT_STAMP_MAX (4096) and refuses (exit(1)) past it, naming the bound in the error. Stamps are a resident-tensor convention -- a handful to a few hundred entries per model -- never the tens of thousands of routed-expert tensors a large MoE checkpoint carries; a container exceeding the cap is malformed by construction. tests/test_fp8_load.c tests both sides of the boundary: exactly ST_FMT_STAMP_MAX entries loads fine (no off-by-one false refusal), one more refuses (fork+waitpid, matching this suite's existing exit(1)-testing convention). 3. Discovery-time abort surface: st_fmt_stamp_ingest's exit(1) calls (this commit's cap included) fire from st_init_multi's shard-header-parse loop -- container DISCOVERY time, before any tensor is resolved against the model or a single weight byte is read. Documented at the call site (st.h) and in FORMATS.md: this is coarser-grained and earlier than qt_resolve_fmt/qt_verify_fmt_stamp's own per-tensor refusals, and the two are distinguishable at a glance -- this surface's messages name a shard FILE, the per-tensor surface names a TENSOR. 4. Non-.qs stamps (orchestrator decision: scope the claim, don't widen verification): confirmed at the source that qt_from_disk only ever calls st_fmt_stamp inside its `.qs`-sidecar branch -- a colibri.fmt entry naming any other tensor (raw f32/bf16, norm, router, embed/ lm_head) is ingested and stored like any other entry but never looked up, so it has no effect. FORMATS.md and st_fmt_stamp's own comment now state this scope explicitly rather than leaving it implicit. RAN (this commit): make clean && make portable -> clean rebuild, zero warnings. make test-c -> full C suite green, exit 0 (includes the two new stamp-map-cap cases). make test-python -> 155 tests, 10 skipped, 0 failures, exit 0. make glm METAL=1 -> clean rebuild, zero warnings. make metal-test under COLI_METAL_RESSET=0 AND =1: both full suite green, exit 0.
…k tool)
FIX ROUND 2, item 2 (clean-room conformance trial finding): docs/FORMATS.md
described the PRE-inversion design in two places quoted directly from the
file -- (a) the PR-pair intro paragraph ("refusing UNCONDITIONALLY at every
ambiguous shape, since it carries no stamp... adds the stamp... that lets
those same collisions resolve instead of refuse") and (b) the metadata-stamp
section ("a stamp can also resolve a genuine byte-count collision... instead
of the collision refusing unconditionally"). Both contradict the code and
the document's own provenance note, which already cites the INVERSION
elsewhere. Rewrote both to the post-inversion truth: an unstamped ambiguous
shape resolves to the incumbent (int8-row) at the reader; the writer refuses
to ever emit unstamped ambiguity; a stamp's role for THAT collision is
letting a genuinely-stamped fmt=7 tensor still be read as fmt=7 (overriding
the unstamped default), not resolving a refusal that no longer happens for
the unstamped case. The OTHER collision (fmt=6-vs-fmt=7, SECOND DESIGN
LANDMINE) is unchanged by the inversion and still refuses unconditionally
unstamped -- kept accurate, not conflated with the first.
Found (while fixing the above, same error class, not a new idea) a third
instance in the "Sources for all rows" section describing `stamped_name`
generically as resolving both collisions "instead of refusing
unconditionally" -- corrected for the same reason, so the document doesn't
contradict itself internally after the main fix. Found a FOURTH instance
while touching tools/repack_fp8_passthrough.py for item 3 (separate commit):
that tool's own METADATA STAMP docstring section had the identical stale
claim -- corrected there too.
docs/FORMATS.md's pre-existing authorship banner and every commit's own
trailers are untouched, per the standing rule from the prior fix round.
No code changes, no test changes -- doc/docstring-only.
Same Linux pipe short-read fix as expect_refuse's (see the commit below this stack): the Part D stamp refusal tests use the identical single-read capture and would truncate long refusal messages the same way.
fe9ac93 to
24de8b1
Compare
|
Authored by Fable 5 in Claude Code, analysis in partnership with @monotophic Restacked on #528's rev3: head |
Authored by Fable 5 in Claude Code (We/our), analysis in partnership with @monotophic (me/my).
Revised per review (2026-07-23): restacked on #528's inverted collision policy — see the comment of this date for the revision + evidence. Where this description says PR 1 "refuses unconditionally" at ambiguous shapes: that predates the inversion. Current behavior: unstamped fmt=1-vs-fmt=7 ambiguity resolves to the incumbent (fmt=1); the stamp lets a stamped fmt=7 at such a shape be read as fmt=7. The fmt=6-vs-fmt=7 (I=98) collision still refuses unstamped, and a stamp there still resolves it.
fmt=7 registry + metadata stamp (TRUST-VERIFY-REFUSE)
Branch:
fmt7/stamp-registry, stacked onfmt7/fp8-passthrough(PR 1 — CPUpath, repack tool, Metal kernel, collision/refusal logic; merge that one
first). This PR is the convention discussion the maintainer asked to keep
separate from the implementation: a self-describing container stamp, and the
in-repo format registry it's documented against. Nothing in PR 1's behavior
changes if this PR is never merged — PR 1 stands on its own: unstamped fmt=1-vs-fmt=7 ambiguity resolves to the incumbent, and the narrower fmt=6-vs-fmt=7 collision refuses. This PR's stamp confirms identity where arithmetic already decides, upgrades an ambiguous-shape fmt=7 from incumbent-resolution to its true format, and turns the remaining refusal into a resolution — it never turns a resolution into a refusal, and it never grants a decoder PR 1 doesn't have.
Why a stamp, and why now
QT.fmtis a plainintwith no enum declaration and no in-repo list ofwhat's taken — the process gap that let #465 (E8/IQ3) and this project's own
FP8 proposal both mint
fmt=6independently, in the same week, with neitherauthor wrong.
docs/FORMATS.md(this PR) is the lightweight fix for theordinal half of that gap: an in-repo table of what's assigned. The stamp is
the fix for the other half — a container's format identity shouldn't have
to depend on getting the number right by social coordination alone, and PR
1's own collision analysis shows byte arithmetic genuinely can't always
decide identity on its own (three structural ambiguities — one of them,
fmt=1-vs-fmt=7, at a shape GLM-5.2 actually ships: o_proj [6144,16384],
the very case the #528 review surfaced; the other two at shapes no real GLM
tensor has.). A NAME-based stamp is the thing that can actually check a NAME
against the bytes.
TRUST-VERIFY-REFUSE
Writer (
tools/repack_fp8_passthrough.py): every tensor it repacksinto the fp8-e4m3-b128 container gets an entry in the output shard's
safetensors
__metadata__["colibri.fmt"]— a JSON-encoded map of exacttensor name → format NAME string (
"fp8-e4m3-b128", never the internalfmt=7ordinal, which stays a colibri.c-only enum value the container neverdepends on). The stamp names the WEIGHT format only, deliberately — not a
scale encoding, since fmt=7 can legitimately carry more than one (see PR 1's
"scale encoding is a declared property"; more below).
Reader (
qt_verify_fmt_stamp+qt_resolve_fmt's newstamped_nameparameter,
colibri.c):it would unstamped.
refuse loudly (tensor name + both identities printed,
exit(1)) — same"untrusted container" discipline
qt_resolve_fmtalready applieseverywhere else.
decides, exactly as before this feature existed — every container that
predates this PR, or comes from a tool that doesn't stamp, is unaffected.
points — fmt=1-vs-fmt=7 (THE DESIGN LANDMINE) and fmt=6-vs-fmt=7 at
I=98(SECOND DESIGN LANDMINE) — a stamp naming exactly one live,decodable candidate resolves it directly instead of refusing. This is
the one case where the stamp becomes load-bearing rather than a
post-hoc cross-check: bytes alone are fundamentally ambiguous at these
shapes, so the stamp is the only thing that CAN decide.
What the stamp cannot do: grant a decoder this build doesn't have. A
tensor whose scale sidecar is UE8M0-shaped still refuses even when
correctly stamped
"fp8-e4m3-b128"— in the clean, non-colliding case (PR1's straightforward "recognized but not implemented" refusal) and in both
places a stamp could otherwise resolve a collision (the small-
Ofmt=1-vs-UE8M0 corner, and the
O∈(384,512], I=98fmt=6-vs-UE8M0 corner).The stamp confirms the WEIGHT format; the scale ENCODING is a separate,
declared property PR 1 only implements one value of.
qt_resolve_fmt's owncomments carry the exact case-by-case rule.
The registry:
docs/FORMATS.mdAdapted from the Feature Request draft already referenced on #524
(
upstream_contribution/FORMATS_registry_draft.mdon our side) into anin-repo reference document:
scale-array layout, status, and the commit/PR it landed in.
the implementation is what's under review, not a proposal anymore.
documenting both values explicitly: f32 (implemented) and UE8M0
(recognized, refused by name, not implemented — the DeepSeek-V4 finding
credited to the maintainer's own Feature request: fp8-e4m3-b128 passthrough weight format — zero loss at int8 cost (repack, not requantize) #524 comment).
in-flight branches) is kept intact, updated to cite fmt=7 itself as the
convention's own worked example: minted
fmt=6→ collided with engine: fmt=6 E8/IQ3 lattice container — CPU decode kernel and dispatch (#452 step 3) #465 →re-minted
fmt=100→ graduated to the maintainer-assignedfmt=7, apure find-and-replace at every step, zero on-disk impact.
declare which scale/secondary-array encoding(s) it implements, not
just its byte layout — the precedent this PR sets.
100+theright private-block convention (or would a different reserved range be
preferred)? Should
colibri.fmtbecome the project's standard stamp key,or is a different shape preferred? Two questions from earlier drafts are
now resolved-by-events and marked as such (the engine: fmt=6 E8/IQ3 lattice container — CPU decode kernel and dispatch (#452 step 3) #465 sequencing question;
the scale-encoding-as-property question, decided by the maintainer on
Feature request: fp8-e4m3-b128 passthrough weight format — zero loss at int8 cost (repack, not requantize) #524).
Testing
tests/test_fp8_load.cPart D (four stamp outcomes: agreeing, mismatching,unrecognized-name, absent — fork+waitpid for the two refusing cases) plus
stamped variants added to Part A2 (both collision directions resolve to the
stamped format; an unrelated stamp does NOT resolve; the UE8M0-scaled
corners of both collisions stay refused even when stamped
"fp8-e4m3-b128") and Part A3 (the clean UE8M0 case stays refused whenstamped; the small-
OUE8M0-vs-fmt=1 collision DOES resolve when stamped"int8-row", since that candidate is real and decodable).tests/test_fp8_repack.pygains two cases (the stamp's tensor-name setmatches selection exactly; the real CLI's output carries a correct,
parseable stamp).
tests/test_fp8_e2e_repack_load.py(inherited from PR 1)now also asserts the real tool's stamp end-to-end against the real C loader.
make glm METAL=1(clean rebuild): zero warnings, at both commits inthis PR individually.
make test-c: full C suite green, exit 0, at both commits — includingthe expanded
test_fp8_load(Parts A–D) andtest_int3(its 5qt_resolve_fmtcall sites updated for the newstamped_nameparameter,
NULLthroughout — this suite predates and is orthogonal tothe stamp feature).
make test-python: 154 tests, 10 skipped, 0 failures, exit 0.make metal-testunderCOLI_METAL_RESSET=0AND=1: both fullgreen, exit 0 — unaffected by this PR (the stamp is a CPU-side,
pre-Metal-dispatch concern;
qt_from_diskresolvesfmtbefore anyMetal call, so the GPU kernel never sees a stamp either way).
Durable vs. current-state (review map)
colibri.fmtas the stamp's__metadata__key, JSON-map-of-name as its shapedocs/FORMATS.md's own "Open questions" section100+PRIVATE ORDINAL BLOCK conventionfmt_extfield)Commits in this PR
fp8: self-describing __metadata__ stamp, trust-verify-refuse reader—st.h's stamp ingest/lookup,colibri.c's FMT_NAMES table andqt_verify_fmt_stamp(post-hoc verify only, not yet resolving), therepack tool's writer side, the two new
test_fp8_repack.pycases.fp8: qt_resolve_fmt lets a metadata stamp resolve byte collisions—threads
stamped_nameintoqt_resolve_fmtitself, making bothcollision points stamp-resolvable (except the UE8M0 corners); updates
the three routed-expert call sites and
test_int3.c's five directcalls for the new signature;
test_fp8_load.cParts A2/A3 gainstamped cases, new Part D.
docs: add FORMATS.md, the quantized-format registry (fmt=6/fmt=7 + convention)— this PR's own documentation, no code changes.