From 31587f232aa9371f4829323a0f0fae5de9dc9a15 Mon Sep 17 00:00:00 2001 From: t Date: Tue, 10 Nov 2026 08:27:46 -0400 Subject: [PATCH] npu-pack: the tile writer refuses a 4-bit code outside 0..15 instead of wrapping it (engine#1918) `encode_tile()` is a nibble packer: it applies `& 0x0F` to every code, so a code outside 0..15 does not saturate, it **wraps a full code range** -- a weight that lands 4.0 away and still looks plausible. That is the mechanism #1918 measured (`container == expected mod 4.0` over 32/32 columns of a group), and it is also what produced #1863's "every negative column reads exactly +4.0 off" and gave #1874's row-order swap a plausible signature. Nothing documents whether the range is the caller's contract or the writer's, and a wrapped weight on a model that then answers fluently is the worst outcome available, so the writer now refuses and names the position: tile: code 16 at k=128 rows=64, element 0: a 4-bit code is 0..15, and packing it would wrap a full code range (engine#1918) -- clamp or re-quantise before encoding The cost is one pass over `t.code` against a copy of the same size. A caller that means 15 says 15. Arms (6) in `tests/npu_pack_import_test.cpp`, and the pair at the ends of the range matters as much as the refusal: code **0** and code **15** must still encode, so an arm that only checked the throw would pass on a writer that refused everything. Refusal arms assert it carries `engine#1918`, states `0..15`, and says "wrap" -- the three things a caller needs to fix it. Proved they can fail: disabling the check (`if (false && t.code[i] > 0x0F)`) makes **6 arms report FAIL**; the revert is green. Still open on #1918 and deliberately not answered here: whether a *quantiser* (as opposed to a decoder round-trip) should saturate at the range ends rather than refuse. This is the writer's contract; the quantiser's intent is a design question the issue raised and I have not measured. Suite 176/176; copyright clean. Refs #1918, #1863, #1874. --- npu/pack_import.cpp | 11 +++++++++++ tests/npu_pack_import_test.cpp | 30 ++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/npu/pack_import.cpp b/npu/pack_import.cpp index 248c2f6ce..1106ee0aa 100644 --- a/npu/pack_import.cpp +++ b/npu/pack_import.cpp @@ -1981,6 +1981,17 @@ TileW decode_tile(const uint8_t* tile, int k, int rows) { std::vector encode_tile(const TileW& t) { if (t.k <= 0 || t.k % kKBlock) throw std::runtime_error("tile: K is not a positive multiple of 128"); + /* engine#1918: the nibble packer below applies `& 0x0F`, so a code outside 0..15 WRAPS instead of saturating -- a + * weight a full code-range away that still looks plausible (`container == expected mod 4.0` over 32/32 columns of a + * group, measured on #1863's arm). Whether the caller is supposed to guarantee the range or the writer should clamp + * is not documented anywhere, and a wrapped weight on a model that then answers fluently is the worst outcome + * available; so the writer refuses instead, naming the position and the value. A caller that means 15 says 15. */ + for (size_t i = 0; i < t.code.size(); ++i) + if (t.code[i] > 0x0F) + throw std::runtime_error("tile: code " + std::to_string(int(t.code[i])) + " at k=" + std::to_string(t.k) + + " rows=" + std::to_string(t.rows) + ", element " + std::to_string(i) + + ": a 4-bit code is 0..15, and packing it would wrap a full code range " + "(engine#1918) -- clamp or re-quantise before encoding"); const int64_t sv = subv_bytes(t.rows); if (sv == 0) throw std::runtime_error("tile: rows is not a positive multiple of 64"); const int nsubv = t.rows / 2, kbs = t.k / kKBlock; diff --git a/tests/npu_pack_import_test.cpp b/tests/npu_pack_import_test.cpp index c63b3fb93..0d9bed662 100644 --- a/tests/npu_pack_import_test.cpp +++ b/tests/npu_pack_import_test.cpp @@ -3661,7 +3661,37 @@ void test_qk_norm_refusal() { std::printf(" qk norm: 7 arms (declared+not-found refuses; found-only and neither are allowed)\n"); } +/* engine#1918: the nibble packer applies `& 0x0F`, so a code outside 0..15 WRAPS a full code range instead of + * saturating -- measured as `container == expected mod 4.0` over 32/32 columns of a group, i.e. a wrong weight that still + * looks plausible. The writer now refuses, and these arms pin the two ends of the legal range plus the refusal, because an + * arm that only checked the throw would pass on a writer that refused everything. */ +void test_encode_tile_code_range() { + const auto tile_with = [](int v) { + pi::TileW t; + t.k = 128; // one 128-wide group, the minimum the layout allows + t.rows = 64; + t.code.assign(size_t(t.rows) * size_t(t.k), uint8_t(v)); + t.scale.assign(size_t(t.rows), 1.0f); + t.zp.assign(size_t(t.rows), 0); + t.bias.assign(size_t(t.rows), 0); + return t; + }; + bool ok0 = true, ok15 = true; + try { (void)pi::encode_tile(tile_with(0)); } catch (const std::exception&) { ok0 = false; } + try { (void)pi::encode_tile(tile_with(15)); } catch (const std::exception&) { ok15 = false; } + check(ok0, "encode tile: code 0 (the bottom of the legal range) encodes"); + check(ok15, "encode tile: code 15 (the top of the legal range) encodes"); + std::string why; + try { (void)pi::encode_tile(tile_with(16)); } catch (const std::exception& e) { why = e.what(); } + check(!why.empty(), "encode tile: code 16 is REFUSED, not wrapped to 0"); + check(why.find("engine#1918") != std::string::npos, "encode tile: the refusal carries its issue"); + check(why.find("0..15") != std::string::npos, "encode tile: and states the range it wants"); + check(why.find("wrap") != std::string::npos, "encode tile: and says what would have happened instead"); + std::printf(" encode tile: 6 arms (0 and 15 encode, 16 refuses by name)\n"); +} + int main(int argc, char** argv) { + test_encode_tile_code_range(); test_qk_norm_refusal(); test_tail_head_placement(); const std::string golden_dir = argc > 1 ? argv[1] : "";