diff --git a/docs/npu.md b/docs/npu.md index a5a47c2a0..1ae6d6ae9 100644 --- a/docs/npu.md +++ b/docs/npu.md @@ -1358,6 +1358,11 @@ differ because the two encodings are different, both exactly: | Q4_0 codes | unsigned `q` (0..15) | **signed** `q-8`, nibble = `q` xor 8 | | code order in a tile | `[row-half(2)][group(8)][col-in-group(32)][row-pair(8)]` (the 16-row split the kernel walks) | plain column major `[col(256)][row-pair(16)]` | +`npu-pack`'s writers hold the container to that unsigned field: `tile_q4_1`, `tile_q4_1_f32` and the +`.fconst` tile encoder all refuse a code above 15, naming where it is, rather than mask it into +a different code one full range away (engine#1921). A signed code has to be converted before it +reaches a writer, which is what the 1.9 decoder does. + Measured, per tensor, against the GGUF as ground truth: **196/196 Q4_0 tensors exact (0.0 error) under each file's own decoder** — so both converters are correct — and **0/197 exact when one is read with the other's decoder** (worst error 2.8-3.0). The scale planes of all 53 760 Q4_0 tiles diff --git a/npu/pack_import.cpp b/npu/pack_import.cpp index 3c6c09bae..492a7d494 100644 --- a/npu/pack_import.cpp +++ b/npu/pack_import.cpp @@ -2079,6 +2079,18 @@ namespace { void tile_q4_impl(const Q4& q, std::vector& out, bool f32, long long* param_inexact) { if (q.N % q4nx::kRows) throw std::runtime_error("q4_1 tiles need N % 32 == 0"); + // The same refusal encode_tile() makes (engine#1921), on the writer that needed it first: the packing below masks + // with `& 0x0F`, so a code outside 0..15 would land as a different, plausible code one full range away. That is + // what engine#1918 measured on this writer (a two's-complement int4 code written into the unsigned field). Its + // cause was fixed at the decoder by #1874, so no caller produces such a code today; this keeps a future one from + // being packed silently. One pass over the codes, before anything is written. + for (size_t i = 0; i < q.codes.size(); ++i) + if (q.codes[i] > 0x0F) + throw std::runtime_error("q4_1 tile: code " + std::to_string(int(q.codes[i])) + " at row " + + std::to_string(q.K > 0 ? i / size_t(q.K) : 0) + ", column " + + std::to_string(q.K > 0 ? i % size_t(q.K) : 0) + + ": a 4-bit code is 0..15, and packing it would wrap a full code range " + "(engine#1921: convert a signed code before it reaches the writer)"); // The grid is 32 x 256, so a tensor whose K is not a whole number of 256-column // tiles (Qwen2.5-0.5B: hidden 896 = 3.5 tiles) is stored with the last column tile // zero-padded: codes 0, scale 0, zero 0, i.e. w = 0 in the padding. The real K is diff --git a/tests/npu_pack_import_test.cpp b/tests/npu_pack_import_test.cpp index ea1437dc9..9087bb95e 100644 --- a/tests/npu_pack_import_test.cpp +++ b/tests/npu_pack_import_test.cpp @@ -5344,6 +5344,42 @@ void test_encode_tile_code_range() { std::printf(" encode tile: 6 arms (0 and 15 encode, 16 refuses by name)\n"); } +// engine#1921 follow-up: the container writer (tile_q4_1 / tile_q4_1_f32) packs with the same `& 0x0F` as +// encode_tile(), and it is the writer engine#1918's wrap was measured on. Both tile forms refuse a code above 15 by +// position, and the legal extremes still write: a guard that refused everything would pass a throw-only arm. +void test_q4_writer_code_range() { + auto make = [](uint8_t code_at_last) { + pi::Q4 q; + q.N = 32; q.K = 256; q.block = 32; + q.codes.assign(size_t(q.N) * q.K, 0); + q.scale.assign(size_t(q.N) * (q.K / 32), 1.0f); + q.zero.assign(size_t(q.N) * (q.K / 32), 0.0f); + q.codes[0] = 15; // the top of the range, legal + q.codes[size_t(31) * q.K + 255] = code_at_last; + return q; + }; + for (int f32 = 0; f32 < 2; ++f32) { + const char* form = f32 ? "q4_1f" : "q4_1"; + auto run = [&](const pi::Q4& q, std::vector& out) { + if (f32) pi::tile_q4_1_f32(q, out); else pi::tile_q4_1(q, out); + }; + std::vector out; + bool ok = false; + try { run(make(15), out); ok = !out.empty(); } catch (const std::exception&) {} + check(ok, std::string("q4 writer (") + form + "): codes 0 and 15 write a tile"); + std::string why; + std::vector out2; + try { run(make(16), out2); } catch (const std::exception& e) { why = e.what(); } + check(!why.empty(), std::string("q4 writer (") + form + "): code 16 is REFUSED, not wrapped to 0"); + check(why.find("row 31, column 255") != std::string::npos, + std::string("q4 writer (") + form + "): the refusal names the row and column"); + check(why.find("0..15") != std::string::npos && why.find("wrap") != std::string::npos, + std::string("q4 writer (") + form + "): and states the range and what would have happened"); + check(out2.empty(), std::string("q4 writer (") + form + "): nothing is written before the refusal"); + } + std::printf(" q4 writer: 10 arms (both tile forms: 15 writes, 16 refuses by row and column)\n"); +} + // engine#1889: the arithmetic that claims the per-head QK gains out of the 640 B Hunyuan-7B carries between its last qkv // tile and o_proj. A wrong offset there reads tile bytes as bf16 and mis-rotates every pack of the family, so both the // right shape (2 * (64 + head_dim*2) = 640 at head_dim 128) and the neighbours that must NOT be taken for it are driven. @@ -5407,6 +5443,7 @@ int main(int argc, char** argv) { } scratch_cleanup{run_scratch}; test_header_witness_refusal(); test_encode_tile_code_range(); + test_q4_writer_code_range(); test_qk_norm_refusal(); test_qk_gain_offsets(); test_tail_head_placement();