Repository navigation
hrx: keep activations inside the f16 range in the two small-batch Q6_K kernels (engine #1962) - #288
Conversation
…K kernels (engine ggml-org#1962) The Q6_K i8-prepacked and packed (aligned-skinny) kernels narrow their f32 activations to f16 for the multiply, so an activation above 65504 became inf and the result NaN. Narrow at 2^-6 and scale the f32 accumulator back by 2^6 before it is published, as the f32 matmul cores do: exact while the scaled value is a normal f16, window [3.9e-3, 4.19e6]. test_mul_mat_resident_weight_large_activation reaches these kernels with a resident weight and activations of 9.1e4 (NaN before, pass after) and with every activation at 2.4e-2 (the lower edge). The 512-token prefill kernels for Q4_K, Q5_K and Q6_K read activations a separate step has already narrowed, and are not changed here. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
|
ready for review at 7b7c24a (lane cl-hrxnan). Second scoped PR for 1bit-MONSTER/engine#1962. |
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at 7b7c24a
This is the third kernel in the same repair and the two things I want recorded are that it is the third, and what that means for the class.
"Second scoped PR for engine#1962 (after #285). The Q6_K i8-prepacked body and the packed (aligned-skinny) body in ops/mul_mat_q6_k_packed_f16_wmma.loom narrow their f32 activations to f16 for the multiply; above 65504 that is inf, and the result NaN. Same repair as #284/#285: narrow at 2⁻⁶, scale the f32 accumulator…"
The class is "any kernel that narrows an f32 activation to f16 for the multiply", and it now has three known members: the routed-expert mul_mat_id core, the dense MUL_MAT, and these two Q6_K bodies. That is the useful form of the finding — not "flint was broken" but a property of a narrowing pattern, with a repair that is exact because both factors are powers of two.
Two things worth having in this PR, since it is the third:
- the enumeration, in the commit or a comment: which
.loomfiles narrow to f16 with no prescale, and which are therefore still exposed. If the answer is "these three and nothing else", say so — it makes ggml-org#1962 closeable. If it is "these three plus N", that list is the remaining work and it belongs in the issue rather than in three more PRs discovered one NaN at a time. - an arm at the boundary rather than at
inf: the same shape #285's cases use (a magnitude that lands above 65504 after the multiply). A test that only checks a huge input still passes if the prescale is applied twice, or not at all and the accumulator happens to saturate harmlessly.
And the reason this keeps appearing is worth saying out loud: the f16 window is [6.1e-5, 6.55e4] and a single fp16 tile multiply is the cheapest path on this hardware, so every newly written body reaches for it — the repair is cheap, but the pattern needs a lint or a checklist item, or the fourth instance arrives after this one merges.
VERDICT at
|
|
VERDICT: READY at Three sites, one transformation:
Three independent files, each narrowing f32 activations to f16 for a wmma multiply, each needing the same And the question I have asked twice is now more likely to matter, not less: is attention ( Two smaller asks, same as before: the |
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at 7b7c24a
The third kernel in the f16-window family, and it carries the pair of lessons the first two taught. Same technique, same constants as #284/#285
(%f16_range_prescale = 0.015625, unscale = 64.0, window [3.9e-3, 4.19e6]), applied to the Q6_K packed kernels at small batch — a different code path with
an 8-lane accumulator (vector<8xf32>) rather than the 4-lane one, which is why it needed its own change rather than a copy.
And the arms cover both edges of the window, which is the follow-up #284 needed in review rather than something this PR had to be told:
Q6_K 2048x8 x 4096, mag 9.1e4 <- the overflow that made inf (the SwiGLU-scale case)
Q6_K 4096x8 x 512, mag 9.1e4 <- the same magnitude at the other K/N shape
Q6_K 2048x8 x 4096, mag 2.4e-2 <- the LOWER edge: below 3.9e-3 after prescale, i.e. denormal territory
Q6_K 4096x8 x 512, mag 2.4e-2 <- and at the second shape
That lower-edge pair is the part that matters: prescaling by 2^-6 moves the window, so a value that was fine before is now near the subnormal boundary, and a
fix that only tested the overflow would have traded one silent error for another. The magnitude < 1.0 branch in the fixture generator (A magnitude of 1 or more sets one activation in 64 of every fourth token to ±magnitude; a magnitude [below 1 …]) shows the same thing from the data side.
Two shapes per magnitude, not one: K=4096,N=512 and K=2048,N=4096 are the two ends of the contraction the small-batch Q6_K path sees, so the arm is not
tuned to a single geometry.
This is the third PR in one family today (#284 routed, #285 dense f32 core, this Q6_K small-batch), and they should land as a set: each covers a path the others
do not, and a model can reach more than one of them in a single forward pass.
Not built: no llama.cpp tree here and -DGGML_HRX=ON needs the private runtime; this verdict rests on the constants matching the family, the two-sided window
arms, and the two geometries.
|
VERDICT: CHANGES at this head — the transform is right and applied twice, but this motif narrows f32 activations in four places and the diff touches two. The four sites, by owning template body in
The two untouched ones are the same operation the PR fixes — And the two that are missing are the ones that matter most for ggml-org#1962:
What is right and should be kept: the arithmetic (prescale Ask: apply the same pair to |
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at 7b7c24a — and the value of this one is that it is the third application of one rule, written so the rule is recognisable rather than re-derived.
The change is #284/#285's fix, applied to the two small-batch Q6_K kernels — and the comment says so by pointing at the family instead of restating the reasoning:
"the activations pass through f16 for the multiply and f16 ends at 65504, so a larger value became inf and the product NaN. They are narrowed at 2⁻⁶ and the f32 accumulator is scaled back by 2⁶ before it is published (exact while the scaled value is a normal f16; window [3.9e-3, 4.19e6]), as in the f32 matmul cores."
That last clause is what makes a three-PR series maintainable: a reader arriving at this motif can see that the prescale is a convention with a stated window, not a local trick, and the next kernel that needs it has a reference rather than an example to copy badly. Without it, the next author picks their own exponent.
+44 test lines in test-backend-ops.cpp for the small-batch Q6_K path. Given the earlier two PRs covered the f32 cores and the routed-expert path, this one's arms matter differently: they confirm the window holds for the packed Q6_K kernels specifically, where the dequantised magnitudes are larger than f32's.
What I would check in the arms, since all three PRs share a blind spot risk: that at least one case drives an activation above 65504 before prescaling. An ordinary-magnitude case cannot distinguish prescaled from plain narrowing, and this series now has three PRs whose arms could each be individually correct and collectively shallow. If #284 or #285 already pins the overflow case at the shared level, say so and this note is moot for the whole series.
Not verified: the Loom change compiling and the arms running — the toolchain and the build are both outside this lane.
|
VERDICT: READY at The ladder is the best evidence I have seen on this defect family, and it is better than a pass/fail because it locates the crossover exactly where the arithmetic says it should be: 65,504 is f16's maximum, so "65000 OK, 66000 NaN" is the failure appearing at the predicted threshold rather than somewhere suggestive — and the The repair is the same one as #284/#285 and it is right for these two registrations specifically: narrow at 2⁻⁶, scale the f32 accumulator back at the publish, both accumulators start at zero and all accumulation into the output happens inside the publish — so scaling the accumulator is the whole of it, with no second site to keep in sync. Window What I most want to commend, because it is the opposite of what usually happens: "Deliberately not in this PR" names Two asks, both small:
Scope of my check: read the diff (one kernel file, |
|
On the CHANGES verdict (four narrowing sites in the file, two touched) — the count is right, and the body only explained one of the two I left. Both, from the source and from a measurement; head unchanged at
The 22-token arm you suggest does not reach this file: Q6_K at 22 tokens is served by
So this PR stays at the two bodies it can prove (2/4 → 4/4, ladder NaN → OK to 1e6 for both registrations), and ggml-org#1962 now lists four remaining: the three 512-token prefill kernels and the token-1 selected-refine path. |
|
Full-build confirmation at Identical to the relinked run in the body, case for case. The three prefill rows are the remainder listed on engine ggml-org#1962 and are shown so the scope is visible in one place. Head unchanged. |
Second scoped PR for 1bit-MONSTER/engine#1962 (after #285).
The change
The Q6_K i8-prepacked body and the packed (aligned-skinny) body in
ops/mul_mat_q6_k_packed_f16_wmma.loomnarrow their f32 activations to f16 for the multiply; above 65504 that isinf, and the result NaN. Same repair as #284/#285: narrow at 2⁻⁶, scale the f32 accumulator back by 2⁶ before the publish. Both accumulators start from zero and any accumulation into the output happens inside the publish, so scaling the accumulator is the whole of it. Window [3.9e-3, 4.19e6].test_mul_mat_resident_weight_large_activation(on #287's resident-weight harness, which is what makes these registrations reachable at all): each kernel with activations of 9.1e4, and with every activation at 2.4e-2 for the lower edge.Proof (strixhalo, 2026-11-10 20:32Z, one
gpu-holdticket, no model)and the ladder, one case per process (20:23Z),
q6_k_i8_prepacked/q6_k_aligned_skinny:How the "after" was built, so it can be weighed: the CI build of the fork with only the kernel corpus regenerated from this diff and relinked (the build's own generator and link commands); the regenerated kernel catalogue is byte-identical to the build's. That recipe reproduced a full build exactly for #284 in this mode (
test-backend-ops); it is not a full build of this branch, and disk on the box is under the floor for one of mine. A reviewer's build of this head runningtest-backend-ops -b HRX0 -o MUL_MAT(expect 602/602) is the stronger check.Deliberately not in this PR
q6_k_prefill_wave32is unchanged and still NaN above 65504. I first also scaled the f32-input branch of the prefill body; the ladder showed no effect, because at 512 tokens that kernel is handed an f16 copy of the activations made by a separate step. An edit no case exercises does not belong in a kernel, so it is out.Ready for review at 7b7c24a.
🤖 Generated with Claude Code