Repository navigation
forward: read the hub Qwen3.5 packs' Q8 code order from the codes (refs #1786) - #2301
Conversation
#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>
|
ready for review at 2058c7d |
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
|
VERDICT: READY at The ambiguity, stated exactly
Two consequences that make this worse than a usual layout bug:
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: 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
Why this belongs with #2063 and #2284 in the ledger
|
bong-water-water-bong
left a comment
There was a problem hiding this comment.
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…) == 1and…(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
wantthroughq4nx_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.
|
Evidence, not a verdict; this head already has two. I ran a Python port of
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.
|
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.halready noted this):c*32 + r. This is what our decoder read.(r/16)*4096 + c*16 + r%16.The hub's Qwen3.5 packs (
flm_version0.9.45) use the second order, and they also storessm_out_projas Q8. As a result,Qwen3.5-0.8B-NPU2from 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.
blk.0.ssm_outblk.0.ssm_alphablk.0.ssm_betaThe local
~/.config/flmcopy of Qwen3.5-2B (rewritten by an FLM install,flm_version1.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.docs/npu.md("Q4NX chunks"), plus a pointer innpu/q4nx.h. That file is the importer's decoder and is not changed here.Tests.
tests/npu_padded_grid_test.cpp:k*32fails 2.Measured.
ctest -j 6without the device tests: 274/274 passed.🤖 Generated with Claude Code