feat(qtip): UQFF geometry discriminator — K is an explicit wire field - #170
Conversation
Code Metrics Report━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ Language Files Lines Code Comments Blanks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ C Header 5 305 210 52 43 CSS 2 1181 1036 34 111 CUDA 81 29475 20161 6275 3039 Dockerfile 1 39 22 8 9 JavaScript 16 3546 2676 482 388 Jinja2 7 694 656 5 33 JSON 75 4896 4893 0 3 Makefile 1 6 5 0 1 Metal Shading Lan| 33 12224 9431 1142 1651 PowerShell 1 300 227 30 43 Python 147 15371 12677 824 1870 Shell 42 10304 6858 2769 677 Plain Text 4 3801 0 2479 1322 TOML 33 1498 1294 54 150 YAML 3 25 23 2 0 ───────────────────────────────────────────────────────────────────────────────── HTML 4 2687 2604 43 40 |- CSS 2 543 479 37 27 |- JavaScript 1 1233 1215 12 6 (Total) 4463 4298 92 73 ───────────────────────────────────────────────────────────────────────────────── Jupyter Notebooks 4 122 83 23 16 |- Markdown 1 60 30 22 8 |- Python 1 122 113 1 8 (Total) 304 226 46 32 ───────────────────────────────────────────────────────────────────────────────── Markdown 213 48932 0 38081 10851 |- BASH 72 1655 1203 331 121 |- C 4 19 19 0 0 |- CUDA 2 84 56 16 12 |- JSON 19 779 779 0 0 |- PowerShell 1 1 1 0 0 |- Python 23 1008 787 113 108 |- Rust 68 2063 1727 78 258 |- TOML 6 207 164 0 43 |- YAML 5 41 36 5 0 (Total) 54789 4772 38624 11393 ───────────────────────────────────────────────────────────────────────────────── Rust 681 340988 293252 18096 29640 |- Markdown 504 32562 471 28063 4028 (Total) 373550 293723 46159 33668 ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ Total 1353 516771 363188 99077 54506 ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ |
b2e7fe5 to
e260a09
Compare
heydryft
changed the title
e260a09 to
268ad8b
Compare
268ad8b to
634262e
Compare
|
CI fixed, but deliberately NOT merging: this changes a published on-disk format. What I fixed. The CUDA lane was red with Worth noting the design caught its own bug: Why I am not merging it. Format and ABI changes deserve the most suspicion — the byte formats are the moat and artifacts are already published. This is a wire-format change, so it should land on a deliberate decision, not on a queue-clearing pass. For whoever does land it, the compatibility matrix checks out on inspection:
Blast radius otherwise: no default flipped, no new kernel dispatched, candle untouched. It does not add a third meaning to the Leaving open, green, and ready. #172 stacks on this and is also CI-fixed. |
Retargeted at the integration branchBase changed: The queue is being restructured to the shape the owner asked for: one PR open against Two things had to land on
This PR was not closed and is not considered stale. An audit of the queue found the overwhelming majority of it to be real work that was never merged, not noise. What you need to do: rebase onto 📚 Stack order — this is the BOTTOMThis PR and its dependent were both retargeted at Merge this PR FIRST. Its dependent carries changes that assume this one is already in the branch, and merging them out of order will produce conflicts or silently drop this PR's changes. |
… themselves
Stage 2 of the K=8/V=4/L=12 rung: an artifact can now say which trellis
geometry it was baked at, the loader reads it, and a build that cannot decode
it refuses instead of guessing.
WHY THIS NEEDS A DISCRIMINATOR AT ALL
Both geometries are 2 bits per weight (`bpw = K/V`), so a K=8/V=4 row and a
K=4/V=2 row of the same `in_features` occupy the SAME number of packed bytes.
A mislabelled artifact therefore indexes in bounds at every symbol and returns
plausible garbage rather than faulting. The table is the only tensor that
differs — `[4096, 4]` BF16 vs `[65536, 2]` F32 — which is why `validate_shapes`
checks the table's dtype AND its length, and why both checks are load-bearing
rather than belt-and-braces.
WIRE FORMAT
A trailing section `[tag=3, K, L, V]`, written ONLY for a non-default geometry
and written BEFORE the codebook section. Both of those are deliberate:
* Default writes nothing, so every artifact Arc has already produced stays
byte-identical and no checksum moves. Pinned as an exact suffix, not as a
vague "unchanged": serializing the same layer with the tag flipped appends
exactly [3, 8, 12, 4] and nothing else.
* BEFORE the codebook, because a build that predates this field parses the
trailing region by handing its first byte to `QtipCodebook::from_wire`,
which refuses every tag it does not know. Tag 3 first means an old Arc
fails closed. Tag 3 last would let it consume the codebook section, stop,
and decode K=8/V=4 symbols as K=4/V=2 without faulting.
The trailing region is now a section loop; a repeated tag is refused rather
than letting the last one win.
`QtipGeometry` has no per-site default: adding the field broke all 11
construction sites at compile time and each one now states its geometry, the
same discipline `search` and `search_detail` already follow. `from_stacked_parts`
takes it explicitly and validates it. `stack_experts` and the 3-D quantize path
refuse mixed-geometry stacks — only one table survives a stack, so at most one
expert could decode.
The computed `sum2` codebook is refused at V=4 on both the write and the read
side: it produces a PAIR of values per state and has no V=4 form.
Guards were mutation-tested (F1-F9). Three passed on broken code and are now
covered: `from_stacked_parts` validated nothing, the table-dtype half of the
discriminator was dead (the element-count check masked it), and
`stack_experts`' mixed-geometry refusal was untested. `assert_tensor_bits_eq`
grew a BF16 arm — it panicked on the new table rather than comparing it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SpVNMpb13HkUXqSqbN1o9H
Stage 2 made a K=8/V=4/L=12 artifact loadable. On its own that is a half-applied change: every decoder behind `forward`, `gather_forward`, `dequantize_weights`, `dequantize_expert` and `qtip_packed` unpacks nibbles and indexes a [2^16, 2] table, and handed a K=8 row none of them would fault — at 2 bits per weight the packed byte count is identical and every index lands in bounds. They would serve garbage. So each entry point now states the geometries it implements. `qtip_packed` returns None rather than a `QtipPackedView`, which carries no geometry field and would therefore be read as K=4/V=2 nibbles by any consumer. The refusal names the entry point, and the test asserts that name. An entry point that merely inherits a callee's guard is not guarded: mutation G1 removed `forward`'s own check and every test stayed green, because the error still arrived from `dequantize_weights` further down. All six mutations (G1-G6) are red now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SpVNMpb13HkUXqSqbN1o9H
Follows the padding decision one branch down. `QtipGeometry::packed_len` now means the allocated stride (what a tensor is shaped at and what this format validates) and `data_bytes` is the bit-rate-governed part, both delegating to `trellis_v4l12::Rung` so there is still one implementation of each. Three tests changed premise rather than value and were rewritten, not patched: the bit-rate ratio claim moved to `data_bytes` where it is exact, and the stride got its own bounds (never smaller than the data, never more than 4 bytes larger). A K=9 row of `in_features=36` is 11 data bytes in a 12-byte stride. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SpVNMpb13HkUXqSqbN1o9H
`quantize_with_options_cuda` builds a QtipLayer directly and was the one initializer the geometry discriminator missed. It is behind `#[cfg(feature = "cuda")]`, so the default Check jobs compiled fine and only the CUDA lane went red (E0063 at qtip/mod.rs:2318). Set the same K4V2L16 tag its CPU sibling sets — the CUDA bake kernels run the identical K=4/V=2/L=16 trellis, so a GPU-baked artifact must carry the identical tag or it would deserialize as a different geometry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
217a259 to
8e81323
Compare
1 participant
Stage 2 of 3. Stacked on #168 — merge that first (D20). Base is
feat/qtip-k8v4l12, notmaster.An artifact declares its trellis geometry, the loader reads it, and a build that cannot decode it refuses instead of guessing — at load and at every decode entry point.
K is data on the wire, which is why the K=8 → K=9 pivot is nearly free
The section is
[tag=3, K, L, V].Kwas already an explicit field, so moving from the K=8 control to the K=9 quality winner (+0.00402 Δw_cos, 5× better than the shipped control) is one byte of artifact change given identical tensors — pinned byfamily_members_differ_by_one_wire_byte.QtipGeometry::TrellisV4L12 { rung }carries K as data rather than as a variant, matching the wire.Why a discriminator is needed at all
K=4/V=2 and K=8/V=4 are both 2 bits per weight, so their rows are byte-for-byte the same size. A mislabelled artifact indexes in bounds at every symbol and returns plausible garbage rather than faulting. The table is the only tensor that differs —
[4096, 4]BF16 vs[65536, 2]F32 — sovalidate_shapeschecks the table's dtype and its length, and both are load-bearing. (A[4096, 4]F32 table has the right count and the wrong type; it would be 64 KiB, blowing the shared-memory budget the family exists for.)Note this is no longer true across the whole enum: K=9/V=4 is 2.25 bpw, so its rows are 12.5% larger. Code that assumed "all QTIP geometries are 2 bpw" — true when the family was K=8-only — is now wrong, and
packed_lenis the generalceil(n·K/8).Wire format
Written only for a non-default geometry and before the codebook section. Both deliberate:
QtipCodebook::from_wire, which refuses unknown tags. Tag 3 first ⇒ an old Arc fails closed. Tag 3 last ⇒ it consumes the codebook section, stops, and decodes K=8/V=4 as K=4/V=2 without faulting.The trailing region is a section loop; a repeated tag is refused. A well-formed but unsupported triple gets a diagnosis quoting
K=…/L=…/V=….Loadable is not servable — second commit
forward,gather_forward,dequantize_weights,dequantize_expertandqtip_packedall sit on K=4-only decoders. Each now states the geometries it implements and refuses the rest.qtip_packedreturnsNone, becauseQtipPackedViewhas no geometry field and any consumer would read the bytes as nibbles.No per-site default
Adding the field broke all 11 construction sites at compile time and each now states its geometry — the discipline
searchandsearch_detailalready follow.from_stacked_partstakes it explicitly and validates.stack_expertsand the 3-D quantize path refuse mixed-geometry stacks: only one table survives a stack.The computed
sum2codebook is refused at any V≠2 on both write and read — it produces a pair of values per state and has no V=4 form.Guards shown red (F1–F9, G1–G6, W1–W6)
Five passed on broken code and are now covered:
from_stacked_partsvalidated nothing.stack_experts' mixed-geometry refusal was untested.forward's own guard could be deleted with every test green, because the error still arrived from a callee. An entry point that inherits a callee's guard is not guarded; the test now asserts which one refused.packed_len's ceiling was unguarded in practice. Every plausiblein_featuresis a multiple of 32, which makesnum_symbols·Ka multiple of 8 and floor equal to ceil at K=9 — so a floored formula passed every realistic fixture. Fixed twice over: the formula now delegates totrellis_v4l12::Rung(one implementation, already exercised at non-byte-aligned symbol counts) and a test pins the ceiling at deliberately unrealistic widths.assert_tensor_bits_eqalso grew a BF16 arm; it panicked on the new table rather than comparing it.Scoped clippy clean;
cargo test -p mistralrs-quant334 + 5 passing;mistralrs-coreandarc-enginecompile.What remains