From d76e90d1234ceb99d6b68dfbe318a38ea27ce60a Mon Sep 17 00:00:00 2001 From: Noah Gift Date: Sun, 20 Sep 2026 18:46:37 +0200 Subject: [PATCH 1/4] PMAT-3430 Phase 1: characterize all 9 boundaries BEFORE touching them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #3430 Q1-c claims Phases 3-4 are behaviour-preserving: no crate accepts an id or a name it refused before. This makes that a measurement instead of a promise. 12 tests, all written against UNCHANGED code and all green on it now. They must pass UNCHANGED after the re-exports and admission functions land, with ONE edit named in advance (below). serve, 7 boundaries — the id sweep is 0..=255 at every one, because the property is about what is REFUSED as much as what is accepted and a sweep over the admitted set alone cannot see a widening: gguf/dtype.rs apr_qtype_to_dtype (+ its refusal names the id), apr_dtype_to_byte infer/mod.rs qtype_to_dtype_str apr/mod.rs the dtype_byte arm of TensorEntry::from_binary — exercised through the REAL parser with a minimal entry, not a re-typed copy of the expression, which would pass while the real path changed apr/special_tokens MappedAprModel::dtype_to_qtype apr/dequant dtype_to_ggml_qtype, which deliberately admits LESS than the enum knows (F32/F16/BF16 -> None) compute, 1 boundary — from_u32 over 0..=255, plus all 15 rows of block geometry and the partial-block rounding, since compute is the only one of the three enums that carried sizes and the extraction agreed with it exactly core, 0 boundaries — measured, not assumed: nothing in core turns an integer or a string into a GgmlType. Its whole exposure is ONE exhaustive match, GgufTensor::byte_size, snapshotted here at 24 (dtype, elements) points Each lives in a #[cfg(test)] child module of the boundary's own module, so the private items are reachable without widening anything; the only non-test edits in this commit are four `mod` lines. THE ONE ROW EXPECTED TO MOVE, named now so the diff cannot hide it: core sizes Q4_1 at 18 bytes/block where upstream says 20 AND core's own shape.rs already says 20. A second test asserts that disagreement DIRECTLY, so it cannot be fixed by accident; Phase 3 deletes that test and moves one row in the case table, 54 -> 60, in the same commit. Refs #3430 Co-Authored-By: Claude Opus 5 (1M context) --- crates/aprender-compute/src/inference/gguf.rs | 7 + .../inference/gguf_characterization_tests.rs | 83 +++++++++++ .../gguf/byte_size_characterization_tests.rs | 90 ++++++++++++ crates/aprender-core/src/format/gguf/types.rs | 6 + .../apr/boundary_characterization_tests.rs | 133 ++++++++++++++++++ crates/aprender-serve/src/apr/mod.rs | 7 + crates/aprender-serve/src/gguf/dtype.rs | 7 + .../src/gguf/dtype_characterization_tests.rs | 98 +++++++++++++ 8 files changed, 431 insertions(+) create mode 100644 crates/aprender-compute/src/inference/gguf_characterization_tests.rs create mode 100644 crates/aprender-core/src/format/gguf/byte_size_characterization_tests.rs create mode 100644 crates/aprender-serve/src/apr/boundary_characterization_tests.rs create mode 100644 crates/aprender-serve/src/gguf/dtype_characterization_tests.rs diff --git a/crates/aprender-compute/src/inference/gguf.rs b/crates/aprender-compute/src/inference/gguf.rs index d3812fd118..a9894fae27 100644 --- a/crates/aprender-compute/src/inference/gguf.rs +++ b/crates/aprender-compute/src/inference/gguf.rs @@ -437,3 +437,10 @@ mod tests { assert_eq!(file.tensors.len(), 0); } } + +// PMAT-3430 Q1-c: the characterization snapshot for compute's one id boundary +// and its block geometry. A child module, so it reaches the private `from_u32` +// without widening it. This `mod` line is the only non-test edit Phase 1 makes. +#[cfg(test)] +#[path = "gguf_characterization_tests.rs"] +mod gguf_characterization_tests; diff --git a/crates/aprender-compute/src/inference/gguf_characterization_tests.rs b/crates/aprender-compute/src/inference/gguf_characterization_tests.rs new file mode 100644 index 0000000000..bc2fbba538 --- /dev/null +++ b/crates/aprender-compute/src/inference/gguf_characterization_tests.rs @@ -0,0 +1,83 @@ +//! CHARACTERIZATION — compute's one id boundary, snapshotted before PMAT-3430 +//! Phases 3-4 re-export `trueno_quant::GgmlType` here. +//! +//! Written against UNCHANGED code; must pass UNCHANGED afterwards, with ONE +//! permitted edit named in advance by #3430 Q1-c: `GgmlType::from_u32(x)` +//! becomes the free function `from_u32(x)`, because an inherent method cannot +//! survive on a re-exported foreign type. The admitted SET may not change. +//! +//! Sizes are characterized here too. compute is the only one of the three +//! enums that carried block geometry, and it is the one the upstream extraction +//! agreed with exactly — so these 15 rows are the evidence that delegating to +//! `TRAITS` is value-identical rather than merely plausible. + +use super::GgmlType; + +/// The 15 ids compute admitted before M1, with their block geometry. +/// (id, blck_size, type_size) +const ADMITTED_TODAY: [(u32, usize, usize); 15] = [ + (0, 1, 4), + (1, 1, 2), + (2, 32, 18), + (3, 32, 20), + (6, 32, 22), + (7, 32, 24), + (8, 32, 34), + (9, 32, 36), + (10, 256, 84), + (11, 256, 110), + (12, 256, 144), + (13, 256, 176), + (14, 256, 210), + (15, 256, 292), + (30, 1, 2), +]; + +fn admitted(id: u32) -> Option<(usize, usize)> { + ADMITTED_TODAY + .iter() + .find(|(candidate, _, _)| *candidate == id) + .map(|(_, blck, size)| (*blck, *size)) +} + +#[test] +fn from_u32_admits_exactly_fifteen_ids_over_the_whole_byte_range() { + for id in 0u32..=255 { + let got = GgmlType::from_u32(id); + assert_eq!( + got.is_some(), + admitted(id).is_some(), + "qtype {id}: compute's admitted set changed" + ); + if let Some(t) = got { + assert_eq!(t as u32, id, "qtype {id} no longer round-trips through the enum"); + } + } +} + +#[test] +fn block_geometry_is_unchanged_for_every_admitted_id() { + for (id, blck, size) in ADMITTED_TODAY { + let t = GgmlType::from_u32(id).expect("an admitted id must construct"); + assert_eq!(t.block_size(), blck, "qtype {id} block_size changed"); + assert_eq!(t.block_bytes(), size, "qtype {id} block_bytes changed"); + } +} + +#[test] +fn tensor_bytes_rounds_partial_blocks_up_exactly_as_it_did() { + for (id, blck, size) in ADMITTED_TODAY { + let t = GgmlType::from_u32(id).expect("an admitted id must construct"); + assert_eq!(t.tensor_bytes(0), 0, "qtype {id} at 0 elements"); + assert_eq!(t.tensor_bytes(blck), size, "qtype {id} at one full block"); + assert_eq!(t.tensor_bytes(blck * 4), size * 4, "qtype {id} at four blocks"); + if blck > 1 { + assert_eq!(t.tensor_bytes(1), size, "qtype {id} rounds 1 element up to a block"); + assert_eq!( + t.tensor_bytes(blck + 1), + size * 2, + "qtype {id} rounds a partial second block up" + ); + } + } +} diff --git a/crates/aprender-core/src/format/gguf/byte_size_characterization_tests.rs b/crates/aprender-core/src/format/gguf/byte_size_characterization_tests.rs new file mode 100644 index 0000000000..c4b6b2ce61 --- /dev/null +++ b/crates/aprender-core/src/format/gguf/byte_size_characterization_tests.rs @@ -0,0 +1,90 @@ +//! CHARACTERIZATION — `GgufTensor::byte_size` for all 12 of core's variants, +//! snapshotted before PMAT-3430 Phase 3 replaces its hand-written match with +//! delegation to `trueno_quant`'s upstream-extracted table. +//! +//! core has NO id or name boundary — nothing here turns an integer or a string +//! into a `GgmlType`; the enum is only ever built from literals. So core's +//! entire exposure to M1 is this one exhaustive match, which stops compiling +//! (E0004) the moment the enum gains variants, and must be replaced by +//! delegation rather than patched with a bare `_ =>` (#3430 Q1-c). +//! +//! EXACTLY ONE ROW IS EXPECTED TO CHANGE, and it is named here in advance: +//! `Q4_1` is sized at **18** bytes per 32-element block by the hand-written arm +//! (`Q4_0 | Q4_1 => …`), where upstream ggml says **20** (2 x f16 scale+min, +//! plus 16 nibble bytes) and core's OWN other size table, `format/gguf/shape.rs`, +//! already says 20. Two tables in one crate disagreeing. When delegation lands, +//! the `Q4_1` expectation below moves 54 -> 60 and that one-line diff IS the +//! record of the behaviour change. Every other row must be untouched. + +use super::{GgmlType, GgufTensor}; + +fn tensor(dtype: GgmlType, elements: u64) -> GgufTensor { + GgufTensor { + name: "t".to_string(), + shape: vec![elements], + dtype, + data: vec![], + } +} + +/// (dtype, elements, bytes) for every variant core carries, at a block-aligned +/// count and a deliberately non-aligned one. +#[test] +fn byte_size_is_unchanged_for_all_twelve_variants() { + let cases: [(GgmlType, u64, usize); 24] = [ + (GgmlType::F32, 64, 256), + (GgmlType::F32, 65, 260), + (GgmlType::F16, 64, 128), + (GgmlType::F16, 65, 130), + // Q4_0: 2 (scale) + 16 (nibbles) = 18 bytes per 32 elements. Correct. + (GgmlType::Q4_0, 64, 36), + (GgmlType::Q4_0, 65, 54), + // Q4_1: THE ROW THAT MOVES. 18 today, 20 upstream — see the module doc. + (GgmlType::Q4_1, 64, 36), + (GgmlType::Q4_1, 96, 54), + (GgmlType::Q8_0, 64, 68), + (GgmlType::Q8_0, 65, 102), + (GgmlType::Q4K, 512, 288), + (GgmlType::Q4K, 513, 432), + (GgmlType::Q6K, 256, 210), + (GgmlType::Q6K, 257, 420), + (GgmlType::I8, 64, 64), + (GgmlType::I8, 65, 65), + (GgmlType::I16, 64, 128), + (GgmlType::I16, 65, 130), + (GgmlType::I32, 64, 256), + (GgmlType::I32, 65, 260), + (GgmlType::I64, 64, 512), + (GgmlType::I64, 65, 520), + (GgmlType::F64, 64, 512), + (GgmlType::F64, 65, 520), + ]; + for (dtype, elements, expected) in cases { + assert_eq!( + tensor(dtype, elements).byte_size(), + expected, + "{dtype:?} at {elements} elements" + ); + } +} + +/// The disagreement, asserted directly so it cannot be fixed by accident and +/// go unnoticed. When Phase 3 lands, this test is DELETED in the same commit +/// that moves the row above — its whole purpose is to name a defect that +/// exists today. +#[test] +fn q4_1_currently_disagrees_with_cores_own_other_size_table() { + // 96 elements = 3 blocks. The hand-written arm: 3 * 18 = 54. + // shape.rs and upstream ggml: 3 * 20 = 60. + assert_eq!( + tensor(GgmlType::Q4_1, 96).byte_size(), + 54, + "Q4_1 is no longer sized at 18 bytes/block here — if this is the Phase 3 \ + delegation, delete this test and move the row in the case table above" + ); + assert_ne!( + tensor(GgmlType::Q4_1, 96).byte_size(), + 60, + "Q4_1 now agrees with upstream; this defect is fixed and the test is obsolete" + ); +} diff --git a/crates/aprender-core/src/format/gguf/types.rs b/crates/aprender-core/src/format/gguf/types.rs index 709c67c1d0..87485c0418 100644 --- a/crates/aprender-core/src/format/gguf/types.rs +++ b/crates/aprender-core/src/format/gguf/types.rs @@ -477,3 +477,9 @@ pub fn export_tensors_to_gguf( #[cfg(test)] #[path = "types_tests.rs"] mod tests; + +// PMAT-3430 Q1-c: the characterization snapshot for core's ONE exhaustive +// match over GgmlType. This `mod` line is the only non-test edit Phase 1 makes. +#[cfg(test)] +#[path = "byte_size_characterization_tests.rs"] +mod byte_size_characterization_tests; diff --git a/crates/aprender-serve/src/apr/boundary_characterization_tests.rs b/crates/aprender-serve/src/apr/boundary_characterization_tests.rs new file mode 100644 index 0000000000..991edd9466 --- /dev/null +++ b/crates/aprender-serve/src/apr/boundary_characterization_tests.rs @@ -0,0 +1,133 @@ +//! CHARACTERIZATION — the four remaining serve boundaries that turn an id or a +//! name into a quant type, snapshotted before PMAT-3430 Phases 3-4 move them +//! onto `trueno_quant::GgmlType`. +//! +//! Written against UNCHANGED code; must pass UNCHANGED afterwards. Together +//! with `gguf/dtype_characterization_tests.rs` this covers all seven boundaries +//! #3430 Q1-c enumerates, which is the only reviewable evidence that the +//! admitted sets were REPRODUCED rather than chosen. +//! +//! `apr/mod.rs`'s boundary is not a function — it is an arm inside +//! `TensorEntry::from_binary`'s `match dtype_byte`. It is therefore exercised +//! through the real parser with a minimal entry, not through a copy of the +//! expression: a test of a re-typed copy would pass while the real path changed. + +use super::{MappedAprModel, TensorEntry}; +use crate::apr::dequant::dtype_to_ggml_qtype; +use crate::infer::qtype_to_dtype_str; + +/// The 16 (id, name) pairs serve admitted before M1. +const ADMITTED_TODAY: [(u32, &str); 16] = [ + (0, "F32"), + (1, "F16"), + (2, "Q4_0"), + (3, "Q4_1"), + (6, "Q5_0"), + (7, "Q5_1"), + (8, "Q8_0"), + (9, "Q8_1"), + (10, "Q2_K"), + (11, "Q3_K"), + (12, "Q4_K"), + (13, "Q5_K"), + (14, "Q6_K"), + (16, "IQ2_XXS"), + (17, "IQ2_XS"), + (30, "BF16"), +]; + +fn admitted_name(id: u32) -> Option<&'static str> { + ADMITTED_TODAY + .iter() + .find(|(candidate, _)| *candidate == id) + .map(|(_, name)| *name) +} + +/// A minimal v2 tensor entry: name "t", the dtype byte under test, 0 dims, +/// offset 0, size 0. Shaped from `TensorEntry::from_binary`'s own doc comment. +fn entry_with_dtype(dtype_byte: u8) -> Vec { + let mut v = Vec::new(); + v.extend_from_slice(&1u16.to_le_bytes()); // name_len + v.push(b't'); // name + v.push(dtype_byte); // dtype + v.push(0u8); // ndim + v.extend_from_slice(&0u64.to_le_bytes()); // offset + v.extend_from_slice(&0u64.to_le_bytes()); // size + v +} + +#[test] +fn infer_qtype_to_dtype_str_names_exactly_the_admitted_ids() { + for id in 0u32..=255 { + let got = qtype_to_dtype_str(id); + match admitted_name(id) { + Some(name) => assert_eq!(got, name, "qtype {id} renamed"), + None => assert_eq!(got, "Unknown", "qtype {id} is newly named {got}"), + } + } +} + +#[test] +fn apr_tensor_entry_dtype_byte_mapping_is_unchanged_over_the_whole_byte_range() { + for byte in 0u8..=255 { + let (entry, _) = TensorEntry::from_binary(&entry_with_dtype(byte)) + .unwrap_or_else(|e| panic!("dtype byte {byte} no longer parses: {e}")); + let expected: String = match byte { + // APR-native ids, which are NOT ggml and must stay that way (GH-438). + 128 => "q4".to_string(), + 129 => "q8".to_string(), + 8 => "q4".to_string(), + 9 => "q8".to_string(), + other => admitted_name(u32::from(other)).unwrap_or("F32").to_string(), + }; + assert_eq!( + entry.dtype, expected, + "APR dtype byte {byte} changed meaning" + ); + } +} + +#[test] +fn special_tokens_dtype_to_qtype_maps_every_admitted_name_and_zero_otherwise() { + for (id, name) in ADMITTED_TODAY { + assert_eq!(MappedAprModel::dtype_to_qtype(name), id, "{name} moved"); + assert_eq!( + MappedAprModel::dtype_to_qtype(&name.to_ascii_lowercase()), + id, + "{name} moved in lower case" + ); + } + // Unknown names collapse to 0 (= F32). Lossy, and characterized as such. + for name in [ + "", "nonsense", "Q9_9", "q4_K", "IQ3_S", "TQ1_0", "MXFP4", "NVFP4", + ] { + assert_eq!( + MappedAprModel::dtype_to_qtype(name), + 0, + "dtype {name:?} is newly recognised" + ); + } +} + +#[test] +fn dequant_dtype_to_ggml_qtype_admits_only_the_quantized_subset() { + // This boundary deliberately admits LESS than the enum knows: F32/F16/BF16 + // are not quantized and APR-native q4/q8 are a different layout, so both + // are None. That narrowing is the property under test. + for (id, name) in ADMITTED_TODAY { + let got = dtype_to_ggml_qtype(name); + let is_quantized_here = !matches!(name, "F32" | "F16" | "BF16"); + if is_quantized_here { + assert_eq!(got, Some(id), "{name} is no longer dequantizable"); + } else { + assert_eq!(got, None, "{name} is newly treated as quantized"); + } + } + for name in ["q4", "q8", "", "nonsense", "IQ3_S", "TQ1_0", "MXFP4"] { + assert_eq!( + dtype_to_ggml_qtype(name), + None, + "dtype {name:?} is newly dequantizable" + ); + } +} diff --git a/crates/aprender-serve/src/apr/mod.rs b/crates/aprender-serve/src/apr/mod.rs index 9dd3f43cd7..69c16643b4 100644 --- a/crates/aprender-serve/src/apr/mod.rs +++ b/crates/aprender-serve/src/apr/mod.rs @@ -407,3 +407,10 @@ impl TensorEntry { include!("metadata.rs"); include!("tokenizer_loading.rs"); include!("special_tokens.rs"); + +// PMAT-3430 Q1-c: the characterization snapshot for the four remaining +// id-or-name admission boundaries (infer, this module's `from_binary` dtype +// arm, special_tokens, dequant). This `mod` line is the only non-test edit. +#[cfg(test)] +#[path = "boundary_characterization_tests.rs"] +mod boundary_characterization_tests; diff --git a/crates/aprender-serve/src/gguf/dtype.rs b/crates/aprender-serve/src/gguf/dtype.rs index d8788190d4..5e380ff842 100644 --- a/crates/aprender-serve/src/gguf/dtype.rs +++ b/crates/aprender-serve/src/gguf/dtype.rs @@ -526,3 +526,10 @@ impl OwnedQuantizedModel { include!("embedding.rs"); include!("loader_apr_quantized.rs"); + +// PMAT-3430 Q1-c: the characterization snapshot for this module's two admission +// boundaries. A child module, so it reaches the private fns without widening +// anything. This `mod` line is the only non-test edit Phase 1 makes here. +#[cfg(test)] +#[path = "dtype_characterization_tests.rs"] +mod dtype_characterization_tests; diff --git a/crates/aprender-serve/src/gguf/dtype_characterization_tests.rs b/crates/aprender-serve/src/gguf/dtype_characterization_tests.rs new file mode 100644 index 0000000000..7474967346 --- /dev/null +++ b/crates/aprender-serve/src/gguf/dtype_characterization_tests.rs @@ -0,0 +1,98 @@ +//! CHARACTERIZATION — what these two boundaries accept TODAY, before PMAT-3430 +//! Phases 3-4 move them onto `trueno_quant::GgmlType`. +//! +//! These tests are written against UNCHANGED code and must pass UNCHANGED after +//! the re-export and the admission functions land. That is the whole mechanism +//! behind #3430 Q1-c's claim that "no crate accepts an id or a name it refused +//! before": not a promise in a PR body, a snapshot that fails if it stops being +//! true. +//! +//! They live in a `#[cfg(test)]` child module of the boundary's own module +//! because both functions are PRIVATE, and a child module sees its ancestors' +//! private items. No visibility is widened to test them. +//! +//! The id sweep is 0..=255 and not "the interesting ids", because the property +//! is about what is REFUSED as much as what is accepted, and a sweep that only +//! visits the accepted set cannot see a widening. + +use super::{apr_dtype_to_byte, apr_qtype_to_dtype}; + +/// The 16 ids serve admitted before M1, with the string each produced. +const ADMITTED_TODAY: [(u32, &str); 16] = [ + (0, "F32"), + (1, "F16"), + (2, "Q4_0"), + (3, "Q4_1"), + (6, "Q5_0"), + (7, "Q5_1"), + (8, "Q8_0"), + (9, "Q8_1"), + (10, "Q2_K"), + (11, "Q3_K"), + (12, "Q4_K"), + (13, "Q5_K"), + (14, "Q6_K"), + (16, "IQ2_XXS"), + (17, "IQ2_XS"), + (30, "BF16"), +]; + +fn admitted(id: u32) -> Option<&'static str> { + ADMITTED_TODAY + .iter() + .find(|(candidate, _)| *candidate == id) + .map(|(_, name)| *name) +} + +#[test] +fn apr_qtype_to_dtype_accepts_exactly_sixteen_ids_over_the_whole_byte_range() { + for id in 0u32..=255 { + let got = apr_qtype_to_dtype(id).ok(); + assert_eq!( + got, + admitted(id), + "qtype {id}: admission changed (this boundary writes APR dtype strings; \ + accepting an id it used to refuse means emitting quantized bytes a reader \ + will decode as something else)" + ); + } +} + +#[test] +fn apr_qtype_to_dtype_refuses_with_a_message_that_names_the_id() { + // The refusal is the point of this boundary (see its doc comment), so the + // shape of the error is characterized too, not just that it is an error. + for id in [4u32, 5, 15, 18, 29, 31, 42, 43, 200, 255] { + let err = apr_qtype_to_dtype(id).expect_err("must refuse an unadmitted id"); + let text = format!("{err}"); + assert!( + text.contains(&id.to_string()), + "the refusal for qtype {id} does not name it: {text}" + ); + } +} + +#[test] +fn apr_dtype_to_byte_maps_every_admitted_name_and_falls_back_to_zero() { + for (id, name) in ADMITTED_TODAY { + assert_eq!( + u32::from(apr_dtype_to_byte(name)), + id, + "dtype {name} no longer maps to byte {id}" + ); + assert_eq!( + u32::from(apr_dtype_to_byte(&name.to_ascii_lowercase())), + id, + "dtype {name} no longer maps from its lower-case spelling" + ); + } + // Unknown names warn and write F32 (0). That is a lossy fallback this + // change must not silently alter. + for name in ["", "nonsense", "Q9_9", "q4_K", "Q4_k", "IQ3_S", "TQ1_0", "MXFP4"] { + assert_eq!( + apr_dtype_to_byte(name), + 0, + "dtype {name:?} is newly recognised by apr_dtype_to_byte" + ); + } +} From 6897df473d7a53560b380fdc0b2b5e2cc3f55db9 Mon Sep 17 00:00:00 2001 From: Noah Gift Date: Sun, 20 Sep 2026 19:05:32 +0200 Subject: [PATCH 2/4] PMAT-3430 Phases 3-4: three ggml enums become one, and the count is now checkable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ticket's headline. `crates/aprender-serve` (16 ids), `crates/aprender-core` (12) and `crates/aprender-compute` (15) each declared their own ggml tensor-type enum; a fourth private table, QTYPE_LABELS (13 rows, #3405), named the same ids again. All four are gone. The tree now holds ONE definition: $ grep -rnE '^\s*pub enum (GgmlType|GgmlQuantType)\b' crates/*/src crates/aprender-quant/src/ggml_type.rs:140:pub enum GgmlType { BEHAVIOUR IS PRESERVED, AND THAT IS MEASURED, NOT ASSERTED. Phase 1's 12 characterization tests were written against the unchanged code and sweep every boundary over ids 0..=255. All of them pass UNCHANGED through this commit, with exactly the two edits named in advance: - compute: `GgmlType::from_u32(x)` -> `from_u32(x)`, because an inherent method cannot follow a type defined in another crate. The admitted SET is untouched — still exactly 15 ids, now stated once in `const ADMITTED`. - core: the Q4_1 row, below. Each crate keeps its own admitted set, stated once and enforced at every boundary, because the leaf knows all 35 live ggml types and no crate may start accepting an id it used to refuse: compute ADMITTED[15] + `from_u32`, at its one GGUF parse boundary serve ADMITTED[16] + `admitted_from_id` / `admitted_from_name`, at all SEVEN boundaries (gguf/dtype x2, infer, apr/mod's from_binary arm, apr/special_tokens, apr/dequant, and the loader test that mirrors the predicate) core none, and none is needed — measured: nothing in core turns an integer or a string into a GgmlType THE ONE VALUE THAT CHANGES, AND THE COUNT THE COLLAPSE IS WORTH. core sized Q4_1 at 18 bytes per block where ggml says 20 (2 x f16 for scale AND min, plus 16 nibble bytes) and where core's OWN other size table, format/gguf/shape.rs, already said 20. Two tables in one crate disagreeing, and the wrong one was the one nothing called — `export_tensors_to_gguf` sizes tensors with data.len(), so no GGUF was ever written wrong by it. A unit test had asserted the 18-byte value since it was written, pinning the defect. So the honest ledger for this collapse: four tables -> one, ONE wrong value removed (Q4_1) and one omission (QTYPE_LABELS had no row for Q8_1, so a refusal printed `qtype 9`). The larger find is not in this diff: measuring against the extracted table is what exposed `ggml_dtype_element_size`, a FIFTH id-indexed table, wrong in 10 of 31 rows including BF16 at 0.375 B/elem instead of 2.0 — that is #3583, deliberately not folded in here. QTYPE_LABELS's replacement goes through `from_id`, never `TRAITS[id]`: it takes an arbitrary u32 from a file and a raw index would panic for any id >= 43. It is also deliberately NOT filtered by the admitted set — it is a diagnostic, and naming id 21 in a refusal beats printing `qtype 21`. GUARD: scripts/check_one_ggml_type_enum.sh, green on this tree, with a 15-row case table (a re-export, a commented-out definition, GgmlTypeError, GgufValue, QuantType and a private enum all correctly NOT matched) and a zero-match arm that FAILS rather than passing vacuously. Mutation: re-adding a second definition to compute turns it RED naming both files; restoring turns it GREEN. guard_tree discovers it automatically via `git ls-files 'scripts/check_*.sh'`, so committing it is what wires it. Verified: 30,168 tests across serve+core+quant (nextest, ci profile) and 3,514 in compute, all passing; `cargo check --workspace --all-targets` clean; clippy -D warnings clean on all four crates; fmt clean. Refs #3430, #3421, #3583 Co-Authored-By: Claude Opus 5 (1M context) --- crates/aprender-compute/src/inference/gguf.rs | 129 +++++------- .../inference/gguf_characterization_tests.rs | 8 +- .../aprender-compute/src/inference/model.rs | 6 +- .../gguf/byte_size_characterization_tests.rs | 29 +-- crates/aprender-core/src/format/gguf/types.rs | 75 ++----- .../src/format/gguf/types_tests.rs | 8 +- crates/aprender-serve/src/apr/dequant.rs | 2 +- crates/aprender-serve/src/apr/mod.rs | 2 +- .../aprender-serve/src/apr/special_tokens.rs | 2 +- crates/aprender-serve/src/gguf/dtype.rs | 4 +- .../aprender-serve/src/gguf/qwen3_moe_load.rs | 47 ++--- .../src/gguf/tests/loader_tests_apr.rs | 5 +- crates/aprender-serve/src/gguf/types.rs | 197 ++++++------------ crates/aprender-serve/src/infer/mod.rs | 2 +- scripts/check_one_ggml_type_enum.sh | 111 ++++++++++ 15 files changed, 296 insertions(+), 331 deletions(-) create mode 100755 scripts/check_one_ggml_type_enum.sh diff --git a/crates/aprender-compute/src/inference/gguf.rs b/crates/aprender-compute/src/inference/gguf.rs index a9894fae27..1519b486d1 100644 --- a/crates/aprender-compute/src/inference/gguf.rs +++ b/crates/aprender-compute/src/inference/gguf.rs @@ -21,84 +21,59 @@ use crate::error::TruenoError; const GGUF_MAGIC: u32 = 0x4655_4747; // "GGUF" in little-endian -/// GGML tensor type IDs (subset used for LLM inference). -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -#[repr(u32)] -pub enum GgmlType { - F32 = 0, - F16 = 1, - Q4_0 = 2, - Q4_1 = 3, - Q5_0 = 6, - Q5_1 = 7, - Q8_0 = 8, - Q8_1 = 9, - Q2K = 10, - Q3K = 11, - Q4K = 12, - Q5K = 13, - Q6K = 14, - Q8K = 15, - Bf16 = 30, -} - -impl GgmlType { - fn from_u32(v: u32) -> Option { - match v { - 0 => Some(Self::F32), - 1 => Some(Self::F16), - 2 => Some(Self::Q4_0), - 3 => Some(Self::Q4_1), - 6 => Some(Self::Q5_0), - 7 => Some(Self::Q5_1), - 8 => Some(Self::Q8_0), - 9 => Some(Self::Q8_1), - 10 => Some(Self::Q2K), - 11 => Some(Self::Q3K), - 12 => Some(Self::Q4K), - 13 => Some(Self::Q5K), - 14 => Some(Self::Q6K), - 15 => Some(Self::Q8K), - 30 => Some(Self::Bf16), - _ => None, - } - } - - /// Bytes per block for this quantization type. - pub fn block_bytes(&self) -> usize { - match self { - Self::F32 => 4, - Self::F16 | Self::Bf16 => 2, - Self::Q4_0 => 18, // 32 weights: 2 (scale) + 16 (4-bit) - Self::Q4_1 => 20, // 32 weights: 2+2 (scale+min) + 16 - Self::Q5_0 => 22, // 32 weights - Self::Q5_1 => 24, - Self::Q8_0 => 34, // 32 weights: 2 (scale) + 32 (8-bit) - Self::Q8_1 => 36, - Self::Q2K => 84, // 256 weights - Self::Q3K => 110, - Self::Q4K => 144, // 256 weights - Self::Q5K => 176, - Self::Q6K => 210, - Self::Q8K => 292, +// PMAT-3430 M1 Phase 3: the enum that used to be declared here is now the +// workspace's ONE ggml tensor-type enum, re-exported from the leaf crate. +// +// compute's copy listed 15 ids and carried the block geometry for them. Every +// one of those 15 rows agreed with upstream ggml exactly — it was the only one +// of the three in-tree enums that did — so nothing about sizes changes here; +// `block_bytes`, `block_size` and `tensor_bytes` now read the same numbers out +// of `TRAITS`, which is extracted from ggml at the pinned commit rather than +// typed. `crates/aprender-compute/src/inference/gguf_characterization_tests.rs` +// asserts all 15 rows and the rounding are unchanged. +pub use trueno_quant::GgmlType; + +/// The ids compute ACCEPTS at its GGUF parse boundary. +/// +/// The leaf knows all 35 live ggml types; this crate knew 15 and must keep +/// knowing exactly 15 (#3430 Q1-c: M1 changes no crate's admitted set, because +/// widening one silently is how a loader starts decoding bytes it has no kernel +/// for). Adding a row here is a deliberate act with its own ticket, not a +/// side effect of the enum growing. +const ADMITTED: [GgmlType; 15] = [ + GgmlType::F32, + GgmlType::F16, + GgmlType::Q4_0, + GgmlType::Q4_1, + GgmlType::Q5_0, + GgmlType::Q5_1, + GgmlType::Q8_0, + GgmlType::Q8_1, + GgmlType::Q2K, + GgmlType::Q3K, + GgmlType::Q4K, + GgmlType::Q5K, + GgmlType::Q6K, + GgmlType::Q8K, + GgmlType::BF16, +]; + +/// The GGUF parse boundary: an id becomes a type only if compute admits it. +/// +/// This was `GgmlType::from_u32`, an inherent method. An inherent method cannot +/// follow a type that is now defined in another crate, so it survives as a free +/// function under the same name — the one call-site change #3430 Q1-c named in +/// advance (`GgmlType::from_u32(x)` -> `from_u32(x)`). +fn from_u32(v: u32) -> Option { + let t = GgmlType::from_id(v)?; + let mut i = 0; + while i < ADMITTED.len() { + if ADMITTED[i] as u32 == t as u32 { + return Some(t); } + i += 1; } - - /// Weights per block. - pub fn block_size(&self) -> usize { - match self { - Self::F32 | Self::F16 | Self::Bf16 => 1, - Self::Q4_0 | Self::Q4_1 | Self::Q5_0 | Self::Q5_1 | Self::Q8_0 | Self::Q8_1 => 32, - Self::Q2K | Self::Q3K | Self::Q4K | Self::Q5K | Self::Q6K | Self::Q8K => 256, - } - } - - /// Total bytes for `n_elements` weights. - pub fn tensor_bytes(&self, n_elements: usize) -> usize { - let bs = self.block_size(); - let n_blocks = (n_elements + bs - 1) / bs; - n_blocks * self.block_bytes() - } + None } /// Info about a single tensor in the GGUF file. @@ -223,7 +198,7 @@ impl GgufFile { dims.push(read_u64(&mut cursor)?); } let dtype_u32 = read_u32(&mut cursor)?; - let dtype = GgmlType::from_u32(dtype_u32).ok_or_else(|| { + let dtype = from_u32(dtype_u32).ok_or_else(|| { TruenoError::InvalidInput(format!( "Unknown GGML type {dtype_u32} for tensor '{name}'" )) diff --git a/crates/aprender-compute/src/inference/gguf_characterization_tests.rs b/crates/aprender-compute/src/inference/gguf_characterization_tests.rs index bc2fbba538..36373b29c3 100644 --- a/crates/aprender-compute/src/inference/gguf_characterization_tests.rs +++ b/crates/aprender-compute/src/inference/gguf_characterization_tests.rs @@ -11,7 +11,7 @@ //! agreed with exactly — so these 15 rows are the evidence that delegating to //! `TRAITS` is value-identical rather than merely plausible. -use super::GgmlType; +use super::{from_u32, GgmlType}; /// The 15 ids compute admitted before M1, with their block geometry. /// (id, blck_size, type_size) @@ -43,7 +43,7 @@ fn admitted(id: u32) -> Option<(usize, usize)> { #[test] fn from_u32_admits_exactly_fifteen_ids_over_the_whole_byte_range() { for id in 0u32..=255 { - let got = GgmlType::from_u32(id); + let got = from_u32(id); assert_eq!( got.is_some(), admitted(id).is_some(), @@ -58,7 +58,7 @@ fn from_u32_admits_exactly_fifteen_ids_over_the_whole_byte_range() { #[test] fn block_geometry_is_unchanged_for_every_admitted_id() { for (id, blck, size) in ADMITTED_TODAY { - let t = GgmlType::from_u32(id).expect("an admitted id must construct"); + let t = from_u32(id).expect("an admitted id must construct"); assert_eq!(t.block_size(), blck, "qtype {id} block_size changed"); assert_eq!(t.block_bytes(), size, "qtype {id} block_bytes changed"); } @@ -67,7 +67,7 @@ fn block_geometry_is_unchanged_for_every_admitted_id() { #[test] fn tensor_bytes_rounds_partial_blocks_up_exactly_as_it_did() { for (id, blck, size) in ADMITTED_TODAY { - let t = GgmlType::from_u32(id).expect("an admitted id must construct"); + let t = from_u32(id).expect("an admitted id must construct"); assert_eq!(t.tensor_bytes(0), 0, "qtype {id} at 0 elements"); assert_eq!(t.tensor_bytes(blck), size, "qtype {id} at one full block"); assert_eq!(t.tensor_bytes(blck * 4), size * 4, "qtype {id} at four blocks"); diff --git a/crates/aprender-compute/src/inference/model.rs b/crates/aprender-compute/src/inference/model.rs index 0656a19bbb..52dc03dbba 100644 --- a/crates/aprender-compute/src/inference/model.rs +++ b/crates/aprender-compute/src/inference/model.rs @@ -541,7 +541,7 @@ fn load_f32_or_dequant_tensor( .ok_or_else(|| TruenoError::InvalidInput(format!("Missing tensor data: {name}")))?; match info.dtype { - GgmlType::F32 | GgmlType::F16 | GgmlType::Bf16 => { + GgmlType::F32 | GgmlType::F16 | GgmlType::BF16 => { Ok(to_f32_from_any(data, info.dtype, expected_elements)) } GgmlType::Q4K => { @@ -606,7 +606,7 @@ fn load_weight_matrix( match info.dtype { GgmlType::Q4K => Ok(WeightMatrix::Q4K { data: data.to_vec(), rows: out_dim }), - GgmlType::F32 | GgmlType::F16 | GgmlType::Bf16 => { + GgmlType::F32 | GgmlType::F16 | GgmlType::BF16 => { let f32_data = to_f32_from_any(data, info.dtype, n_elements); Ok(WeightMatrix::F32 { data: f32_data, rows: out_dim }) } @@ -721,7 +721,7 @@ fn to_f32_from_any(data: &[u8], dtype: GgmlType, n_elements: usize) -> Vec }) .collect() } - GgmlType::Bf16 => { + GgmlType::BF16 => { let count = n_elements.min(data.len() / 2); (0..count) .map(|i| { diff --git a/crates/aprender-core/src/format/gguf/byte_size_characterization_tests.rs b/crates/aprender-core/src/format/gguf/byte_size_characterization_tests.rs index c4b6b2ce61..ef056316b5 100644 --- a/crates/aprender-core/src/format/gguf/byte_size_characterization_tests.rs +++ b/crates/aprender-core/src/format/gguf/byte_size_characterization_tests.rs @@ -39,9 +39,11 @@ fn byte_size_is_unchanged_for_all_twelve_variants() { // Q4_0: 2 (scale) + 16 (nibbles) = 18 bytes per 32 elements. Correct. (GgmlType::Q4_0, 64, 36), (GgmlType::Q4_0, 65, 54), - // Q4_1: THE ROW THAT MOVES. 18 today, 20 upstream — see the module doc. - (GgmlType::Q4_1, 64, 36), - (GgmlType::Q4_1, 96, 54), + // Q4_1: THE ROW THAT MOVED, in Phase 3, and the only one. 2 blocks x 20 + // and 3 blocks x 20, where the hand-written arm gave 18 per block. + // Upstream ggml and core's own shape.rs both say 20. + (GgmlType::Q4_1, 64, 40), + (GgmlType::Q4_1, 96, 60), (GgmlType::Q8_0, 64, 68), (GgmlType::Q8_0, 65, 102), (GgmlType::Q4K, 512, 288), @@ -67,24 +69,3 @@ fn byte_size_is_unchanged_for_all_twelve_variants() { ); } } - -/// The disagreement, asserted directly so it cannot be fixed by accident and -/// go unnoticed. When Phase 3 lands, this test is DELETED in the same commit -/// that moves the row above — its whole purpose is to name a defect that -/// exists today. -#[test] -fn q4_1_currently_disagrees_with_cores_own_other_size_table() { - // 96 elements = 3 blocks. The hand-written arm: 3 * 18 = 54. - // shape.rs and upstream ggml: 3 * 20 = 60. - assert_eq!( - tensor(GgmlType::Q4_1, 96).byte_size(), - 54, - "Q4_1 is no longer sized at 18 bytes/block here — if this is the Phase 3 \ - delegation, delete this test and move the row in the case table above" - ); - assert_ne!( - tensor(GgmlType::Q4_1, 96).byte_size(), - 60, - "Q4_1 now agrees with upstream; this defect is fixed and the test is obsolete" - ); -} diff --git a/crates/aprender-core/src/format/gguf/types.rs b/crates/aprender-core/src/format/gguf/types.rs index 87485c0418..028184aacb 100644 --- a/crates/aprender-core/src/format/gguf/types.rs +++ b/crates/aprender-core/src/format/gguf/types.rs @@ -49,35 +49,15 @@ pub enum GgufValueType { Float64 = 12, } -/// GGUF tensor types (from ggml) -#[repr(u32)] -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum GgmlType { - /// 32-bit float - F32 = 0, - /// 16-bit float - F16 = 1, - /// 4-bit quantization (type 0) - Q4_0 = 2, - /// 4-bit quantization (type 1) - Q4_1 = 3, - /// 8-bit quantization (type 0) - Q8_0 = 8, - /// K-quant 4-bit (Q4_K) - 256-element super-blocks, 144 bytes each - Q4K = 12, - /// K-quant 6-bit (Q6_K) - 256-element super-blocks, 210 bytes each - Q6K = 14, - /// 8-bit signed integer - I8 = 24, - /// 16-bit signed integer - I16 = 25, - /// 32-bit signed integer - I32 = 26, - /// 64-bit signed integer - I64 = 27, - /// 64-bit float - F64 = 28, -} +// PMAT-3430 M1 Phase 3: the enum that used to be declared here is now the +// workspace's ONE ggml tensor-type enum, re-exported from the leaf crate. +// +// core listed 12 ids. It has NO admission function and needs none: measured +// across the crate, nothing in core turns an integer or a string into a +// GgmlType — `format/gguf/reader.rs` keeps `dtype: u32` raw, and every +// construction here is from a literal. So core's entire exposure to this change +// is `GgufTensor::byte_size` below. +pub use trueno_quant::GgmlType; /// GGUF metadata value #[derive(Debug, Clone)] @@ -354,33 +334,22 @@ pub struct GgufTensor { } impl GgufTensor { - /// Calculate the byte size based on dtype and shape + /// Calculate the byte size based on dtype and shape. + /// + /// PMAT-3430: this was a hand-written match over core's 12 variants. It now + /// delegates to the upstream-extracted table, which is value-identical for + /// eleven of the twelve and CORRECTS the twelfth: the old arm sized `Q4_1` + /// at 18 bytes per 32-element block (`Q4_0 | Q4_1 => …`), where ggml says + /// **20** — 2 x f16 for scale and min, plus 16 nibble bytes — and core's own + /// other size table, `format/gguf/shape.rs`, already said 20. Two tables in + /// one crate disagreeing; the one that was wrong is the one nothing called. + /// + /// `tensor_bytes` keeps the rounding-up behaviour the old arm had, so a + /// partial block still costs a whole block. #[must_use] pub fn byte_size(&self) -> usize { let elements: u64 = self.shape.iter().product(); - match self.dtype { - GgmlType::F32 | GgmlType::I32 => elements as usize * 4, - GgmlType::F16 | GgmlType::I16 => elements as usize * 2, - GgmlType::I8 => elements as usize, - GgmlType::Q4_0 | GgmlType::Q4_1 => { - // Block-quantized: 32 elements per block - // Q4_0: 2 bytes scale + 16 bytes data = 18 bytes per 32 elements - (elements as usize).div_ceil(32) * 18 - } - GgmlType::Q8_0 => { - // Q8_0: 2 bytes scale + 32 bytes data = 34 bytes per 32 elements - (elements as usize).div_ceil(32) * 34 - } - GgmlType::Q4K => { - // Q4_K: 256-element super-blocks, 144 bytes each - (elements as usize).div_ceil(256) * 144 - } - GgmlType::Q6K => { - // Q6_K: 256-element super-blocks, 210 bytes each - (elements as usize).div_ceil(256) * 210 - } - GgmlType::F64 | GgmlType::I64 => elements as usize * 8, - } + self.dtype.tensor_bytes(elements as usize) } } diff --git a/crates/aprender-core/src/format/gguf/types_tests.rs b/crates/aprender-core/src/format/gguf/types_tests.rs index ed7ca1a973..b773e473d6 100644 --- a/crates/aprender-core/src/format/gguf/types_tests.rs +++ b/crates/aprender-core/src/format/gguf/types_tests.rs @@ -357,8 +357,12 @@ fn test_gguf_tensor_byte_size_q4_1() { dtype: GgmlType::Q4_1, data: vec![], }; - // (96 + 31) / 32 = 3 blocks * 18 bytes = 54 - assert_eq!(tensor.byte_size(), 54); + // PMAT-3430: 3 blocks * 20 bytes = 60, NOT 54. This test asserted 54 for as + // long as the hand-written arm sized Q4_1 the same as Q4_0, at 18 bytes per + // block. Upstream ggml says 20 — `block_q4_1` is 2 x ggml_half (scale AND + // min) plus QK4_1/2 = 16 nibble bytes — and core's own `format/gguf/shape.rs` + // has always said 20. The 54 was a test pinning a defect. + assert_eq!(tensor.byte_size(), 60); } #[test] diff --git a/crates/aprender-serve/src/apr/dequant.rs b/crates/aprender-serve/src/apr/dequant.rs index f6c87d658c..f19c2357ff 100644 --- a/crates/aprender-serve/src/apr/dequant.rs +++ b/crates/aprender-serve/src/apr/dequant.rs @@ -232,7 +232,7 @@ pub fn dtype_to_ggml_qtype(dtype: &str) -> Option { // GH-321: Use unified GgmlQuantType enum for GGML-compatible formats. // APR-native Q8/Q4 are NOT GGML — different binary layout — returns None. // F32/F16/BF16 are not quantized — returns None. - crate::gguf::GgmlQuantType::from_str_lossy(dtype) + crate::gguf::admitted_from_name(dtype) .filter(|qt| { matches!( qt, diff --git a/crates/aprender-serve/src/apr/mod.rs b/crates/aprender-serve/src/apr/mod.rs index 69c16643b4..7872be65e5 100644 --- a/crates/aprender-serve/src/apr/mod.rs +++ b/crates/aprender-serve/src/apr/mod.rs @@ -321,7 +321,7 @@ impl TensorEntry { 8 => "q4".to_string(), // Legacy AprQ4 (was Q4=8 before GH-438) 9 => "q8".to_string(), // Legacy AprQ8 (was Q8=9 before GH-438) // GGML-standard types - _ => crate::gguf::GgmlQuantType::from_id(u32::from(dtype_byte)) + _ => crate::gguf::admitted_from_id(u32::from(dtype_byte)) .map_or_else( || { eprintln!("WARN: Unknown APR dtype byte {dtype_byte}, treating as F32"); diff --git a/crates/aprender-serve/src/apr/special_tokens.rs b/crates/aprender-serve/src/apr/special_tokens.rs index c783f71fc9..6ae6b9cc21 100644 --- a/crates/aprender-serve/src/apr/special_tokens.rs +++ b/crates/aprender-serve/src/apr/special_tokens.rs @@ -207,7 +207,7 @@ impl MappedAprModel { /// GH-321: Convert APR dtype string to GGML qtype using unified enum. #[must_use] pub fn dtype_to_qtype(dtype: &str) -> u32 { - crate::gguf::GgmlQuantType::from_str_lossy(dtype) + crate::gguf::admitted_from_name(dtype) .map_or(0, crate::gguf::GgmlQuantType::as_id) } } diff --git a/crates/aprender-serve/src/gguf/dtype.rs b/crates/aprender-serve/src/gguf/dtype.rs index 5e380ff842..fb4f41a2d8 100644 --- a/crates/aprender-serve/src/gguf/dtype.rs +++ b/crates/aprender-serve/src/gguf/dtype.rs @@ -13,7 +13,7 @@ /// policy this now follows: an unsupported quant must fail loudly rather than be /// silently decoded as something else. fn apr_qtype_to_dtype(qtype: u32) -> Result<&'static str> { - crate::gguf::GgmlQuantType::from_id(qtype) + crate::gguf::admitted_from_id(qtype) .map(crate::gguf::GgmlQuantType::as_str) .ok_or_else(|| RealizarError::FormatError { reason: format!( @@ -194,7 +194,7 @@ mod hybrid_gpu_unsupported_quant_tests { /// GH-321: Convert APR dtype string to byte using unified enum. /// GH-191 FIX: Use GGML dtype values directly so they match TensorEntry::from_binary reader. fn apr_dtype_to_byte(dtype: &str) -> u8 { - crate::gguf::GgmlQuantType::from_str_lossy(dtype).map_or_else( + crate::gguf::admitted_from_name(dtype).map_or_else( || { eprintln!( "WARN: Unknown dtype '{}' in dtype_to_byte, writing as F32", diff --git a/crates/aprender-serve/src/gguf/qwen3_moe_load.rs b/crates/aprender-serve/src/gguf/qwen3_moe_load.rs index 662f8c8630..0cfb8e5c3d 100644 --- a/crates/aprender-serve/src/gguf/qwen3_moe_load.rs +++ b/crates/aprender-serve/src/gguf/qwen3_moe_load.rs @@ -83,35 +83,26 @@ pub struct Qwen3MoeQuantizedLayer { /// fails if the two drift in either direction. pub(crate) const SUPPORTED_EXPERT_QTYPES: &[u32] = &[GGUF_TYPE_Q4_K, GGUF_TYPE_Q6_K]; -/// `(ggml type id, name)` for the qtypes this crate's GGUF reader knows. -/// -/// Used only to make a refusal readable ("Q4_0 (2)" rather than "2"). -/// A table rather than a `match` so the lookup stays complexity-1. -const QTYPE_LABELS: &[(u32, &str)] = &[ - (0, "F32"), - (1, "F16"), - (2, "Q4_0"), - (3, "Q4_1"), - (6, "Q5_0"), - (7, "Q5_1"), - (8, "Q8_0"), - (10, "Q2_K"), - (11, "Q3_K"), - (12, "Q4_K"), - (13, "Q5_K"), - (14, "Q6_K"), - (30, "BF16"), -]; - -/// Render a qtype id as `NAME (id)`, or bare `qtype N` when unknown. +/// Render a qtype id as `NAME(id)`, or bare `qtype N` when the id is not a +/// live ggml type. +/// +/// PMAT-3430: this used to consult `QTYPE_LABELS`, a private 13-row table of +/// (id, name) — the FOURTH copy in the tree of the same fact, added by #3405 +/// while three enums of it already existed. It now reads the one table. +/// +/// Two consequences, both deliberate: +/// - **it goes through `from_id`, never `TRAITS[id]`**, because this function +/// takes an arbitrary `u32` from a file and a raw index would panic for any +/// id >= 43; +/// - **it is not filtered by serve's admitted set.** This is a DIAGNOSTIC, not +/// an admission boundary: naming ids 9, 15 and 16..29 in a refusal instead of +/// printing `qtype 21` is strictly better for the reader, and the refusal +/// still happens either way. The 13 names it used to produce are unchanged. fn qtype_label(qtype: u32) -> String { - QTYPE_LABELS - .iter() - .find(|(id, _)| *id == qtype) - .map_or_else( - || format!("qtype {qtype}"), - |(_, n)| format!("{n}({qtype})"), - ) + crate::gguf::GgmlQuantType::from_id(qtype).map_or_else( + || format!("qtype {qtype}"), + |t| format!("{}({qtype})", t.as_str()), + ) } /// Render [`SUPPORTED_EXPERT_QTYPES`] for an error message. diff --git a/crates/aprender-serve/src/gguf/tests/loader_tests_apr.rs b/crates/aprender-serve/src/gguf/tests/loader_tests_apr.rs index a58579da87..7b4eee7d6a 100644 --- a/crates/aprender-serve/src/gguf/tests/loader_tests_apr.rs +++ b/crates/aprender-serve/src/gguf/tests/loader_tests_apr.rs @@ -308,7 +308,10 @@ fn test_to_apr_bytes_various_qtypes() { // emitted quantized bytes under that label, so the file was structurally // valid and its weights were garbage. The test asserting is_ok() on an // invalid input is what made that permanent. - if crate::gguf::GgmlQuantType::from_id(qtype).is_some() { + // PMAT-3430: asks the same predicate the writer now asks. The leaf knows + // 35 live ggml types; serve admits 16, and this test is about serve's + // boundary, not about what ggml has ids for. + if crate::gguf::admitted_from_id(qtype).is_some() { assert!( result.is_ok(), "to_apr_bytes should succeed for known qtype {}: {:?}", diff --git a/crates/aprender-serve/src/gguf/types.rs b/crates/aprender-serve/src/gguf/types.rs index 621dfe59bf..172f6909c7 100644 --- a/crates/aprender-serve/src/gguf/types.rs +++ b/crates/aprender-serve/src/gguf/types.rs @@ -81,147 +81,78 @@ pub const APR_TYPE_Q4: u32 = 128; /// GH-478: Same role as `APR_TYPE_Q4` for the 8-bit native format. /// Matches `aprender::format::TensorDType::Q8 = 129` (file-format value). pub const APR_TYPE_Q8: u32 = 129; - -// ============================================================================ -// GH-321: Unified GGML Quantization Type Enum -// ============================================================================ - -/// GGML quantization type — single source of truth for ID<->string mapping. +// PMAT-3430 M1 Phase 4: the enum that used to be declared here is now the +// workspace's ONE ggml tensor-type enum, re-exported from the leaf crate. +// +// serve's copy listed 16 ids and was the only one of the three with string +// conversions and a Display impl. Those move into the leaf with unchanged names +// and unchanged behaviour: `as_str` returns the same 16 GGUF spellings, and +// `from_str_lossy` still accepts exactly the exact spelling and its all-lower +// form (so ggml's own `q4_K` is still refused, as it always was). +// +// The NAME survives as an alias because roughly thirty call sites and several +// public signatures use it, and renaming them is not what M1 is for. +pub use trueno_quant::GgmlType; +/// serve's historical name for the ggml tensor-type enum. +pub use trueno_quant::GgmlType as GgmlQuantType; + +/// The ids serve ACCEPTS at its parse boundaries. /// -/// CRITICAL (GH-438): These IDs MUST match the GGML standard (llama.cpp ggml.h). -/// aprender's `TensorDType` IDs in the 0-31 range must also match. -/// APR-native quantization types (AprQ4=128, AprQ8=129 in aprender) are -/// outside this enum's scope — they are handled by `TensorEntry::from_binary` -/// before this enum is consulted. See LAYOUT-002. +/// The leaf knows all 35 live ggml types; serve knew 16 and must keep knowing +/// exactly 16 (#3430 Q1-c). Every boundary in this crate that turns an id or a +/// name into a type goes through [`admitted_from_id`] or [`admitted_from_name`] +/// rather than through the leaf directly, so the set is stated once and cannot +/// widen by accident when the enum grows. /// -/// Replaces 6+ duplicated match statements across infer/mod.rs, gguf/dtype.rs, -/// apr/mod.rs, apr/dequant.rs, and apr/special_tokens.rs. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -#[repr(u32)] -pub enum GgmlQuantType { - /// 32-bit float (GGML type 0) - F32 = 0, - /// 16-bit float (GGML type 1) - F16 = 1, - /// 4-bit quantization, block size 32 (GGML type 2) - Q4_0 = 2, - /// 4-bit quantization with min, block size 32 (GGML type 3) - Q4_1 = 3, - /// 5-bit quantization, block size 32 (GGML type 6) - Q5_0 = 6, - /// 5-bit quantization with min, block size 32 (GGML type 7) - Q5_1 = 7, - /// 8-bit quantization, block size 32 (GGML type 8) - Q8_0 = 8, - /// 8-bit quantization type 1 (GGML type 9) - Q8_1 = 9, - /// 2-bit K-quantization, super-block size 256 (GGML type 10) - Q2K = 10, - /// 3-bit K-quantization, super-block size 256 (GGML type 11) - Q3K = 11, - /// 4-bit K-quantization, super-block size 256 (GGML type 12) - Q4K = 12, - /// 5-bit K-quantization, super-block size 256 (GGML type 13) - Q5K = 13, - /// 6-bit K-quantization, super-block size 256 (GGML type 14) - Q6K = 14, - /// Importance-weighted 2-bit, extra-extra-small (GGML type 16) - IQ2XXS = 16, - /// Importance-weighted 2-bit, extra-small (GGML type 17) - IQ2XS = 17, - /// BFloat16 (GGML type 30) - BF16 = 30, -} - -impl GgmlQuantType { - /// Convert from u32 GGML type ID. Returns `None` for unknown types. - #[must_use] - pub const fn from_id(id: u32) -> Option { - match id { - 0 => Some(Self::F32), - 1 => Some(Self::F16), - 2 => Some(Self::Q4_0), - 3 => Some(Self::Q4_1), - 6 => Some(Self::Q5_0), - 7 => Some(Self::Q5_1), - 8 => Some(Self::Q8_0), - 9 => Some(Self::Q8_1), - 10 => Some(Self::Q2K), - 11 => Some(Self::Q3K), - 12 => Some(Self::Q4K), - 13 => Some(Self::Q5K), - 14 => Some(Self::Q6K), - 16 => Some(Self::IQ2XXS), - 17 => Some(Self::IQ2XS), - 30 => Some(Self::BF16), - _ => None, - } - } - - /// Convert to u32 GGML type ID. - #[must_use] - pub const fn as_id(self) -> u32 { - self as u32 - } - - /// Convert to u8 for APR binary format. - #[must_use] - pub const fn as_byte(self) -> u8 { - self as u8 - } - - /// Convert to canonical string representation. - #[must_use] - pub const fn as_str(self) -> &'static str { - match self { - Self::F32 => "F32", - Self::F16 => "F16", - Self::Q4_0 => "Q4_0", - Self::Q4_1 => "Q4_1", - Self::Q5_0 => "Q5_0", - Self::Q5_1 => "Q5_1", - Self::Q8_0 => "Q8_0", - Self::Q8_1 => "Q8_1", - Self::Q2K => "Q2_K", - Self::Q3K => "Q3_K", - Self::Q4K => "Q4_K", - Self::Q5K => "Q5_K", - Self::Q6K => "Q6_K", - Self::IQ2XXS => "IQ2_XXS", - Self::IQ2XS => "IQ2_XS", - Self::BF16 => "BF16", +/// A type in this list is not a promise that serve can DECODE it — `apr/dequant` +/// narrows further, to the quantized subset it has a dequantiser for. +pub const ADMITTED: [GgmlQuantType; 16] = [ + GgmlQuantType::F32, + GgmlQuantType::F16, + GgmlQuantType::Q4_0, + GgmlQuantType::Q4_1, + GgmlQuantType::Q5_0, + GgmlQuantType::Q5_1, + GgmlQuantType::Q8_0, + GgmlQuantType::Q8_1, + GgmlQuantType::Q2K, + GgmlQuantType::Q3K, + GgmlQuantType::Q4K, + GgmlQuantType::Q5K, + GgmlQuantType::Q6K, + GgmlQuantType::IQ2XXS, + GgmlQuantType::IQ2XS, + GgmlQuantType::BF16, +]; + +/// Is this type one serve admits? +#[must_use] +pub const fn is_admitted(t: GgmlQuantType) -> bool { + let mut i = 0; + while i < ADMITTED.len() { + if ADMITTED[i] as u32 == t as u32 { + return true; } + i += 1; } + false +} - /// Parse from string (case-insensitive). Returns `None` for unknown strings. - #[must_use] - pub fn from_str_lossy(s: &str) -> Option { - match s { - "F32" | "f32" => Some(Self::F32), - "F16" | "f16" => Some(Self::F16), - "BF16" | "bf16" => Some(Self::BF16), - "Q4_0" | "q4_0" => Some(Self::Q4_0), - "Q4_1" | "q4_1" => Some(Self::Q4_1), - "Q5_0" | "q5_0" => Some(Self::Q5_0), - "Q5_1" | "q5_1" => Some(Self::Q5_1), - "Q8_0" | "q8_0" => Some(Self::Q8_0), - "Q8_1" | "q8_1" => Some(Self::Q8_1), - "Q2_K" | "q2_k" => Some(Self::Q2K), - "Q3_K" | "q3_k" => Some(Self::Q3K), - "Q4_K" | "q4_k" => Some(Self::Q4K), - "Q5_K" | "q5_k" => Some(Self::Q5K), - "Q6_K" | "q6_k" => Some(Self::Q6K), - "IQ2_XXS" | "iq2_xxs" => Some(Self::IQ2XXS), - "IQ2_XS" | "iq2_xs" => Some(Self::IQ2XS), - _ => None, - } +/// An id becomes a type only if serve admits it. The replacement for a bare +/// `GgmlQuantType::from_id` at every parse boundary in this crate. +#[must_use] +pub const fn admitted_from_id(id: u32) -> Option { + match GgmlQuantType::from_id(id) { + Some(t) if is_admitted(t) => Some(t), + _ => None, } } -impl std::fmt::Display for GgmlQuantType { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.write_str(self.as_str()) - } +/// A name becomes a type only if serve admits it. The replacement for a bare +/// `GgmlQuantType::from_str_lossy`. +#[must_use] +pub fn admitted_from_name(name: &str) -> Option { + GgmlQuantType::from_str_lossy(name).filter(|t| is_admitted(*t)) } // ============================================================================ diff --git a/crates/aprender-serve/src/infer/mod.rs b/crates/aprender-serve/src/infer/mod.rs index 2b33fa5584..40c75ed64f 100644 --- a/crates/aprender-serve/src/infer/mod.rs +++ b/crates/aprender-serve/src/infer/mod.rs @@ -36,7 +36,7 @@ use std::time::Instant; /// PMAT-173 / GH-321: Convert GGML quantization type to human-readable string. /// Uses unified `GgmlQuantType` enum — single source of truth. pub(crate) fn qtype_to_dtype_str(qtype: u32) -> &'static str { - crate::gguf::GgmlQuantType::from_id(qtype).map_or("Unknown", crate::gguf::GgmlQuantType::as_str) + crate::gguf::admitted_from_id(qtype).map_or("Unknown", crate::gguf::GgmlQuantType::as_str) } /// Configuration for inference diff --git a/scripts/check_one_ggml_type_enum.sh b/scripts/check_one_ggml_type_enum.sh new file mode 100755 index 0000000000..598a4e71fa --- /dev/null +++ b/scripts/check_one_ggml_type_enum.sh @@ -0,0 +1,111 @@ +#!/usr/bin/env bash +# check_one_ggml_type_enum.sh — the workspace declares ONE ggml tensor-type enum. +# +# WHY (PMAT-3430, PP-QUANT-001 M1). aprender declared three, and they disagreed +# about which ids exist, about spelling, and about sizes: aprender-serve's +# `GgmlQuantType` (16 ids), aprender-core's `GgmlType` (12), aprender-compute's +# `GgmlType` (15), against 35 live ids upstream. Nothing compared any of them to +# ggml, so NVFP4 (40), Q1_0 (41) and Q2_0 (42) appeared upstream with no signal +# in this tree. M1 collapsed them into `trueno_quant::GgmlType`; this refuses the +# fourth. +# +# THE ENUM IS THE UNIT, NOT THE NAME. A re-export (`pub use trueno_quant::GgmlType`) +# is fine and expected — there are several. What may not exist twice is a +# DEFINITION (`pub enum GgmlType {`). The two are different lines and this guard +# distinguishes them, which is the whole reason it can be green at all. +# +# ROOT FROM $0, NOT FROM git. `git rev-parse --show-toplevel` fails inside the CI +# container ("dubious ownership in repository at '/workspace'": the tree is uid +# 1000, the container is root), and locating our own siblings never needed git. +# Measured: that exact line turned workspace-test-shard (3/3) red on this +# ticket's first CI run (#3586 generalises it). +# +# bash scripts/check_one_ggml_type_enum.sh # check +# bash scripts/check_one_ggml_type_enum.sh --self-test # the case table +set -euo pipefail + +here=$(cd -- "$(dirname -- "$0")" && pwd) +root=${here%/scripts} + +# THE PATTERN, and it ships a case table because this repo's guard regexes have +# been wrong five times and a table caught every one while review caught none. +# A definition is `pub enum {` at the start of a line (leading spaces +# allowed for a nested one); `//` anywhere before it disqualifies the line, so a +# commented-out enum is not a definition. +ENUM_DEF_RE='^[[:space:]]*pub[[:space:]]+enum[[:space:]]+(GgmlType|GgmlQuantType)[[:space:]]*\{' + +find_definitions() { + # $1 = tree to scan. Prints "path:line:text" per definition. + grep -rnE "$ENUM_DEF_RE" "$1" --include='*.rs' 2>/dev/null || true +} + +self_test() { + td=$(mktemp -d) + trap 'rm -rf "${td:?}"' RETURN + mkdir -p "$td/src" + fails=0 + ran=0 + + row() { # row