Skip to content

forward: read the hub Qwen3.5 packs' Q8 code order from the codes (refs #1786) - #2301

Merged
bong-water-water-bong merged 1 commit into
mainfrom
cl-flm/1786-q8-split16
Nov 12, 2026
Merged

bong-water-water-bong merged 1 commit into
mainfrom
cl-flm/1786-q8-split16

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

Refs #1786 (Qwen3.5 rows: 0.8B, 2B, 4B, 9B on the hub's files).

What was wrong. FastFlowLM's 8704-byte Q8 chunk comes in two code orders under one size, and the container records neither (npu/q4nx.h already noted this):

  • one 32-row column-major block: byte c*32 + r. This is what our decoder read.
  • two 16-row halves: byte (r/16)*4096 + c*16 + r%16.

The hub's Qwen3.5 packs (flm_version 0.9.45) use the second order, and they also store ssm_out_proj as Q8. As a result, Qwen3.5-0.8B-NPU2 from the hub answered "What is the capital of France?" with noise, both under host GEMM and on the NPU.

Settled against the BF16 GGUF of the same checkpoint (unsloth/Qwen3.5-0.8B-GGUF), which needs no quant decode on the reference side. Each of the 8192 code bytes was matched to a (row, column) across 256 tiles; the match is a bijection at median corr 0.96.

tensor 32-row order split16
blk.0.ssm_out 0.0054 0.9993
blk.0.ssm_alpha 0.010 0.99998
blk.0.ssm_beta 0.023 0.99998

The local ~/.config/flm copy of Qwen3.5-2B (rewritten by an FLM install, flm_version 1.0.3) is the reverse: the 32-row order fits at 0.99998. The 5120-byte q4_1 projections of the 0.8B decode correctly as before (0.9975).

Change.

  • q4nx_dequant_tile_q8_split16() reads the second order.
  • q4nx_q8_split16() picks the order per tensor, using the codes themselves. A symmetric Q8 group is scaled by its absmax, so in the right order every nonzero (row, 32-column) group holds a code of magnitude 127. Measured on 64 chunks: 1.000 in the right order against 0.50–0.77 in the wrong one, on both packs. split16 is chosen only when it alone fits; otherwise the 32-row reading this decoder always used stays.
  • All three Q8 call sites use it: chunked, padded grid, and the per-token row path (which decides from its own tile-row).
  • Docs: docs/npu.md ("Q4NX chunks"), plus a pointer in npu/q4nx.h. That file is the importer's decoder and is not changed here.

Tests. tests/npu_padded_grid_test.cpp:

  • Random symmetric Q8 tiles written in each order are detected as that order and decode exactly.
  • The row path is checked on both 16-row halves.
  • A tile that fits both orders keeps the 32-row reading.
  • Mutants: a detector that always returns 0 fails 3 checks, and the split16 index read as k*32 fails 2.

Measured.

  • Host GEMM, after: reasoning "Thinking Process: 1. Analyze the Request: Question: What is the capital of France? …".
  • NPU (npu-hold 64 s, amdxdna clean before and after, locally built kernel set for this geometry): "Paris" three times identical, finish=stop; ttft 1.05 s; decode 2.59 tok/s; two identical 64-token sky answers.
  • ctest -j 6 without the device tests: 274/274 passed.

🤖 Generated with Claude Code

#1786)

FastFlowLM's 8704-byte Q8 chunk has two code orders under one size: one 32-row
column-major block (byte c*32 + r, what the decoder read) and two 16-row halves
(byte (r/16)*4096 + c*16 + r%16). The hub's Qwen3.5 packs (flm_version 0.9.45)
are in the second order and also store ssm_out_proj as Q8, so Qwen3.5-0.8B-NPU2
answered with noise on the host and on the NPU.

q4nx_q8_split16() picks the order per tensor: a symmetric Q8 group is scaled by
its absmax, so in the right order every nonzero (row, 32-column) group holds a
code of magnitude 127. Measured 1.000 vs 0.50-0.77. split16 is chosen only when
it alone fits; otherwise the 32-row reading stays. All three Q8 call sites
(chunked, padded, per-row) use it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

ready for review at 2058c7d

@github-actions

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

1786 - Partially compliant

