Repository navigation
npu: the batched GDN path reads projection rows at the design's N (Refs #1786) - #2239
Conversation
gdn_layer_batch read row m of the fused q|k|v and of the gate projection at m * (model width); the projection returns rows at the design's N, the width rounded up to the kernel's N grid. Every shipped qwen3_5 pack is on that grid, so it was right by coincidence; off it, a batched prefill of two or more tokens left other logits and another state than decode. The fabricated qwen3_5 row is off-grid on purpose and is now a full comparison, not load only. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
|
ready for review at f22728b |
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT at f22728b: READY
It merges cleanly onto main. CI build at f22728b is green: 100% tests passed, 0 tests failed out of 254.
Code: gdn_layer_batch() now reads row m of the q|k|v and gate projections at designs_[i_qkv].N / designs_[i_z].N, the width npu_gemm_rows() actually returns rows at (out.assign(M * N)). A short projection is refused by name. I checked the rest of the function's per-row reads:
AHS/BHSare filled on the host atNVHper row;COREandxnare host buffers.- The
ssm_outresult is consumed bystep_batch's shared residual add atq4nx_pad_n(H), which is why the fixture's H 256 (also off the grid) is right.
So the q|k|v and the gate were the only two reads at the wrong width.
Checked at this head (ONEBIT_NPU=ON ONEBIT_NPU_HOST=ON build):
tests/batch_check_state_ci.py ./batch_check_state_test qwen3_5:PASS last chunk's logits vs the sequential reference: rel-L1 0.00e+00,ALL PASS.- The same script with
lfm2:ALL PASS. - Mutant (the q|k|v stride back to
CD, the gate left fixed): rc 1,FAIL … rel-L1 7.84e-01. That is the body's 7.10e-01 shape, with the q|k|v alone enough to break it. After restoring the file the tree is clean. - On-grid real pack (
gdn_batch_order_test, host GEMM, FastFlowLMQwen3.5-2B-NPU2):PASS … 18 GDN layer pass(es) … bit for bit. A pack whose widths are on the grid is unchanged, as the body says.
Turning the load-only batch_check_state_qwen35_load row into the full batch_check_state_qwen35 comparison is the right follow-through: the off-grid fixture is what found this.
Refs #1786 (and #1889, closed by #2218, where this was found).
gdn_layer_batch()read rowmof the fused q|k|v projection atm * (2*KD + VD)and rowmof the gate projection atm * VD.npu_gemm_rows()returns rows at the design's N, which is the width rounded up to the kernel's N grid (512). Every shipped qwen3_5 pack has both widths on that grid, so the two numbers coincide and the code was right by accident. On a geometry off the grid, every row after the first was read from the wrong place. A batched prefill of two or more tokens then left other logits and another conv/recurrent state than token-by-token decode.The fix
Rows are read at
designs_[i_qkv].Nanddesigns_[i_z].N. A projection that returns fewer thanM * Nvalues is refused by name instead of being read past. That is 23 lines innpu/forward/src/q4nx_forward.cpp.Found by, and pinned by, the fabricated qwen3_5 row
#2218 added
batch_check_state_qwen35_load: a qwen3_5 hybrid thattests/batch_check_state_ci.pywrites at test time. It has a linear-attention layer, then attention, and is deliberately off the N grid: the q|k|v is 768 wide (design N 1024) and the gate is 256 (design N 512). It was load only, because its full comparison failed and I could not say why. This is why. The row is now a full batched-against-sequential comparison, namedbatch_check_state_qwen35, and the load-only switch inbatch_check_state_test.cppis removed.How it was localised, with a scratch diagnostic that compares the two paths directly, without the
NPU_BATCH_CHECKoracle. It reports logits and the conv and recurrent planes after each chunk:Before the fix, the comparison was exact with attention layers only and wrong with GDN layers only (0.45).
NPU_GDN_CHECK's two internal comparisons were identical, because both arms read the same mis-strided rows.On real packs: unchanged
The same reply hash,
57b0578884, came out of the same row before the fix (ata7bbf94's parent code). So on an on-grid pack the output is byte-identical, as it should be.Checks
ctest -j 6 --timeout 200: 241 of 243 pass. The two that do not arenpu_rt_shimandnpu_dmabuf_import, which do not link in my build directory on any branch.lfm2and qwen3 variants ofbatch_check_state_ci.pypass unchanged.tools/copyright.py --checkrc 0.git diff --stat origin/main: 4 files, +25/−24.🤖 Generated with Claude Code