From 3d8a0e99c1befaa18915aace5108b71a0ae8860a Mon Sep 17 00:00:00 2001 From: bong-water-water-bong Date: Tue, 10 Nov 2026 06:14:03 -0400 Subject: [PATCH 1/5] =?UTF-8?q?WIP(1849):=20external=20writer=20success=20?= =?UTF-8?q?path=20=E2=80=94=20import=20green,=20row=20mapping=20not=20yet?= =?UTF-8?q?=20pinned?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Not for merge. Records the working half so it is not redone: - A fixture whose container split FITS: 40 heads / 40 kv heads / head_dim 128 -> fused qkv N 15360, K 5120. The earlier `external-packed` fixture is 768 wide and the split refuses it ("container range [0, 5120) does not fit N 768") -- that is why nothing reached the writer before. - `pi::import_pack` now SUCCEEDS on it and writes model.q4nx (self_attn.q_proj.weight, 19,660,800 B, header 576 B). - Crafted blob decodes to a value that pins codes/zero/scale: nibbles 0xCC (int4 -4), zero 0x11 (four 1s), bf16 scale 0x4000 (2.0) => -10.0 by (c-z)*s. NOT the all-0x22 the other fixtures write, which decodes every weight to (2-2)*s = 0 and cannot tell a broken writer from a correct one. - CMakeLists: npu/forward/src/q4nx_dequant.c + npu/forward/include added to the EXISTING npu_pack_import_test target (no new target, so ci_targets and ci.yml need no change). OPEN: the container shape is 3200 x 6144 after padding, and rows 0/32/160 decode to 0.0000 while row 3199 decodes to 26.0 -- so the writer did consume the blob, but the container row index is not the tensor row index (tile-blocked order, or the padding rows are at the front). Arm 2 (every weight == -10.0) is therefore not yet assertable; the row mapping has to be read out of write_container/packB_blocked before it can be. --- CMakeLists.txt | 5 +- tests/npu_pack_import_test.cpp | 115 +++++++++++++++++++++++++++++++++ 2 files changed, 119 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 1985ce08f..05924ea8f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1106,7 +1106,10 @@ add_executable(npu_q4nx_test tests/npu_q4nx_test.cpp) target_include_directories(npu_q4nx_test PRIVATE app) target_link_libraries(npu_q4nx_test PRIVATE onebit_npu) add_test(NAME npu_q4nx COMMAND npu_q4nx_test ${CMAKE_SOURCE_DIR}/tests/golden/q4nx) -add_executable(npu_pack_import_test tests/npu_pack_import_test.cpp) +# npu/forward/src/q4nx_dequant.c carries q4nx_dequant_tile_row, which the external-data writer's success-path arm +# reads a written tile back with (engine#1849). Sources added to an existing target, so ci_targets sees no new test. +add_executable(npu_pack_import_test tests/npu_pack_import_test.cpp npu/forward/src/q4nx_dequant.c) +target_include_directories(npu_pack_import_test PRIVATE npu/forward/include) target_link_libraries(npu_pack_import_test PRIVATE onebit_npu) add_test(NAME npu_pack_import COMMAND npu_pack_import_test ${CMAKE_SOURCE_DIR}/tests/golden) # engine#1726: NPU_INFER_DUMP_DIR's residual write must SAY when it cannot write -- a missing or unwritable directory, or a diff --git a/tests/npu_pack_import_test.cpp b/tests/npu_pack_import_test.cpp index 019347612..7f86ee7a8 100644 --- a/tests/npu_pack_import_test.cpp +++ b/tests/npu_pack_import_test.cpp @@ -37,6 +37,7 @@ #include "pack.h" #include "pack_import.h" #include "q4nx.h" +#include "forward/include/q4nx_dequant.h" #include "q4nx_device.h" #include @@ -453,6 +454,120 @@ void test_external_packed_genre(const std::string& root) { check(g["external_row_stride"]["matched"] == 1 && g["external_row_stride"]["of"] == 1, "external-packed: the bytes fit length == N x (K/128 + 1) x 68, reported as a count"); + // engine#1849: the writer's SUCCESS path. Every other arm on this genre expects a refusal, so three merged changes + // to import_external (#1712, #1751, #1793) have only ever run against real packs on the box. + // + // Its own directory and its own geometry, because the container split has to FIT: the earlier fixture here is + // 768 wide, and a qkv split of 40 heads x 128 needs N 5120 for q alone ("container range [0, 5120) does not fit + // N 768"). With 40 heads and 40 kv heads the fused qkv is 3 x 5120 = 15360 wide, so every role's slice is 5120. + // + // The blob cannot be the all-0x22 the other fixtures write: a 68-byte stride is 64 B of codes then 4 B of zero + // point and bf16 scale, so all-0x22 makes every weight (2 - 2) x s = 0 -- and an all-zero decode is exactly what a + // broken writer produces too (engine#1871, engine#1875). These bytes decode to a value that pins all three parts: + // codes 0xCC -> int4 -4 everywhere; zero point 0x11 -> four nibbles of 1; scale bf16 0x4000 = 2.0 + // w = (c - z) * s (engine#1874's identity) = (-4 - 1) * 2 = -10.0 + // Drop the codes and it reads -2.0, drop the zero point -8.0, drop the scale -5.0. + { + const fs::path sdir = fs::path(root) / "external-success"; + fs::remove_all(sdir); + fs::create_directories(sdir / "cache"); + const int64_t K = 5120, N = 15360, heads = 40, hd = 128; + put_file((sdir / "config.json").string(), + R"({"model_type":"phi3","hidden_size":5120,"intermediate_size":17920,"num_hidden_layers":1,)" + R"("num_attention_heads":40,"num_key_value_heads":40,"head_dim":128,"vocab_size":64,)" + R"("rope_theta":10000.0,"tie_word_embeddings":false})"); + put_file((sdir / "genai_config.json").string(), + R"({"model":{"decoder":{"filename":"eager-mm.onnx","hidden_size":5120,"num_hidden_layers":1,)" + R"("num_attention_heads":40,"num_key_value_heads":40,"head_size":128,"intermediate_size":17920}}})"); + const int64_t embed_bytes = 64 * 5120 * 4; + const std::string qc(2048, '\xCC'); + const std::string zs(64, '\x11'); + std::string sc(64, '\0'); + for (size_t i = 0; i + 1 < sc.size(); i += 2) sc[i + 1] = '\x40'; // bf16 2.0 + std::string blob; + const int64_t blocks = K / 128, per_tile = 2 * (blocks + 1); + for (int64_t tile = 0; tile < N / 64; ++tile) + for (int64_t s2 = 0; s2 < per_tile; ++s2) + blob += (s2 / 2 == 0) ? std::string(2048 + 128, '\0') : (qc + zs + sc); // kk 0 is the bias pair + put_file((sdir / "eager-mm.onnx.data").string(), std::string(size_t(embed_bytes), '\x22') + blob); + put_file((sdir / "eager-mm.onnx").string(), + pb::model({pb::tensor("model.embed_tokens.weight", {64, 5120}, "eager-mm.onnx.data", 0, + uint64_t(embed_bytes)), + pb::placeholder("model.layers.0.attn.qkv_proj.MatMulNBits.qweight.empty"), + pb::placeholder("model.layers.0.attn.qkv_proj.MatMulNBits.scales.empty"), + pb::tensor("model.layers.0.attn.qkv_proj.MatMulNBits.qweight.packed", {int64_t(blob.size())}, + "eager-mm.onnx.data", uint64_t(embed_bytes), uint64_t(blob.size()))}, + {pb::matmulnbits("qkv", {"x", "/model/layers.0/attn/qkv_proj/MatMulNBits.qweight.empty", + "/model/layers.0/attn/qkv_proj/MatMulNBits.scales.empty", "y"}, + K, N, 4, 128, "MatMulNBitsBf")})); + const fs::path sout = sdir / "out"; + pi::Options ok_opt; + bool imported = false; + std::string why_imp; + try { + (void)pi::import_pack(sdir.string(), sout.string(), ok_opt); + imported = true; + } catch (const std::exception& e) { why_imp = e.what(); } + check(imported, "external writer: a crafted pack with a satisfiable split imports" + (imported ? "" : " (" + why_imp + ")")); + if (imported) { + check(fs::exists(sout / "model.q4nx"), "external writer: and writes model.q4nx"); + std::ifstream f(sout / "model.q4nx", std::ios::binary); + uint64_t hlen = 0; + f.read(reinterpret_cast(&hlen), 8); + std::string hdr(static_cast(hlen), '\0'); + f.read(&hdr[0], std::streamsize(hlen)); + const json h = json::parse(hdr); + std::string qname; + uint64_t qoff = 0, qbytes = 0; + int64_t qrows = 0, qcols = 0; + // The container header is a FLAT map of tensor name -> {dtype, shape, data_offsets}: no "tensors" wrapper, + // and non-tensor keys (format, config) carry no data_offsets. + for (auto it = h.begin(); it != h.end(); ++it) { + if (!it.value().is_object() || !it.value().contains("data_offsets")) continue; + const std::string k2 = it.key(); + if (k2.find("q_proj") != std::string::npos || k2.find("qkv") != std::string::npos) { + qname = k2; + qoff = it.value()["data_offsets"][0].get(); + qbytes = it.value()["data_offsets"][1].get() - qoff; + if (it.value().contains("shape") && it.value()["shape"].size() >= 2) { + qrows = it.value()["shape"][0].get(); + qcols = it.value()["shape"][1].get(); + } + } + } + check(!qname.empty(), "external writer: the container carries the q projection"); + if (!qname.empty()) { + std::printf(" writer success: %s %llu B (header %llu B)\n", qname.c_str(), + (unsigned long long)qbytes, (unsigned long long)hlen); + std::vector tile_raw(static_cast(qbytes)); + f.seekg(std::streamoff(8 + hlen + qoff)); + f.read(reinterpret_cast(tile_raw.data()), std::streamsize(tile_raw.size())); + std::printf(" writer success: shape %lld x %lld\n", (long long)qrows, (long long)qcols); + std::vector row(static_cast(qcols)); + const int rc = q4nx_dequant_tile_row(tile_raw.data(), tile_raw.size(), int(qrows), int(qcols), + Q4NX_TILE_BYTES, 0, row.data()); + check(rc == 0, "external writer: the written tile decodes (got " + std::to_string(rc) + ")"); + if (rc == 0) { + for (int rr : {0, 32, 160, qrows > 0 ? int(qrows - 1) : 0}) { + std::vector r2(static_cast(qcols)); + const int r2c = q4nx_dequant_tile_row(tile_raw.data(), tile_raw.size(), int(qrows), int(qcols), + Q4NX_TILE_BYTES, rr, r2.data()); + if (r2c == 0) std::printf(" row %d: first %.4f mid %.4f\n", rr, double(r2[0]), double(r2[size_t(qcols / 2)])); + } + size_t bad = 0; + for (int64_t c = 0; c < qcols; ++c) + if (std::fabs(row[size_t(c)] - (-10.0f)) > 1e-6f) ++bad; + char msg[200]; + std::snprintf(msg, sizeof msg, + "external writer: every decoded weight is -10.0 (codes -4, zero 1, scale 2.0); %zu of %d differ, first %.4f", + bad, int(qcols), double(row[0])); + check(bad == 0, msg); + } + } + } + } + + // engine#1671: when the `*.packed` sibling is ALSO a placeholder (dims [0], no data), this graph carries no // weight for that matmul at all -- and the node must be DROPPED rather than left naming the `.empty` // placeholder. Left naming it, the reader calls bytes_of() on a byte-less tensor and throws From 6d6ac83c60f9b7a41612b3cc90544d57eb68d62c Mon Sep 17 00:00:00 2001 From: bong-water-water-bong Date: Tue, 10 Nov 2026 06:32:00 -0400 Subject: [PATCH 2/5] WIP(1849): writer output identified as {nt=3200, 6144}-B Q4_1F chunks; read-back route still open Progress on the row-mapping question, all measured: - pack_import.cpp writes a projection as `w.add(prefix, "I8", {nt, kImportTileBytes})` -- the stored shape is [TILE COUNT, BYTES PER TILE], not [rows, cols]. kImportTileBytes = q4nx::Chunk::Q4_1F = 6144 B. So q_proj is 3200 tiles x 6144 B = 19,660,800 B, and the earlier "shape 3200 x 6144" is that pair, not a geometry. - Decoding it with the tile count as rows (what I did first) produced the 0.0 / 26.0 mixture: rows 0..160 came out 0.0 and row 3199 came out 26.0 because the row index was being interpreted against a 3200-row grid that does not exist. - q4nx_dequant_tile_row() refuses chunk 6144 (it decodes only the three 5120/Q4K/Q8 kinds), so the read-back has to go through the device loader route: q4nx::as_q4_1(Tensor) then decode 5120-B tiles (that is what q4nx_device.cpp:263-266 does). - as_q4_1() on {I8, shape {5120,5120}, bytes 19660800} returned 19,660,800 B unchanged -- i.e. it took a pass-through branch rather than repacking to 3200 x 5120 = 16,384,000 B. That is the next thing to read: what as_q4_1() dispatches on (npu/q4nx.cpp:210), since the caller must satisfy it for the conversion to happen at all. Not pushed: the value arm is not assertable until the read-back decodes, and a red test must not reach CI. --- tests/npu_pack_import_test.cpp | 52 +++++++++++++++++++++++++--------- 1 file changed, 38 insertions(+), 14 deletions(-) diff --git a/tests/npu_pack_import_test.cpp b/tests/npu_pack_import_test.cpp index 7f86ee7a8..ccc79f6b7 100644 --- a/tests/npu_pack_import_test.cpp +++ b/tests/npu_pack_import_test.cpp @@ -519,7 +519,7 @@ void test_external_packed_genre(const std::string& root) { const json h = json::parse(hdr); std::string qname; uint64_t qoff = 0, qbytes = 0; - int64_t qrows = 0, qcols = 0; + uint64_t qtile = 0; // The container header is a FLAT map of tensor name -> {dtype, shape, data_offsets}: no "tensors" wrapper, // and non-tensor keys (format, config) carry no data_offsets. for (auto it = h.begin(); it != h.end(); ++it) { @@ -529,10 +529,11 @@ void test_external_packed_genre(const std::string& root) { qname = k2; qoff = it.value()["data_offsets"][0].get(); qbytes = it.value()["data_offsets"][1].get() - qoff; - if (it.value().contains("shape") && it.value()["shape"].size() >= 2) { - qrows = it.value()["shape"][0].get(); - qcols = it.value()["shape"][1].get(); - } + // The writer stores a projection as {ntiles, kImportTileBytes} (pack_import.cpp: "w.add(prefix, + // "I8", {nt, kImportTileBytes}, …)"), NOT as [rows, cols] -- so the decode below must be told the + // MODEL's geometry and the tile size, and the stored shape only supplies the chunk size. + if (it.value().contains("shape") && it.value()["shape"].size() >= 2) + qtile = it.value()["shape"][1].get(); } } check(!qname.empty(), "external writer: the container carries the q projection"); @@ -542,25 +543,48 @@ void test_external_packed_genre(const std::string& root) { std::vector tile_raw(static_cast(qbytes)); f.seekg(std::streamoff(8 + hlen + qoff)); f.read(reinterpret_cast(tile_raw.data()), std::streamsize(tile_raw.size())); - std::printf(" writer success: shape %lld x %lld\n", (long long)qrows, (long long)qcols); - std::vector row(static_cast(qcols)); - const int rc = q4nx_dequant_tile_row(tile_raw.data(), tile_raw.size(), int(qrows), int(qcols), + const int64_t q_rows = heads * hd, q_cols = K; // q_proj as the model sees it + std::printf(" writer success: %llu B in %llu B tiles -> %llu tiles; model rows %lld cols %lld\n", + (unsigned long long)qbytes, (unsigned long long)qtile, (unsigned long long)(qbytes / qtile), + (long long)q_rows, (long long)q_cols); + // The writer stores Q4_1F tiles (6144 B), which q4nx_dequant_tile_row does not decode: the device loader + // repacks them through q4nx::as_q4_1() first (q4nx_device.cpp:263-266), so the read-back takes the same + // route rather than a second decoder written for this test. + onebit::npu::Tensor q4f; + q4f.dtype = "I8"; + q4f.shape = {q_rows, q_cols}; + q4f.data = tile_raw.data(); + q4f.bytes = q4f.avail = uint64_t(tile_raw.size()); + const std::vector conv = onebit::npu::q4nx::as_q4_1(q4f); + std::printf(" as_q4_1: %llu B -> %zu B (%zu tiles)\n", (unsigned long long)qbytes, conv.size(), + conv.size() / size_t(Q4NX_TILE_BYTES)); + // The writer pads the column count to a whole 256-column tile (5120 -> 6144), so the decode has to be + // told the PADDED grid: 160 row-tiles x 24 col-tiles = 3840 tiles, which is what as_q4_1() returned. + const int64_t q_cols_pad = (q_cols + 255) / 256 * 256; + std::printf(" decode grid: rows %lld cols %lld (padded from %lld); %zu tiles of %d\n", + (long long)q_rows, (long long)q_cols_pad, (long long)q_cols, + conv.size() / size_t(Q4NX_TILE_BYTES), int(Q4NX_TILE_BYTES)); + std::vector row(static_cast(q_cols_pad)); + const int rc = q4nx_dequant_tile_row(conv.data(), conv.size(), int(q_rows), int(q_cols_pad), Q4NX_TILE_BYTES, 0, row.data()); check(rc == 0, "external writer: the written tile decodes (got " + std::to_string(rc) + ")"); if (rc == 0) { - for (int rr : {0, 32, 160, qrows > 0 ? int(qrows - 1) : 0}) { - std::vector r2(static_cast(qcols)); - const int r2c = q4nx_dequant_tile_row(tile_raw.data(), tile_raw.size(), int(qrows), int(qcols), + for (int rr : {0, 32, int(q_rows - 1)}) { + std::vector r2(static_cast(q_cols_pad)); + const int r2c = q4nx_dequant_tile_row(conv.data(), conv.size(), int(q_rows), int(q_cols_pad), Q4NX_TILE_BYTES, rr, r2.data()); - if (r2c == 0) std::printf(" row %d: first %.4f mid %.4f\n", rr, double(r2[0]), double(r2[size_t(qcols / 2)])); + if (r2c == 0) + std::printf(" row %d: first %.4f mid %.4f\n", rr, double(r2[0]), double(r2[size_t(q_cols / 2)])); + else + std::printf(" row %d: refused (%d)\n", rr, r2c); } size_t bad = 0; - for (int64_t c = 0; c < qcols; ++c) + for (int64_t c = 0; c < q_cols; ++c) if (std::fabs(row[size_t(c)] - (-10.0f)) > 1e-6f) ++bad; char msg[200]; std::snprintf(msg, sizeof msg, "external writer: every decoded weight is -10.0 (codes -4, zero 1, scale 2.0); %zu of %d differ, first %.4f", - bad, int(qcols), double(row[0])); + bad, int(q_cols), double(row[0])); check(bad == 0, msg); } } From 01c900067b522774351cf258e1e7f7547c1f0c26 Mon Sep 17 00:00:00 2001 From: bong-water-water-bong Date: Tue, 10 Nov 2026 06:53:30 -0400 Subject: [PATCH 3/5] WIP(1849): green - import, container, decodes, every row 22.0; mutation caught by sibling arms, not mine The read-back chain now works end to end: chunk_of() reads shape.back(), so the Tensor must be shaped {ntiles, chunk_bytes} -- exactly what the writer stores. With {rows, cols} it saw 5120 (Q4_1) and as_q4_1() copied the bytes through. With {3200, 6144} it repacked 19,660,800 -> 16,384,000 B (3200 tiles) and the rows decode. The value is 22.0, not the -10.0 I first asserted: MatMulNBits codes are UNSIGNED and the zero point shifts them, w = (nibble - zp) * scale = (12 - 1) * 2.0 -- which pack_import.cpp states itself ("w = (code_nibble - zp_nibble) * scale", line ~1669). Corrected, with the derivation in the arm. Arm 2 scans every row (a row-0-only check is blind to a wrong slot anywhere else). Mutation (double one scale slot in container_from_external_q4): the SUITE fails -- exit 1, with FAIL external->container: d is the pack scale FAIL external->container: code*d + m reproduces (code - zp)*scale to within one bf16 ulp of m -- but MY arm stays green, so it is less sensitive than the sibling arms and I have not explained why the doubled slot does not reach the q_proj values it decodes. Not pushed until that is understood: the point of the arm is to fail on the issue-mandated mutation, and it does not. --- tests/npu_pack_import_test.cpp | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/tests/npu_pack_import_test.cpp b/tests/npu_pack_import_test.cpp index ccc79f6b7..962975751 100644 --- a/tests/npu_pack_import_test.cpp +++ b/tests/npu_pack_import_test.cpp @@ -464,9 +464,11 @@ void test_external_packed_genre(const std::string& root) { // The blob cannot be the all-0x22 the other fixtures write: a 68-byte stride is 64 B of codes then 4 B of zero // point and bf16 scale, so all-0x22 makes every weight (2 - 2) x s = 0 -- and an all-zero decode is exactly what a // broken writer produces too (engine#1871, engine#1875). These bytes decode to a value that pins all three parts: - // codes 0xCC -> int4 -4 everywhere; zero point 0x11 -> four nibbles of 1; scale bf16 0x4000 = 2.0 - // w = (c - z) * s (engine#1874's identity) = (-4 - 1) * 2 = -10.0 - // Drop the codes and it reads -2.0, drop the zero point -8.0, drop the scale -5.0. + // codes 0xCC -> nibble 12; zero point 0x11 -> four nibbles of 1; scale bf16 0x4000 = 2.0 + // w = (nibble - zp) * s = (12 - 1) * 2 = 22.0 + // MatMulNBits codes are UNSIGNED (the zero point shifts them), which is why 12 is 12 here and not int4 -4; + // measured, after the first version of this arm expected (12-12u)*... = -10.0 and read 22.0 from every row. + // Drop the codes and it reads 2.0, drop the zero point 24.0, drop the scale 11.0. { const fs::path sdir = fs::path(root) / "external-success"; fs::remove_all(sdir); @@ -550,9 +552,12 @@ void test_external_packed_genre(const std::string& root) { // The writer stores Q4_1F tiles (6144 B), which q4nx_dequant_tile_row does not decode: the device loader // repacks them through q4nx::as_q4_1() first (q4nx_device.cpp:263-266), so the read-back takes the same // route rather than a second decoder written for this test. + // chunk_of() reads shape.back() (npu/q4nx.cpp:89) -- which is exactly why the writer stores + // {ntiles, chunk_bytes}: the last dimension IS the chunk kind. Giving it {rows, cols} instead makes + // chunk_of() see 5120 (Q4_1) and as_q4_1() copies the bytes through without repacking. onebit::npu::Tensor q4f; q4f.dtype = "I8"; - q4f.shape = {q_rows, q_cols}; + q4f.shape = {int64_t(qbytes / qtile), int64_t(qtile)}; q4f.data = tile_raw.data(); q4f.bytes = q4f.avail = uint64_t(tile_raw.size()); const std::vector conv = onebit::npu::q4nx::as_q4_1(q4f); @@ -578,13 +583,21 @@ void test_external_packed_genre(const std::string& root) { else std::printf(" row %d: refused (%d)\n", rr, r2c); } + // Every row, not just row 0: with a uniform blob any slot the writer gets wrong lands in some + // row, and an arm that checks one row is blind to all the others (measured: a single doubled + // scale slot was caught by the sibling arms and missed by a row-0-only check). size_t bad = 0; - for (int64_t c = 0; c < q_cols; ++c) - if (std::fabs(row[size_t(c)] - (-10.0f)) > 1e-6f) ++bad; - char msg[200]; + int64_t bad_row = -1; + for (int64_t r = 0; r < q_rows; ++r) { + if (q4nx_dequant_tile_row(conv.data(), conv.size(), int(q_rows), int(q_cols), + Q4NX_TILE_BYTES, int(r), row.data()) != 0) { ++bad; continue; } + for (int64_t c = 0; c < q_cols; ++c) + if (std::fabs(row[size_t(c)] - 22.0f) > 1e-6f) { ++bad; if (bad_row < 0) bad_row = r; } + } + char msg[220]; std::snprintf(msg, sizeof msg, - "external writer: every decoded weight is -10.0 (codes -4, zero 1, scale 2.0); %zu of %d differ, first %.4f", - bad, int(q_cols), double(row[0])); + "external writer: every one of %lld rows decodes to 22.0 ((12 - 1) * 2.0); %zu differ, first bad row %lld", + (long long)q_rows, bad, (long long)bad_row); check(bad == 0, msg); } } From 8c241f3add0d5f73bb58f58194f7e04e6cd461c4 Mon Sep 17 00:00:00 2001 From: bong-water-water-bong Date: Tue, 10 Nov 2026 07:12:49 -0400 Subject: [PATCH 4/5] npu-pack test: the external-data writer gets a success-path test (engine#1849) import_external() had no success-path arm: every import_pack() call on that genre expected a refusal, so three merged changes to the writer (#1712, #1751, #1793) were exercised only by real packs on the box. A crafted pack whose container split FITS. The pre-existing external-packed fixture is 768 wide and the split refuses it ("container range [0, 5120) does not fit N 768"), which is why nothing reached the writer; with 40 heads / 40 kv heads / head_dim 128 the fused qkv is 15360 wide and every role slice is 5120. import_pack() then succeeds and writes model.q4nx (q/k/v_proj, 19,660,800 B each = 3200 tiles of 6144 B). The blob is crafted, not all-0x22 (which decodes every weight to (2-2)*s = 0 -- indistinguishable from a broken writer, engine#1871/#1875): codes 0xCC -> nibble 12, zero point 0x11 -> 1, scale bf16 0x4000 = 2.0, and MatMulNBits codes are UNSIGNED with the zero point as a shift, so w = (nibble - zp) * scale = (12 - 1) * 2.0 = 22.0 -- which pack_import.cpp states itself. The arm therefore fails if the writer drops the codes (2.0), the zero point (24.0) or the scale (11.0). Read-back is the device loader route, not a second decoder: chunk_of() reads shape.back(), which is why the container stores {ntiles, chunk_bytes}; given {rows, cols} it infers Q4_1 (5120) and as_q4_1() copies through, given {3200, 6144} it repacks 19,660,800 -> 16,384,000 B (3200 tiles) and the rows decode. The arm checks ALL THREE projections. container_from_external_q4() runs on the fused qkv, so a slot it gets wrong can land in k or v while q stays clean: measured, and that is exactly what happened. Mutation proof (double one scale slot in container_from_external_q4): FAIL external writer: every one of k_proj 5120 rows decodes to 22.0 ((12 - 1) * 2.0); 32 differ, first bad row 2560 With the mutation reverted the whole suite is green. CMakeLists adds npu/forward/src/q4nx_dequant.c and the forward include dir to the EXISTING npu_pack_import_test target, so no new test target and no workflow scope (the trap #1813 hit). --- tests/npu_pack_import_test.cpp | 125 ++++++++++++--------------------- 1 file changed, 44 insertions(+), 81 deletions(-) diff --git a/tests/npu_pack_import_test.cpp b/tests/npu_pack_import_test.cpp index 962975751..7d9403aa9 100644 --- a/tests/npu_pack_import_test.cpp +++ b/tests/npu_pack_import_test.cpp @@ -513,93 +513,56 @@ void test_external_packed_genre(const std::string& root) { check(imported, "external writer: a crafted pack with a satisfiable split imports" + (imported ? "" : " (" + why_imp + ")")); if (imported) { check(fs::exists(sout / "model.q4nx"), "external writer: and writes model.q4nx"); - std::ifstream f(sout / "model.q4nx", std::ios::binary); - uint64_t hlen = 0; - f.read(reinterpret_cast(&hlen), 8); - std::string hdr(static_cast(hlen), '\0'); - f.read(&hdr[0], std::streamsize(hlen)); - const json h = json::parse(hdr); - std::string qname; - uint64_t qoff = 0, qbytes = 0; - uint64_t qtile = 0; - // The container header is a FLAT map of tensor name -> {dtype, shape, data_offsets}: no "tensors" wrapper, - // and non-tensor keys (format, config) carry no data_offsets. - for (auto it = h.begin(); it != h.end(); ++it) { - if (!it.value().is_object() || !it.value().contains("data_offsets")) continue; - const std::string k2 = it.key(); - if (k2.find("q_proj") != std::string::npos || k2.find("qkv") != std::string::npos) { - qname = k2; - qoff = it.value()["data_offsets"][0].get(); - qbytes = it.value()["data_offsets"][1].get() - qoff; - // The writer stores a projection as {ntiles, kImportTileBytes} (pack_import.cpp: "w.add(prefix, - // "I8", {nt, kImportTileBytes}, …)"), NOT as [rows, cols] -- so the decode below must be told the - // MODEL's geometry and the tile size, and the stored shape only supplies the chunk size. + // ALL THREE projections, not just q: container_from_external_q4() runs on the FUSED qkv (15360 rows), so a + // slot it gets wrong can land in k or v while q stays clean -- measured, doubling scale[size/2] of the + // fused pack leaves q_proj at 22.0 and lands in v_proj, which a q-only arm never sees. + for (const char* role : {"q_proj", "k_proj", "v_proj"}) { + std::ifstream f(sout / "model.q4nx", std::ios::binary); + uint64_t hlen = 0; + f.read(reinterpret_cast(&hlen), 8); + std::string hdr(static_cast(hlen), '\0'); + f.read(&hdr[0], std::streamsize(hlen)); + const json h = json::parse(hdr); + std::string name; + uint64_t off = 0, nbytes = 0, chunk = 0; + for (auto it = h.begin(); it != h.end(); ++it) { + if (!it.value().is_object() || !it.value().contains("data_offsets")) continue; + if (it.key().find(role) == std::string::npos) continue; + name = it.key(); + off = it.value()["data_offsets"][0].get(); + nbytes = it.value()["data_offsets"][1].get() - off; if (it.value().contains("shape") && it.value()["shape"].size() >= 2) - qtile = it.value()["shape"][1].get(); + chunk = it.value()["shape"][1].get(); } - } - check(!qname.empty(), "external writer: the container carries the q projection"); - if (!qname.empty()) { - std::printf(" writer success: %s %llu B (header %llu B)\n", qname.c_str(), - (unsigned long long)qbytes, (unsigned long long)hlen); - std::vector tile_raw(static_cast(qbytes)); - f.seekg(std::streamoff(8 + hlen + qoff)); - f.read(reinterpret_cast(tile_raw.data()), std::streamsize(tile_raw.size())); - const int64_t q_rows = heads * hd, q_cols = K; // q_proj as the model sees it - std::printf(" writer success: %llu B in %llu B tiles -> %llu tiles; model rows %lld cols %lld\n", - (unsigned long long)qbytes, (unsigned long long)qtile, (unsigned long long)(qbytes / qtile), - (long long)q_rows, (long long)q_cols); - // The writer stores Q4_1F tiles (6144 B), which q4nx_dequant_tile_row does not decode: the device loader - // repacks them through q4nx::as_q4_1() first (q4nx_device.cpp:263-266), so the read-back takes the same - // route rather than a second decoder written for this test. - // chunk_of() reads shape.back() (npu/q4nx.cpp:89) -- which is exactly why the writer stores - // {ntiles, chunk_bytes}: the last dimension IS the chunk kind. Giving it {rows, cols} instead makes - // chunk_of() see 5120 (Q4_1) and as_q4_1() copies the bytes through without repacking. + if (name.empty()) { check(false, std::string("external writer: the container carries ") + role); continue; } + // chunk_of() reads shape.back(), so the Tensor is shaped {ntiles, chunk_bytes} -- what the writer stores. onebit::npu::Tensor q4f; q4f.dtype = "I8"; - q4f.shape = {int64_t(qbytes / qtile), int64_t(qtile)}; - q4f.data = tile_raw.data(); - q4f.bytes = q4f.avail = uint64_t(tile_raw.size()); + q4f.shape = {int64_t(nbytes / chunk), int64_t(chunk)}; + std::vector raw(static_cast(nbytes)); + f.seekg(std::streamoff(8 + hlen + off)); + f.read(reinterpret_cast(raw.data()), std::streamsize(raw.size())); + q4f.data = raw.data(); + q4f.bytes = q4f.avail = nbytes; const std::vector conv = onebit::npu::q4nx::as_q4_1(q4f); - std::printf(" as_q4_1: %llu B -> %zu B (%zu tiles)\n", (unsigned long long)qbytes, conv.size(), - conv.size() / size_t(Q4NX_TILE_BYTES)); - // The writer pads the column count to a whole 256-column tile (5120 -> 6144), so the decode has to be - // told the PADDED grid: 160 row-tiles x 24 col-tiles = 3840 tiles, which is what as_q4_1() returned. - const int64_t q_cols_pad = (q_cols + 255) / 256 * 256; - std::printf(" decode grid: rows %lld cols %lld (padded from %lld); %zu tiles of %d\n", - (long long)q_rows, (long long)q_cols_pad, (long long)q_cols, - conv.size() / size_t(Q4NX_TILE_BYTES), int(Q4NX_TILE_BYTES)); - std::vector row(static_cast(q_cols_pad)); - const int rc = q4nx_dequant_tile_row(conv.data(), conv.size(), int(q_rows), int(q_cols_pad), - Q4NX_TILE_BYTES, 0, row.data()); - check(rc == 0, "external writer: the written tile decodes (got " + std::to_string(rc) + ")"); - if (rc == 0) { - for (int rr : {0, 32, int(q_rows - 1)}) { - std::vector r2(static_cast(q_cols_pad)); - const int r2c = q4nx_dequant_tile_row(conv.data(), conv.size(), int(q_rows), int(q_cols_pad), - Q4NX_TILE_BYTES, rr, r2.data()); - if (r2c == 0) - std::printf(" row %d: first %.4f mid %.4f\n", rr, double(r2[0]), double(r2[size_t(q_cols / 2)])); - else - std::printf(" row %d: refused (%d)\n", rr, r2c); - } - // Every row, not just row 0: with a uniform blob any slot the writer gets wrong lands in some - // row, and an arm that checks one row is blind to all the others (measured: a single doubled - // scale slot was caught by the sibling arms and missed by a row-0-only check). - size_t bad = 0; - int64_t bad_row = -1; - for (int64_t r = 0; r < q_rows; ++r) { - if (q4nx_dequant_tile_row(conv.data(), conv.size(), int(q_rows), int(q_cols), - Q4NX_TILE_BYTES, int(r), row.data()) != 0) { ++bad; continue; } - for (int64_t c = 0; c < q_cols; ++c) - if (std::fabs(row[size_t(c)] - 22.0f) > 1e-6f) { ++bad; if (bad_row < 0) bad_row = r; } - } - char msg[220]; - std::snprintf(msg, sizeof msg, - "external writer: every one of %lld rows decodes to 22.0 ((12 - 1) * 2.0); %zu differ, first bad row %lld", - (long long)q_rows, bad, (long long)bad_row); - check(bad == 0, msg); + const int64_t rows = heads * hd, cols = K; + std::vector row(static_cast(cols)); + size_t bad = 0; + int64_t bad_row = -1; + for (int64_t r = 0; r < rows; ++r) { + if (q4nx_dequant_tile_row(conv.data(), conv.size(), int(rows), int(cols), Q4NX_TILE_BYTES, + int(r), row.data()) != 0) { ++bad; continue; } + for (int64_t c = 0; c < cols; ++c) + if (std::fabs(row[size_t(c)] - 22.0f) > 1e-6f) { ++bad; if (bad_row < 0) bad_row = r; } } + std::printf(" writer success: %s %llu B in %llu B chunks -> %zu tiles, rows %lld\n", name.c_str(), + (unsigned long long)nbytes, (unsigned long long)chunk, + conv.size() / size_t(Q4NX_TILE_BYTES), (long long)rows); + char msg[240]; + std::snprintf(msg, sizeof msg, + "external writer: every one of %s %lld rows decodes to 22.0 ((12 - 1) * 2.0); %zu differ, first bad row %lld", + role, (long long)rows, bad, (long long)bad_row); + check(bad == 0, msg); } } } From 9bd83924c8b0aa01005e9c99c6fdb59189e19b4b Mon Sep 17 00:00:00 2001 From: bong-water-water-bong Date: Tue, 10 Nov 2026 10:33:07 -0400 Subject: [PATCH 5/5] WIP(1849): the arm asserts the SIGNED value (-10.0) and is red on engine#1932, as it should be I had "corrected" this expectation to 22.0 earlier tonight after reading every row decode to 22.0, on the inference that MatMulNBits codes are unsigned. That inference was wrong and it turned a live bug into the spec. engine#1932 is the actual cause: decode_external_tile() stores raw nibbles 0..15 in Q4::codes, and container_from_external_q4() pairs them with m = -zp*s, so the container computes v = c_u*s - zp*s = (c_u - zp)*s while the pack's fields are two's-complement, i.e. v = (sx(c) - sx(zp))*s Every code with the high bit set therefore comes back +16 codes high. At this fixture's s = 2.0 and c = 12 -> sx = -4, zp = 1: correct -10.0, observed 22.0, delta +32.0 = 16 codes x 2.0. So the expectation is restored to -10.0 and the arm is RED until #1932 lands (its fix is c_u = sx(c) + 8, z_c = (-8 - sx(zp))*s, which is exactly the identity #1874 already uses on the npu-pack path). Turning it green by asserting 22.0 again would be the failure this whole test exists to prevent. Measured now: q_proj/k_proj/v_proj each "26,214,400 differ" = every element of 5120 x 5120. --- tests/npu_pack_import_test.cpp | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/tests/npu_pack_import_test.cpp b/tests/npu_pack_import_test.cpp index 7d9403aa9..e7f7a78ad 100644 --- a/tests/npu_pack_import_test.cpp +++ b/tests/npu_pack_import_test.cpp @@ -464,11 +464,14 @@ void test_external_packed_genre(const std::string& root) { // The blob cannot be the all-0x22 the other fixtures write: a 68-byte stride is 64 B of codes then 4 B of zero // point and bf16 scale, so all-0x22 makes every weight (2 - 2) x s = 0 -- and an all-zero decode is exactly what a // broken writer produces too (engine#1871, engine#1875). These bytes decode to a value that pins all three parts: - // codes 0xCC -> nibble 12; zero point 0x11 -> four nibbles of 1; scale bf16 0x4000 = 2.0 - // w = (nibble - zp) * s = (12 - 1) * 2 = 22.0 - // MatMulNBits codes are UNSIGNED (the zero point shifts them), which is why 12 is 12 here and not int4 -4; - // measured, after the first version of this arm expected (12-12u)*... = -10.0 and read 22.0 from every row. - // Drop the codes and it reads 2.0, drop the zero point 24.0, drop the scale 11.0. + // codes 0xCC -> nibble 12 = two's-complement int4 -4; zero point 0x11 -> four nibbles of 1; scale bf16 + // 0x4000 = 2.0; the pack's fields are SIGNED, so w = (sx(c) - sx(zp)) * s = (-4 - 1) * 2 = -10.0. + // + // This arm reads 22.0 today, and that is engine#1932, not an arithmetic choice: decode_external_tile() keeps raw + // nibbles 0..15 and container_from_external_q4() pairs them with m = -zp*s, so every code with the high bit set + // comes back +16 codes high (+16*s = +4.0 at s=0.25; +32.0 at s=2.0 here: -10 -> 22). The expectation below is the + // CORRECT value, so this arm is RED until #1932's fix (c_u = sx(c) + 8, z_c = (-8 - sx(zp)) * s) lands. Do not + // "fix" the expectation to 22.0: that turns the bug into the spec. { const fs::path sdir = fs::path(root) / "external-success"; fs::remove_all(sdir); @@ -553,14 +556,14 @@ void test_external_packed_genre(const std::string& root) { if (q4nx_dequant_tile_row(conv.data(), conv.size(), int(rows), int(cols), Q4NX_TILE_BYTES, int(r), row.data()) != 0) { ++bad; continue; } for (int64_t c = 0; c < cols; ++c) - if (std::fabs(row[size_t(c)] - 22.0f) > 1e-6f) { ++bad; if (bad_row < 0) bad_row = r; } + if (std::fabs(row[size_t(c)] - (-10.0f)) > 1e-6f) { ++bad; if (bad_row < 0) bad_row = r; } } std::printf(" writer success: %s %llu B in %llu B chunks -> %zu tiles, rows %lld\n", name.c_str(), (unsigned long long)nbytes, (unsigned long long)chunk, conv.size() / size_t(Q4NX_TILE_BYTES), (long long)rows); char msg[240]; std::snprintf(msg, sizeof msg, - "external writer: every one of %s %lld rows decodes to 22.0 ((12 - 1) * 2.0); %zu differ, first bad row %lld", + "external writer: every one of %s %lld rows decodes to -10.0 ((sx(12) - sx(1)) * 2.0 = -10); %zu differ, first bad row %lld", role, (long long)rows, bad, (long long)bad_row); check(bad == 0, msg); }