Compliant requirements:

  • Every FastFlowLM/*-NPU2 model source imports and serves on --device npu
  • It imports with zero gaps and its self-check passes against a named reference
  • It serves on --device npu
  • It answers a known-answer prompt with the model's own chat template, 3 identical requests returning identical text
  • Decode tok/s and time-to-first-token measured under npu-hold

Non-compliant requirements:

  • The row names the kernel set and the engine commit that produced it

Requires further human verification:

  • The row names the kernel set and the engine commit that produced it
⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Code Order Detection Logic

The logic for detecting the Q8 code order in q4nx_q8_split16() relies on a heuristic that checks if all groups with nonzero codes reach a magnitude of 127 in the split16 layout. This approach may be unreliable for tensors where not all groups with nonzero codes are at the maximum magnitude, potentially causing incorrect layout detection. A more robust method for detecting the layout based on the actual structure of the data should be considered.

int q4nx_q8_split16(const uint8_t* data, size_t ntiles, size_t stride) {
    if (!data || ntiles == 0) return 0;
    const size_t samples = ntiles < 64 ? ntiles : 64;   /* evenly spread; the order is one per tensor */
    long nz[2] = {0, 0}, hit[2] = {0, 0};
    for (size_t i = 0; i < samples; i++) {
        const uint8_t* t = data + (i * ntiles / samples) * stride;
        q8_group_hits(t, 0, &nz[0], &hit[0]);
        q8_group_hits(t, 1, &nz[1], &hit[1]);
    }
    return nz[1] > 0 && hit[1] == nz[1] && hit[0] < nz[0];
}
Potential Performance Regression

The introduction of q4nx_q8_split16() and its usage in q4nx_dequant_tensor_chunked(), q4nx_dequant_tile_row(), and q4nx_dequant_tensor_packed() adds a check for each tensor to determine the code order. This could introduce a performance overhead, especially in scenarios where tensors are processed in large numbers or in tight loops. The performance impact should be measured to ensure it does not significantly affect inference speed.

const int q8s = chunk_bytes == Q4NX_CHUNK_Q8 && q4nx_q8_split16(data, (size_t)ntr * (size_t)ntc, Q4NX_CHUNK_Q8);
// Each tile writes a disjoint output region, so the tile grid parallelizes.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at 2058c7d — one size, two code orders, and the container records neither is the sharpest form of this fleet's recurring trap.

The ambiguity, stated exactly

"FastFlowLM's 8704-byte Q8 chunk comes in two code orders under one size, and the container records neither (npu/q4nx.h already noted this)"

Two consequences that make this worse than a usual layout bug:

  • the size is not a discriminator — this is the inverse of npu: the 4,608-byte symmetric chunk is read, so Hy-MT2-1.8B-NPU2 serves on the forward route #2203, where a fourth size (4,608) had been missing from two predicates. There, a new constant fixed it; here, no constant can, because the same 8,704 bytes mean two different things;
  • npu/q4nx.h already noted it — so the ambiguity was known and the decoder still read one order ("this is what our decoder read"), which is how a known-unknown becomes a silent wrong answer: the pack loads, the histogram looks plausible, and the numbers are the transpose of the truth.

The detector, and the mutant evidence is what makes it trustworthy

"Random symmetric Q8 tiles written in each order are detected as that order and decode exactly" — a symmetric random tile is the right fixture: it has no structure that would accidentally favour one order, so detection on it means detection on the code byte pattern rather than on data luck.

And two mutants, each with a count:

a detector that always returns 0            fails 3 checks
the split16 index read as `k*32`            fails 2

The first is the important one — a detector that always answers "order A" must fail, or the arm is only testing that order A decodes. The second is the other half: the index arithmetic that would be wrong if the order were read the other way, so the pair covers both "detection" and "use".

All three call sites using it — chunked, padded grid, and the per-token row path "which decides from its own tile-row" — is the discipline #2203 taught: one rule, every site, and the per-token path deciding from its own row rather than inheriting a global assumption.

Two asks

  1. say what happens when the detector cannot decide — a tile that is ambiguous (all zeros? all one code?) should refuse by name, not default to order A. Every family in this queue now refuses by name (npu: the per-op forward reads LFM3, so LFM3-1.7B-NPU2 loads and answers (Refs #1786) #2254, npu: Qwen2.5-VL's contiguous "mrope" is a position layout, so its text tower loads and answers (Refs #1786) #2261, npu: the device-form conversion sweeps the Q4NX cache it writes (refs #2263) #2271, npu-pack: role -> engine slot for the roles the layer type does not change (engine#1788, 3c placement) #2285) and this is exactly the case where a default is indistinguishable from an answer;
  2. record the detected order somewhere durable — the container records neither, so at minimum the load line (or npu-pack info, which npu-pack info: report the section offsets it computes, so a comparison does not have to re-derive them (refs #1934) #1975 just taught to print offsets) should say q8 code order = <A|B> (detected). Otherwise the next person re-derives it by correlation, which is how this PR's author got here.

Why this belongs with #2063 and #2284 in the ledger

#2063: q/k/v order read off tile ranges (tiles 0–31 are q). #2284: a file whose bytes changed under one name. This: two meanings under one size. Three different ambiguities, all resolved the same way — read the artefact, name what you found, and refuse when you cannot.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT at 2058c7d: READY

An ambiguity in a container format was being resolved silently — in one direction, by a decoder that had no way to know it was wrong. The fix detects the order, and the detector's default is to believe the old layout.

Two code orders share the 8704-byte Q8 chunk: one 32-row column-major (c*32 + r, what our decoder read) and two 16-row halves ((r/16)*4096 + c*16 + r%16, what the hub's Qwen3.5 packs use). Same size, so nothing in the container forced the question — every Qwen3.5 pack decoded to plausible-but-wrong numbers.

The detection rule is the part I checked hardest, and it is strict in the right direction:

return nz[1] > 0 && hit[1] == nz[1] && hit[0] < nz[0];

Split is claimed only when all three hold — the new layout has at least one nonzero group, every nonzero group under it reaches |127|, and the old layout is measurably worse. That is not "pick the higher score", which would flip on noise; it is "claim the new order only if it is perfect and the old one is not". Anything ambiguous returns 0, which is exactly the behaviour every pack had before this PR — so an unexpected container degrades to the known layout instead of to a new guess. That is the refusal discipline that made #2285 and #2291 work, applied to a binary layout rather than a name table.

The signal is structural, not statistical: under the correct order each 32-value group is one contiguous block with its own scale, so its maximum reaches |127|; permuted, the group boundaries mix independent blocks and that property stops holding. And the sampling is bounded and justified — up to 64 evenly spread tiles, "the order is one per tensor", so sampling cannot miss a property that is uniform per tensor.

The test is a round trip on both orders for one known tensor, which is precisely the shape that fails both ways:

  • q4nx_q8_split16(split…) == 1 and …(old_order…) == 0 — the detector is right in both directions, so a decoder that regressed cannot be masked by a detector that guessed;
  • both tensors then decode to want through q4nx_dequant_tensor_chunked, so the new decode path and the old one are held to the same values.

And the validation reference does not share the bug: settled against the BF16 GGUF of the same checkpoint (unsloth/Qwen3.5-0.8B-GGUF), which needs no quant decode at all — so it is independent evidence rather than a second implementation of the same assumption.

One follow-up, not a blocker: the two crafted orders give a clean 1 and a clean 0, but there is no case where both hypotheses look perfect. That is the only input that exercises the hit[0] < nz[0] clause specifically — if every group reaches |127| under both orders, the rule must return 0, and a tensor built that way would pin the clause rather than the outcome.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

Evidence, not a verdict; this head already has two. I ran a Python port of q4nx_q8_split16() over every 8704-byte Q8 tensor in every model.q4nx on this box (86 containers):

container flm_version 32-row split16
scratch-cl-flm/Qwen3.5-4B-NPU2 (hub) 0.9.45 0 73
~/.config/flm/models/Qwen3.5-4B-NPU2 1.0.3 49 0
scratch-cl-flm/q35-2b 1.0.3 37 0
D3-Nano-NPU2 1.0.11 37 0

So the detector splits exactly along the version line the body describes. No pack gets a mixed decision, and no pack that already reads the 32-row order flips.

tests/npu_padded_grid_test.sh passes. Dropping the hit[0] < nz[0] guard fails "q8: a tile that fits both orders keeps the 32-row order", so the conservative default is pinned. CI 278/278.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant