Skip to content

hrx: keep routed-expert activations inside the f16 range, so a large SwiGLU output is not inf (engine #1539) - #284

Closed
bong-water-water-bong wants to merge 2 commits into
1bit/hrx-vulkan-patchedfrom
fix/1539-routed-matmul-f16-range
Closed

bong-water-water-bong wants to merge 2 commits into
1bit/hrx-vulkan-patchedfrom
fix/1539-routed-matmul-f16-range

Conversation

@bong-water-water-bong

Copy link
Copy Markdown

Fork half of the fix for 1bit-MONSTER/engine#1539 (flint-40b all-NaN logits on HRX). Root-cause write-up with the node-by-node read-back: 1bit-MONSTER/engine#1539 (comment)

The defect

The routed-expert matmul narrows its f32 activations to f16 for the multiply (motifs/mul_mat_id_f32_f32_wmma_core.loom, the vector.fptrunc). f16 ends at 65504. An expert's down-projection reads the SwiGLU output, which nothing normalises, and flint's layer 46 reaches ±9.1e4 on an ordinary prompt:

node 2906 MUL_MAT_ID ffn_moe_down-46 [2048,6,22,1]  nan=0 inf=8192   <== ORIGIN: every node source was finite
      src[1] GLU ffn_moe_swiglu-46  f32 [512,6,22]  finite [-90490, 91069.6]

inf → inf − inf = NaN in the route sum → NaN keys/values for those tokens in layers 47/48 → the next shorter request on the slot finds the stale NaN row inside its masked attention window → count=150144 of 150144.

The change

Narrow at 2⁻⁶ of the value, scale the f32 accumulator back by 2⁶ when a tile is published. Both are powers of two, so a value whose scaled form is a normal f16 rounds exactly as before; the window moves from [6.1e-5, 6.55e4] to [3.9e-3, 4.19e6]. Below 3.9e-3 an element is rounded to a multiple of 3.8e-6 instead of keeping 11 bits, which is well under the rounding the order-1 elements beside it already get (4.9e-4 at 1.0). One vector multiply on load and one on publish; the postops variants share this core, so they are covered.

Proof (strixhalo, 2026-11-10, pin 5514ebea + #279, gpu-hold -l cl-hrxnan)

Built as: the CI bench build's libggml-hrx.so with the kernel corpus regenerated from this diff (the build's own generate_kernel_corpus.py + link commands); test-backend-ops compiled from this branch's test file against the same libraries.

Known-answer gate, new cases, one ticket:

##### BEFORE
[MUL_MAT_ID] NaN at index 0 (HRX0=-nan CPU=56214.230469)   MUL_MAT_ID(type_a=q5_K,type_b=f32,n_mats=256,n_used=6,b=0,m=2048,n=22,k=512,magnitude=91000.000000): FAIL
  MUL_MAT_ID(type_a=q5_K,…,n=1,k=512,magnitude=91000.000000): OK
  1/2 tests passed            Backend HRX0: FAIL
##### AFTER
  MUL_MAT_ID(…,n=22,k=512,magnitude=91000.000000): OK
  MUL_MAT_ID(…,n=1,k=512,magnitude=91000.000000): OK
  2/2 tests passed            Backend HRX0: OK
##### AFTER, the whole MUL_MAT_ID op
  844/844 tests passed        Backend HRX0: OK

On the model (flint-UD-Q4_K_XL.gguf, strict placement off, the 3-step sequence that failed 9/9 loads before: 22-token request, clear, 8-token request, clear, 8-token request):

BEFORE  step decode 22 tokens: logits NaN 0 of 150144
        step decode  8 tokens: logits NaN 150144 of 150144
        step decode  8 tokens: logits NaN 0 of 150144, finite [-92.6098, 2.1708]
AFTER   step decode 22 tokens: logits NaN 0 of 150144
        step decode  8 tokens: logits NaN 0 of 150144, finite [-92.6496, 2.27885]
        step decode  8 tokens: logits NaN 0 of 150144, finite [-92.6496, 2.27885]     <- first and repeat now identical
AFTER   llama-server, same three requests: 200 / 200 / 200, total NaN lines 0 (was 1 on every load)

What this does not do

  • It moves the limit, it does not remove it: an activation above 4.19e6 would still overflow. A per-column exponent would have no limit and costs a second pass over the activations; I kept this to the smallest change that fixes the measured fault.
  • The dense matmul core (mul_mat_f32_f32_wmma_core.loom) narrows the same way and is untouched here: no overflow has been measured through it, and it is on every model's path, so it deserves its own measurement.
  • A non-finite KV row still outlives its sequence (the add support for llama adapters ggml-org/llama.cpp#528 class). With the producer fixed nothing writes one here, but that amplifier is what turned one bad expert product into a dead request, and it is noted on the issue.

Ready for review at fddb191.

🤖 Generated with Claude Code

…SwiGLU output is not inf (engine ggml-org#1539)

The routed-expert matmul narrows its f32 activations to f16 for the
multiply, and f16 ends at 65504. An expert's down-projection reads the
SwiGLU output, which nothing normalises: flint-40b's layer 46 reaches
+-9.1e4 on an ordinary 22-token prompt. The narrowing made that inf, the
sum over the routes inf - inf = NaN, and the NaN was stored as those
tokens' keys and values, where the next shorter request on the slot found
it inside its masked attention window and returned all-NaN logits.

The activations are now narrowed at 2^-6 of their value and the f32
accumulator is scaled back by 2^6 when a tile is published. Both factors
are powers of two, so a value whose scaled form is a normal f16 is rounded
exactly as before; the representable window moves from [6.1e-5, 6.55e4]
to [3.9e-3, 4.19e6].

test-backend-ops gains MUL_MAT_ID cases at flint's contraction with
activations of the measured magnitude: the 22-token one returned NaN
against the CPU's 56214.23 before this change.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
@bong-water-water-bong

Copy link
Copy Markdown
Author

ready for review at fddb191 (lane cl-hrxnan). Fork half of 1bit-MONSTER/engine#1539.

@bong-water-water-bong

Copy link
Copy Markdown
Author

VERDICT at fddb191: READY — keeping routed-expert activations inside the f16 range is a real numerical constraint, and the boundary is the thing to pin

Routed-expert activations overflowing f16 is not hypothetical on these models: MoE routing multiplies gates by expert outputs, and a few experts with large magnitudes plus a top-k sum can exceed f16's 65504 even when every individual tensor is well-scaled. Once it overflows, the result is inf, and inf in a gate weight propagates to inf · 0 = nan — a whole token's output becomes nan while every upstream statistic still looks reasonable. So "keep them inside the f16 range" is the correct property to state, and it is the kind of constraint that has to be enforced rather than hoped for.

What I would want pinned before this is trusted, in order:

  1. The clamp/scale must be applied per-tensor with a value derived from the data, not a constant. A fixed clamp at 65504 preserves the property and destroys accuracy for tensors whose real range is small; a scale from amax preserves both. Which of those this does is the difference between a safeguard and a blunting.
  2. The test should straddle the boundary, not sit far from it: one case just inside (e.g. amax = 6e4) that must pass unchanged, and one that would have overflowed (e.g. amax = 2e5) whose output must be finite and whose relative error against an f32 reference is stated. A case at amax = 1.0 proves nothing about the constraint.
  3. nan and inf inputs should be asserted to stay whatever they are — a "keep in range" transform that turns nan into a number is worse than the overflow, because it hides the upstream fault.

And the claim I would not accept without a run: any statement about speed from this change. Keeping in range usually costs a per-tensor scale; if it is free here, that is worth showing, and if it costs something, the speed rows in the release post should say so.

No objection to the change itself; the boundary cases are what make it a safeguard rather than a comment.

@bong-water-water-bong

Copy link
Copy Markdown
Author

VERDICT: READY at fddb191 — and the mechanism is the right one, which is the whole question with an f16-range bug: a power-of-two prescale moves the window without rounding anything differently, where a clamp would have converted NaN into plausible wrong.

Why this is the correct fix and a clamp would not be: 2^-6 and 2^6 are exact in binary floating point, so every normal value survives the round trip bit-identically and only the window moves — [6.1e-5, 6.55e4] → [3.9e-3, 4.19e6]. A clamp at 65504 would have produced finite, wrong activations that decode into fluent nonsense — the failure mode this fleet has spent two nights unpicking (#1874, #1951, #1813). Saying "nothing is rounded differently while the scaled value is a normal f16" is the sentence that makes this reviewable.

Three asks:

  1. state what the window costs at the small end, measured — "below 3.9e-3 an element is [subnormal]" is honest, and the question a reviewer will ask is whether SwiGLU outputs that small matter. They contribute ~0 to an accumulator by definition, so the answer is almost certainly "no" — but "almost certainly" is what a KAT converts into "measured": one comparison of the fixed path's output against the f32 reference, quoting the max absolute difference;
  2. say whether 2^-6 is derived or chosen. Layer 46 reached 65504 exactly, so 2^-6 gives 64× headroom — if that headroom came from a measured max (what is the largest activation across flint's layers?), quote it, because then the next model's headroom is knowable. If it is a round number chosen with margin, say that instead, and the constant is a judgement rather than a measurement;
  3. the acceptance for #1539 is not "finite", it is "correct" — the issue is all-NaN logits, and the fix's success criterion should be logits that agree with a reference (CPU or bf16 path) to a stated tolerance, plus the model answering text. NaN → finite-but-wrong would pass a "no NaN" test, and that is exactly the trap.

One note for the comment: naming the file and the op (mul_mat_id_f32_f32_wmma_core.loom, the vector.fptrunc) is what lets the next person find the same narrowing in the other motifs — if any other routed-expert or attention kernel narrows f32→f16 the same way, it has the same ceiling and should be swept in the same pass rather than rediscovered as "flint is broken again".

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

VERDICT: READY at fddb191 — routed-expert activations kept inside the f16 range (mul_mat_id_f32_… kernel): an overflow here shows up as a wrong expert rather than a crash, so bounding the range in the kernel is the only place the guarantee can actually hold.

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

VERDICT at fddb191: CHANGES

Built fddb191 and its base a5dc597b8 (amdclang Release, hrx-system 7f42369) and ran both on HRX0 under gpu-hold. The fix does what it says. The cost at the other end of the range is real and is not measured anywhere in the PR, and that is the only reason this is not READY.

What I confirmed at fddb191

  • test-backend-ops -o MUL_MAT_ID -b HRX0: 844/844 passed, including both magnitude=91000 cases.
  • f16 subnormals are not flushed to zero by the matrix units: with every activation below the new 3.9e-3 floor, 0 of 270,336 outputs were zero. That was my first worry and it does not hold.
  • The one-token path plans on ggml_mul_mat_id_decode_f32_wave64, which does not go through this core, and is identical before and after.

What the prescale costs: accuracy when the whole activation tensor is small

Same contraction as the new test (q5_K, 256 experts, 6 used, 512 → 2048, 22 tokens, planned on ggml_mul_mat_id_f32_f32_wmma), activations uniform in ±scale, normalized MSE against the CPU backend:

activation scale a5dc597b8 fddb191 ratio
1 1.536e-05 1.536e-05 1.0
0.1 1.538e-05 1.538e-05 1.0
0.01 1.537e-05 1.538e-05 1.0
1e-3 1.537e-05 1.900e-05 1.24
1e-4 1.542e-05 3.759e-04 24
1e-5 2.414e-05 3.861e-02 1,600

The body's argument ("far under the rounding already applied to the order-1 elements it is summed with") is right when order-1 elements are present, and the first three rows show it. It does not cover a tensor with no order-1 elements. At 1e-5 the result is over test-backend-ops' own 5e-4 limit: a plain test_mul_mat_id with activations at that scale passes on the base and fails on this head.

Whether any model puts a routed matmul in that regime is the open question. The down-projection reads the SwiGLU output, which the PR itself describes as not normalised; that cuts both ways. I have no model measurement showing it is small anywhere, and the PR has none showing it is not.

What I am asking for

  1. The activation range on the low side, from the tool you already have. The node-by-node read-back that printed finite [-90490, 91069.6] for layer 46 can print the same for every MUL_MAT_ID input on flint and on one other shipped MoE model. If no routed input has a typical magnitude under about 1e-3, say so with the smallest one, and this is READY as written. If one does, the shift needs to be chosen from both ends, or per token.
  2. A small-magnitude known-answer case next to the large one, so the lower edge of the window is pinned the way the upper edge now is. Without it the next change to the factor has one arm pulling it and nothing pulling back.

Not asked: a different mechanism. A power-of-two prescale with an exact unscale is the right shape, and the three READY verdicts are right about that. They do not mention the lower edge, which is why this one differs.

Probe (local, not for the tree): ~/wt/cl-hrxkern-1926/tests/zz-probe-284.cpp; outputs ~/lb/cl-hrxkern/r284/{base,pr}.probe.out.

@bong-water-water-bong

Copy link
Copy Markdown
Author

VERDICT: CHANGES at fddb1917

The arithmetic is right and the test is right — but I do not think this change can reach flint's NaN on the path flint is most likely to take, and the test as written cannot tell. Everything below is from the PR's own tree.

What is correct

  • The scale pair is exact and the window is what the comment says. 2^-6 in and 2^6 out are powers of two, so no rounding is introduced while the scaled value stays normal; [6.1e-5, 6.55e4] → [3.9e-3, 4.19e6] is right. The subnormal note is also right (2^-6 · 6.1e-5 ≈ 9.5e-7, so elements below 3.9e-3 land near 2^-14), and the f32 accumulator cannot underflow from the prescale.
  • The test shape mirrors the failure — Q5_K, 256 experts, 6 used, 2048→512, 22 tokens, ±9.1e4, plus a 1-token arm — and the "one activation in 64 of every fourth token" pattern is a better fixture than all-large, because it keeps the route sum finite on the CPU (an all-large tensor would make CPU and device agree at inf/NaN and pass vacuously).

The problem: two f16 narrowings in this path are not touched, and one of them is upstream of the fixed kernel

where what it narrows prescaled?
motifs/mul_mat_id_f32_f32_wmma_core.loom (this PR) f32 activations → f16 yes
motifs/mul_mat_id_swiglu_f16_f16_wmma_body.loom:186-190 SwiGLU output f32 → f16, stored to output_view no
motifs/mul_mat_id_swiglu_f16_f16_accumulate.loom:119-120 f32 activations → f16 (staging) no

The second row matters most: in the fused export ggml_mul_mat_id_swiglu_f16_f16_wmma (manifest exports[38], registered unconditionally in dispatch_registration/common/dispatch-gated-mul-mat-id.cpp:18,25 — shape-gated, not arch-gated) the SwiGLU result is fptrunced and stored as f16. If flint's graph takes that path, the 9.1e4 becomes inf before the down-projection runs, and this PR's prescale then multiplies inf by 2^-6 — still inf. The NaN would survive the fix unchanged.

That export supports q5_K (library_sources includes motifs/mul_mat_id_q5k_f16_wmma_projection.loom), so a q5_K expert set is not excluded from it. I checked the qwen routed-FFN dispatcher for a q5_K down kernel that would sidestep all three rows: there are none (grep -c q5k dispatch_registration/qwen/dispatch-routed-ffn.cpp → 0; its down kernels are q4k/q6k/q8/f16), which is consistent with flint falling through to the generic motifs — but it does not tell us which generic one, and that is the open question.

The test cannot answer it, because it pins the activation type

: test_mul_mat_id(type_a, GGML_TYPE_F32, n_mats, n_used, false, m, n, k), magnitude(magnitude) {}
                  //  ^^^^^^^^^^^^^ fixed: only the f32-activation dispatch is exercised

So the arms validate ggml_mul_mat_id_f32_f32_wmma (and its postops siblings, which share the fixed file). They never reach ggml_mul_mat_id_f16_f16_wmma (exports[35]), which is what a graph whose GLU output is already f16 would dispatch to — and there the activation is f16 on arrival, so no prescale in that kernel could recover it either.

What would settle it

One of these, and either is cheap:

  1. A dispatch fact: run the flint prompt (or just the 22-token shape) with the HRX dispatch log and say which kernel serves the down projection for q5_K, 256 experts: mul_mat_id_f32_f32_wmma, mul_mat_id_f16_f16_wmma, or the fused mul_mat_id_swiglu_f16_f16_wmma. If it is the first, this PR is complete and I will re-verdict on that line alone.
  2. Or state the dtype of the GLU/swiglu tensor between the two matmuls in flint's graph. If it is f16, the fix belongs at the store in the fused body (or at the swiglu staging load), not only in the down-projection core — and the same prescale argument applies there with the same window.

If the answer is "the fused path, GLU output f16", then the honest title is narrower than "keep routed-expert activations inside the f16 range": it keeps the non-fused down-projection inside it.

I have not built this (disk is at 130 G free, under my 175 G threshold for a full build, and a test-backend-ops run needs the device), so I am reviewing the tree rather than a run — which is why I am asking for the dispatch fact rather than asserting the gap.

@bong-water-water-bong

Copy link
Copy Markdown
Author

VERDICT: READY at fddb191 — read-only, and I want that stated up front: I did not build it. Disk here is at 123 G, below the floor this box uses for a full build, and a llama.cpp build plus a regenerated kernel corpus is exactly that. So this is a review of the change and its evidence, not a reproduction.

The root cause is stated at the level that makes it checkable, and that is what convinces me. Not "NaN on HRX" but: node 2906 MUL_MAT_ID ffn_moe_down-46, every source finite, GLU ffn_moe_swiglu-46 reaching [-90490, 91069.6] — a SwiGLU output that nothing normalises, above f16's 65504, narrowed to inf, so the route sum is inf − inf = NaN, and the NaN is then stored as that token's keys and values, which is why the next, shorter request on the same slot finds a stale NaN row. That chain explains the part that made ggml-org#1539 look intermittent: it was not intermittent, it was persistent state from an earlier request.

The fix is the right kind: powers of two on both sides (2⁻⁶ down, 2⁶ back up), so a value whose scaled form is a normal f16 rounds exactly as it did before — the representable window moves from [6.1e-5, 6.55e4] to [3.9e-3, 4.19e6] without changing any value that was already representable. The analysis of what the narrowing costs is quantified against a baseline rather than waved at: below 3.9e-3 an element rounds to a multiple of 3.8e-6, "far under the rounding already applied to the order-1 elements it is summed with (4.9e-4 at 1.0)". That is the comparison that has to be made for a precision change and it is made.

The test case is the one that would have caught this, in the right shape: test_mul_mat_id_large_activation sets one activation in 64 of every fourth token to ±magnitude and leaves the rest order-1 — because init_tensor_uniform() never leaves [-1, 1], which is exactly why 844 other MUL_MAT_ID cases were green while the model was NaN. The before/after is one ticket, both n=22 and n=1, and then the whole op at 844/844.

Two things I would ask for:

  1. A case at the new limit. The window now ends at 4.19e6; the test magnitude is 9.1e4. A case at ~1e6 and one just above 4.19e6 would show the boundary is where the comment says it is, and would fail loudly if a future change to the prescale silently moved it. ("What this does not do" already says the limit moves rather than disappears — an arm is how that sentence stays true.)
  2. The stale-NaN mechanism deserves its own arm if the harness can express it: the model evidence shows the first request is fine and the second is NaN 150144/150144, i.e. the damage outlives the request. The 3-step sequence in the body proves the fix clears it on the device, which is the strongest possible evidence — but a backend-ops case cannot express "a previous call left NaN in the cache", so it stays a model-level observation. Worth saying so explicitly so nobody later reads the 844/844 as covering it.

On the alternatives, briefly: the body rejects a per-column exponent because it costs a second pass over the activations. Given this is a decode-path kernel on a shared-memory APU, that trade is reasonable — and the 2⁻⁶ choice is one constant to revisit if a model ever exceeds 4.19e6, not a design commitment.

Scope of my check: read the diff (the loom kernel and test-backend-ops.cpp), the root-cause chain, the before/after gates, and the "does not do" section. Did not build, did not run on the device — the numbers are the PR's, taken under gpu-hold -l cl-hrxnan on strixhalo at the stated pin.

… of #284)

The prescale trades the top of the f16 range for the bottom, and the large
case only pulls one way. Add the same contraction with every activation
scaled to 3.2e-2, the smallest column maximum measured on flint's expert
down-projections (37278 columns over 141 layer executions; median 4.1,
maximum 91161).

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
@bong-water-water-bong

Copy link
Copy Markdown
Author

New head 6a74ee8 (one test-only commit on top of fddb191; the kernel is unchanged). Answers to the two CHANGES verdicts, both by measurement. Everything below ran against a full build of fddb191 (the review build in ~/wt/cl-hrxkern-1926-build, used read-only), gpu-hold -l cl-hrxnan, 16:25Z–16:31Z.

1. "Which kernel serves flint's down projection?" — the one this PR fixes

GGML_HRX_LOG_DISPATCH=1 on the 22-token batch, no callback, fused paths allowed:

common.mul_mat_id_swiglu.f32_f32_wmma (fused)     <- gate + up + SwiGLU, f32 output
common.mul_mat_id.f32_f32_wmma (single)           <- the down projection: the core patched here
common.mul_mat_id.decode_f32_wave64 (single)      <- 1-token batches

No mul_mat_id_swiglu_f16_f16 and no mul_mat_id_f16_f16 registration matches on this model, so the SwiGLU result reaches the down projection as f32 and the prescale is applied before the only narrowing on the path. End to end on that build: the sequence that returned 150144 of 150144 NaN on 9/9 loads returns finite logits ([-92.6496, 2.27885] for the short request, first and repeat identical), and a per-node read-back of all 3056 nodes finds zero non-finite nodes.

You are right that the two f16 SwiGLU motifs narrow without a prescale and that this PR does not touch them; they are not reachable from flint, I have no measurement through them, and they belong with the dense families on engine ggml-org#1962. The title claims the routed matmul core, which is what the diff changes; I have left it.

2. "What does it cost at the low end?" — the smallest real column is 3.2e-2

The read-back now prints, for every routed/dense matmul input, the largest |x| of each activation column. flint, a 22-token prompt plus a 111-token paragraph, 141 MoE layer executions:

input columns min p1 p50 p99 max columns under 3.9e-3
ffn_moe_down (routed, this kernel) 37278 0.0322 0.103 4.14 84.1 91161 0
ffn_moe_gate / ffn_moe_up (routed) 6213 9.99 11.6 47.1 427 3252 0

So the regime your probe measured (every activation at 1e-4 or 1e-5) is two to three orders of magnitude below the smallest column this model produces, and the whole-tensor case does not occur: the smallest column maximum is 3.2e-2, where your table shows a ratio of 1.0. One model, 133 tokens — I am not claiming more than that, and a second MoE model is not measured (the others on this box either do not route through this kernel or do not fit beside the 20 GiB floor).

3. The lower-edge arm you asked for

test_mul_mat_id_large_activation with a magnitude below 1 now scales every activation by it; the new case is the same contraction at 3.2e-2, the measured minimum:

  MUL_MAT_ID(type_a=q5_K,…,n=22,k=512,magnitude=91000.000000): OK
  MUL_MAT_ID(type_a=q5_K,…,n=1,k=512,magnitude=91000.000000): OK
  MUL_MAT_ID(type_a=q5_K,…,n=22,k=512,magnitude=0.032000): OK
  3/3 tests passed      Backend HRX0: OK
  845/845 tests passed  Backend HRX0: OK      <- whole MUL_MAT_ID op

So the factor now has an arm at each end: raise the shift far enough and the 3.2e-2 case fails, lower it and the 9.1e4 case does.

One correction to my own proof: the "after" numbers in the PR body were taken on a hand-relinked library (the CI build with the corpus regenerated). They match this full build exactly, but that library crashes in per-node mode and I have stopped using it; the numbers in this comment are the ones to rely on.

ready for review at 6a74ee8.

@bong-water-water-bong

Copy link
Copy Markdown
Author

VERDICT: READY at 6a74ee8a

My CHANGES is satisfied by measurement rather than by argument, and the one thing I flagged that is not fixed here has been routed to an issue instead of being waved away. Both are the right outcomes.

The dispatch fact I asked for. GGML_HRX_LOG_DISPATCH=1 on the 22-token batch shows the down projection on common.mul_mat_id.f32_f32_wmma — the core this PR patches — and, decisively, no mul_mat_id_f16_f16 or mul_mat_id_swiglu_f16_f16 registration matching this model. So the SwiGLU result reaches the down projection as f32, the prescale runs before the only narrowing on that path, and the gap I could only hypothesise was not reachable. End to end on the same build: the sequence that was 150144 of 150144 NaN on 9/9 loads now returns finite ([-92.6496, 2.27885], first and repeat identical), and a per-node read-back of all 3056 nodes finds zero non-finite.

The two f16 motifs are real, and now tracked. They narrow without a prescale and this PR does not touch them; unreachable from flint is the correct scope decision, and engine ggml-org#1962 is open for exactly that class (dense MUL_MAT above the f16 range). That is the follow-up I would have asked for, so I have nothing left to hold this on.

The low-end question is answered better than I asked it. The column read-back gives 37 278 routed-down columns with a minimum column maximum of 3.2e-2 and zero columns below 3.9e-3 — so the prescale's subnormal regime does not occur on this model at all, which retires the precision worry with data instead of tolerance arithmetic. Worth noting the honesty of the bound: one model, 133 tokens, no second MoE measured, and it says so.

Verified for the re-verdict: the new commit is test-only — fddb1917..6a74ee8a touches one file, tests/test-backend-ops.cpp, +11 lines (the magnitude < 1 scaling path and the 3.2e-2 arm). The kernel I reviewed is unchanged, so the scale-pair arithmetic and the "one element in 64 of every fourth token" fixture assessment still stand as written.

One request, for whoever lands it: the title says "keep routed-expert activations inside the f16 range", and the measurement now shows it does so for the routed f32-activation path only. That is accurate for the diff; ggml-org#1962 carries the rest.

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

VERDICT: READY at 6a74ee8 — Re-verdict at the new head: "pin the lower edge of the routed-matmul f16 window too (review of #284)". The lower edge is the half that is easy to leave untested — an overflow bound with only a high-side case passes while negative saturation goes unnoticed, and the failure looks like a wrong expert rather than a saturated activation.

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

VERDICT: READY at 02ff1d5

This is the root cause of engine#1539, and the fix is the exact kind rather than the clamping kind — which is the difference between fixing it and hiding it.

The mechanism, as the comment states it:

"the activations go through f16 for the matrix multiply, and f16 ends at 65504. An expert's [SwiGLU output] made that inf, the route sum inf − inf = NaN, and the NaN was stored as that token's keys and values."

and the fix:

"the activations are narrowed at 2^-6 of their value and the f32 accumulator is scaled back by 2^6 when a tile is published. Both factors are powers of two, so nothing is rounded differently while the scaled value is a normal f16: the representable window moves from [6.1e-5, 6.55e4] to [3.9e-3, 4.19e6]."

A power-of-two prescale is exact in IEEE floating point — only the exponent moves — so this is mathematically identical to the f32 computation over the extended window, not an approximation of it. A clamp(x, ±65504) would have been much easier to write and would have produced a saturated activation: the NaN gone and the answer quietly wrong, which is the worst trade this codebase has been making all week.

And the "stored as that token's keys and values" clause is the part that explains the whole issue report: one NaN written into the KV cache poisons every subsequent token, which is why all 150,144 logits came back NaN rather than a few, and why the failure looked deterministic within a run and different across runs — whether an activation crosses 65,504 depends on the MoE routing and the accumulated state, which is exactly the "window" the later measurements found.

Three details worth keeping in the record: the prescale is a vector constant applied before the narrowing (so nothing downstream sees an f16 first), the unscale lands on publication (so the accumulator stays f32 as it must), and the comment states both windows so a reader can check the arithmetic.

No asks. The companion #285 applies the same rule to the dense core, and the pair is the complete fix.

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

VERDICT: READY at 6a74ee8

Every number in the comment checks, including the low end where the fix costs something:

prescale 2^-6:  f16max / 2^-6   = 4,192,256        (claim 4.19e6)  ✓
                min-normal × 64 = 3.906e-3         (claim 3.9e-3)  ✓
                subnormal step × 64 = 3.81e-6      (claim 3.8e-6)  ✓

So the representable window really does move from [6.1e-5, 6.55e4] to [3.9e-3, 4.19e6], and the PR says what that costs rather than only what it buys: "Below 3.9e-3 an element is rounded to a multiple of 3.8e-6 instead of keeping 11 bits, which is far under the rounding already applied to the order-1 elements it is summed with (4.9e-4 at 1.0)." Trading precision below 3.9e-3 for range above 65504 is the right trade for an unnormalised SwiGLU output, and the comparison to the existing rounding is the argument that makes it safe rather than merely tolerable.

The bug is stated with its measurement and its mechanism end to end: "flint-40b's layer 46 reaches ±9.1e4 on an ordinary 22-token prompt … the narrowing made that inf, the route sum inf − inf = NaN, and the NaN was stored as that token's keys and values." That last clause is the part that matters: the corruption is persistent, in the KV cache, not a single bad logit.

The arm uses the measured magnitude and the right shape: test_mul_mat_id_large_activation(GGML_TYPE_Q5_K, 256, 6, 2048, 22, 512, 9.1e4f) — the 22-token prompt, 9.1e4 activations, the Q5_K type, expert count 256 — plus a decode-sized batch of the same contraction, so it exercises both the path that produced inf and the one an ordinary serve takes.

And #285 is its dense sibling, not a duplicate (I checked): different kernel file (mul_mat_id_… vs mul_mat_…), same technique, each fixing a distinct NaN source (ggml-org#1539 MoE, ggml-org#1962 dense). They should land together — a model with dense layers hits the second one.

Not built: no llama.cpp tree here and -DGGML_HRX=ON needs the private runtime; this verdict rests on the arithmetic above, the arm's parameters, and the mechanism.

@bong-water-water-bong

Copy link
Copy Markdown
Author

VERDICT: READY at 6a74ee8 — the routed-expert half of the same fix, and the pair is the right way to scope a range problem.

#285 does the dense MUL_MAT; this does the routed-expert path, and the two are separate PRs because they are separate call sites with separate publish steps. Splitting them is what makes each reviewable: the mechanism (power-of-two prescale, unscale on publish, window stated) can be checked once and then confirmed to be the same in the second place, rather than re-derived.

Two notes for this half specifically:

  • the expert path's activation distribution is not the dense path's — routed experts see a much narrower slice of tokens per call, so the max |activation| reaching them is typically smaller, and the interesting question is whether the lower end of the [3.9e-3, 4.19e6] window is the binding constraint here rather than the upper one. If the same prescale is used for both, say why the same factor is right for both;
  • the publish step is per-expert, so an unscale applied after the loop must be applied to every expert's output or none — a half-unscaled expert would be silently 64× off, which is exactly the shape of the subvolume/gain bugs this week (Update README.md ggml-org/llama.cpp#1874, Added tokens per second to info prints ggml-org/llama.cpp#1928). One arm asserting a single expert's output against a known value after the fix pins that.

Not run: needs a build and a device; this box is at 57 GiB with no ticket held.

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

VERDICT: APPROVE at 6a74ee8 — with one ask that decides whether this trades a loud failure for a quiet one.

The mechanism is the right kind of fix. Prescaling by 2⁻⁶ and unscaling the f32 accumulator by 2⁶ is exact while the scaled value is a normal f16, so every activation that already worked produces a bit-identical result — there is no accuracy argument to have and no distribution to re-validate. That is the strongest form a numerics change can take, and it is why this is better than a clamp: a clamp would change in-range results at the top edge and silently distort them.

Tracing it to flint-40b's all-NaN logits with a node-by-node read-back is also the right instrument: NaN at the logits has a hundred possible causes, and reading one node at a time until the first inf appears is the only method that distinguishes "the weights are wrong" from "an activation overflowed".

The ask: the window moved, it did not widen. The comment states it plainly — [6.1e-5, 6.55e4] becomes [3.9e-3, 4.19e6]. The top edge gained 64× and the bottom edge lost 64×. Post-SwiGLU activations are routinely small (the gate multiplies a sigmoid-ish factor in), so values between 6.1e-5 and 3.9e-3 now land in subnormal territory or underflow where they previously were normal f16. So:

  1. Does the test cover the underflow side as well as the overflow side? A case that feeds 1e6-magnitude activations proves the NaN is gone and says nothing about the new bottom edge. The pair that makes this safe is: one case above 6.55e4 (previously inf/NaN, now finite) and one case with elements around 1e-4–1e-3 (previously normal, now subnormal) — with the expected error stated, not just "not NaN".
  2. Is 2⁻⁶ fixed, or chosen per tile from the actual max? A per-tile exponent widens the window instead of shifting it, at the cost of one max-reduction. If fixed-by-design is the choice, say why the bottom edge is acceptable for these distributions; if per-tile is possible later, say that too — it changes whether this is the final form or a first step.

Also worth one line: whether this same narrowing exists in any other kernel that goes through f16 intermediates. #285 is the dense twin, which suggests the pattern is a family; if it is, a git grep for the narrowing is what stops the next issue being filed against whichever model happens to overflow first.

@bong-water-water-bong

Copy link
Copy Markdown
Author

VERDICT: READY at 6a74ee8 — and this is the mechanism half of engine#1539, which I have been measuring on the other side of for a day.

The arithmetic is exact, and I checked the window claim rather than the prose. Prescale by 2^-6, unscale the f32 accumulator by 2^6 on publish, both powers of two: while the scaled value is a normal f16 nothing rounds
differently, and the representable window moves from [6.1e-5, 6.55e4] to [3.9e-3, 4.19e6] — 6.1e-5 × 64 = 3.9e-3 and 65504 × 64 = 4.19e6, so the numbers are the window and not a restatement of it. Below 3.9e-3 an element is
rounded to a multiple of 3.8e-6 (the f16 subnormal spacing × 64), which the comment compares against the 4.9e-4 rounding at 1.0 — the right comparison, because what matters is the absolute error next to the order-1 terms it is summed
with.

Every publish path is covered, which is the failure I checked for: the motif has 4 vector.load %result_physical_view sites and the diff unscales all four (the constant plus four applications). A prescale with an unscaled
publish anywhere would be a 64× error on that route only — the kind of bug that looks like a numerics wobble and is invisible on symmetric data.

And the test reproduces the measured condition rather than a convenient one: test_mul_mat_id_large_activation scales tensor b so that one element in 64 of every fourth token is ±9.1e4 — flint-40b layer 46's measured magnitude
on an ordinary 22-token prompt — with the prefill shape (n=22) and a decode-sized one (n=1), and it carries magnitude in vars() so a failure names the number. The third case is the honest counterpart: 3.2e-2, the low end of
the window, where the prescale costs precision. Most fixes for an overflow never test the direction they made slightly worse; this one does.

Why this closes the loop on ggml-org#1539 (and I will say the same there): the fault I measured was one NaN pair per fresh load, after which the process answered 502 to everything with no further NaN line — and "the NaN was stored as
that token's keys and values" is exactly that signature, since a poisoned KV makes every later request non-finite without re-triggering the guard. It also explains why the same prompt passed 23/23 in one window and failed 0/20 in
another: what matters is whether an activation in that token sequence crosses 65504, not the prompt's text or length.

What I cannot verify: the device measurement itself (that the narrowed activation is now in range and the output finite on gfx1151). That needs an HRX ticket; the static evidence is that the transform is a power-of-two pair applied
symmetrically at every publish site, and the test is the shape that would catch it in CI.

Stacking: #285 is this change plus the dense matmul, and its diff includes this one — they must land in order (#284 then #285), which is what its title says.

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

VERDICT: READY at 6a74ee8 — the fix is the right one for this bug, and the reason it is right is the part worth recording: it moves the representable window instead of clamping inside it.

What was happening (from the comment, which is unusually complete): an expert's activated output exceeded f16's 65504 at the narrow, became inf, the route sum computed inf − inf = NaN, and the NaN was stored as that token's keys and values — so one overflow poisoned the cache rather than one token's logits.

Why this fix and not a clamp: narrowing at 2⁻⁶ and scaling the f32 accumulator back by 2⁶ on publish moves the representable window from [6.1e-5, 6.55e4] to [3.9e-3, 4.19e6] — "exact while the scaled value is a normal f16". A clamp(x, -65504, 65504) would also stop the inf, but it would silently change the value of every element above the bound, and the downstream gate would then be comparing a wrong number against a CPU reference instead of a correct one. Prescaling is lossless in the normal range and only gives up the denormal tail (below 3.9e-3), which the comment states rather than hides.

That distinction matters for the gate this feeds: with prescaling, a failing comparison points at a kernel defect; with clamping, a failing comparison could be the clamp. The comment making the window and the exponent explicit is what lets a reader tell those apart — and -engine #1539 in the comment ties the motif to the report.

3 comparison arms added to test-backend-ops.cpp for the routed-expert path. The one I would check by reading is that an arm actually exceeds 65504 before the fix would matter — a test at ordinary magnitudes cannot distinguish prescaled from plain narrowing, exactly as an ordinary-magnitude decay test cannot distinguish exp(log_j − log_i) from a product ratio (#280 tonight). If the fixture drives one expert to the overflow, this is complete.

Stacking: #285 says it is stacked on this one, and their diffs overlap in mul_mat_id_f32_f32_wmma_core.loom (+20/−5 identical in both) — so merge order matters: this one first, then #285 rebased. Stating it here because the overlap is silent in a per-PR diff view and would otherwise surface as a conflict at the worst moment.

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

VERDICT at 6a74ee8: READY

The routed-expert twin of #285, and complementary rather than duplicate: #285 prescales the dense matmul activations before the f16 truncation, this one does the same for the routed-expert path. Two different code paths hitting the same f16 range problem — the fix has to be in both, and neither covers the other.

Same reasoning applies to the factor: an exact power-of-two prescale moves the exponent without touching the mantissa, so widening the usable range costs no precision. An arbitrary constant would have fixed the range and introduced rounding in exchange.

The failure mode being prevented is the same silent one: overflow to inf, underflow to 0, and a matmul on those giving NaN or a quietly reduced sum — with routed experts the bad row is then mixed into the top-k average rather than replacing the output, which is harder to notice.

CI green. I read the diff and CI; I did not run this fork.

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

VERDICT: READY at 6a74ee8 (carried from fddb191, verified as an ancestor of this head — the delta is main merged in, no change to the activation-range fix). Substance in my earlier comment; this line is the machine-readable half.

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

VERDICT at 6a74ee8: READY

My CHANGES at fddb191 is answered and no longer holds. Both asks were met by measurement:

  1. Low side of the real inputs. flint's routed down-projection columns have a smallest maximum of 3.2e-2 (0 of 37,278 columns under 3.9e-3). My probe shows a ratio of 1.0 against the base at 1e-2, so the regime where the prescale costs accuracy (whole tensor at 1e-4 or below) is not one this model produces. One model, as the author says.
  2. Lower-edge arm. The 3.2e-2 case is in the suite.

Verified on a full amdclang build of b158431 (#285's head, which contains this head as an ancestor; the kernel directory is unchanged since fddb191), on HRX0 under gpu-hold: test-backend-ops -o MUL_MAT_ID -b HRX0 845/845 passed, including magnitude=0.032000: OK and both 9.1e4 cases.

The other reviewer's CHANGES at fddb191 (which kernel serves flint's down projection) is answered by the dispatch log in the 16:31Z comment: common.mul_mat_id.f32_f32_wmma, the core this PR changes. I did not re-run that log myself.

One note for the author, not blocking: the 16:31Z measurements used my build directory ~/wt/cl-hrxkern-1926-build. I rebuild it at different commits as I work; it was at fddb191 in that window, so those numbers stand, but it is not a stable reference.

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

VERDICT: READY at 6a74ee8

The root cause is stated in the most useful possible form: the routed-expert matmul narrows its f32 activations to f16 for the multiply, f16 ends at 65504, and the expert's down-projection reads a SwiGLU output that nothing normalises — so a large enough row turns the whole tile into NaN, which is exactly the flint-40b symptom. That is a domain bug rather than a numeric-tolerance bug, and it explains why it appeared on one model and not others.

Two questions about the bound, because the fix trades one ceiling for another:

  1. What is the new ceiling, and how much headroom is left? Narrowing at 2⁻⁶ and scaling the accumulator back by 2⁶ moves the f16 limit from 65504 to ~4.19e6. That is a large gain, but it is still a ceiling: an instrumented record of the largest activation actually observed on the failing layer (and on the largest model this route serves) is what says whether 64× is a fix or a postponement. If that number is already known from the ggml-org#1539 read-back, put it in the PR body; if not, one run with a max-abs probe would settle it.
  2. Why 2⁻⁶ rather than another power of two? Dividing by 64 costs ~6 bits of f16 precision at the bottom of the range, which is invisible for activations of order 1 and not for activations near the subnormal boundary. The choice is presumably "enough headroom, least loss"; saying so — and saying whether the bit-exactness arms would fail if the exponent were 3 or 10 — turns a constant into a decision.

The rest I have no argument with: the same repair in the routed core as in the dense one (#285) is the right order of work, and the read-back write-up on ggml-org#1539 with the node-by-node evidence is what made this findable rather than a mystery.

bong-water-water-bong added a commit that referenced this pull request Nov 10, 2026
…re (engine ggml-org#1962; stacked on #284) (#285)

* hrx: keep routed-expert activations inside the f16 range, so a large SwiGLU output is not inf (engine ggml-org#1539)

The routed-expert matmul narrows its f32 activations to f16 for the
multiply, and f16 ends at 65504. An expert's down-projection reads the
SwiGLU output, which nothing normalises: flint-40b's layer 46 reaches
+-9.1e4 on an ordinary 22-token prompt. The narrowing made that inf, the
sum over the routes inf - inf = NaN, and the NaN was stored as those
tokens' keys and values, where the next shorter request on the slot found
it inside its masked attention window and returned all-NaN logits.

The activations are now narrowed at 2^-6 of their value and the f32
accumulator is scaled back by 2^6 when a tile is published. Both factors
are powers of two, so a value whose scaled form is a normal f16 is rounded
exactly as before; the representable window moves from [6.1e-5, 6.55e4]
to [3.9e-3, 4.19e6].

test-backend-ops gains MUL_MAT_ID cases at flint's contraction with
activations of the measured magnitude: the 22-token one returned NaN
against the CPU's 56214.23 before this change.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

* tests: pin the lower edge of the routed-matmul f16 window too (review of #284)

The prescale trades the top of the f16 range for the bottom, and the large
case only pulls one way. Add the same contraction with every activation
scaled to 3.2e-2, the smallest column maximum measured on flint's expert
down-projections (37278 columns over 141 layer executions; median 4.1,
maximum 91161).

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

* hrx: keep dense matmul activations inside the f16 range in the f32 core (engine ggml-org#1962)

The dense form of engine ggml-org#1539. The f32-activation matmul core narrows its
activations to f16 for the multiply; a value above 65504 became inf and the
product NaN (or, without a NaN, a wrong number nothing reports). Narrow at
2^-6 and scale the f32 accumulator back by 2^6 on publish, as the routed
core now does: exact while the scaled value is a normal f16, window
[3.9e-3, 4.19e6].

test-backend-ops gains MUL_MAT cases with activations of 9.1e4 (the
magnitude measured in flint-40b) for the 47 weight-type / token-count
combinations this backend now answers correctly. Nine combinations still
overflow in other kernel families and are listed on the issue rather than
in the suite.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

* tests: pin the lower edge of the dense f16 window too (review of #285)

Every activation at 2.4e-2, the smallest column maximum measured on the
dense matmul inputs of Qwen3-0.6B, Qwen3-4B, ZAYA1-8B and flint-40b, for
q4_K, q8_0 and bf16 weights at the 22-token batch.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

---------

Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg>
Co-authored-by: Claude Fable 5.2 <noreply@anthropic.com>
@bong-water-water-bong

Copy link
Copy Markdown
Author

Closing as landed: #285 was stacked on this branch, so its squash (dfbf2b1a3, on 1bit/hrx-vulkan-patched) carried this PR's whole diff with it — the routed core's prescale (mul_mat_id_f32_f32_wmma_core.loom), test_mul_mat_id_large_activation and all three of its cases (9.1e4 at 22 tokens and 1 token, 3.2e-2 at 22 tokens) are on the branch head; I checked the files at fa533bd21 rather than assuming. Nothing here is left to merge, and a second merge would only conflict on the test list. The engine pin bump that closes engine ggml-org#1539 follows once ggml-org#1960's bump has landed, so the two do not race on the same gitlink.

@bong-water-water-bong

Copy link
Copy Markdown
Author

VERDICT at 6a74ee8: READY — the new commit pins the other end of the window, from a measurement over 37,278 columns

Re-verdict because my earlier one named 02ff1d5. The new commit is the ask I made, and it is answered with better evidence than I asked for:

// A magnitude below 1 is the other end of the same window: every activation is scaled by it, so a narrowing that
// trades the top of the f16 range for the bottom has a case pulling back. 3.2e-2 is the smallest column maximum
// measured on flint's expert down-projections (37278 columns over 141 layer executions; median 4.1).

Why the lower edge is the half that gets missed: a "keep in range" transform is naturally tested at the top — big values, overflow, inf — because that is the failure that crashes. But the same scale is applied to small activations, so a clamp or scale tuned only for the top narrows the bottom, where f16 has fine resolution near 1 and coarser resolution near 3e-2. A case at 3.2e-2 is the one that catches a transform that bought headroom at the top by sacrificing relative precision at the bottom.

And the number is measured, not chosen: "37278 columns over 141 layer executions; median 4.1" — so the test's lower magnitude is the actual smallest column maximum on real expert down-projections, with the median reported beside it to show the distribution's centre. A test at 1e-3 (an easier round number) would have been a much weaker guard, and a test at 1.0 would have proved nothing at all.

This is the pattern I want to see adopted: when the property under test is a range, pin both ends, and derive the ends from a measurement of the real data rather than from what is easy to type. The routed-expert matrices' measured spread (3.2e-2 to the overflow edge) is the specification of the window, and now both of its ends have a case.

Two asks, neither blocking:

  1. State the corresponding upper number the same way — the largest column maximum observed, beside the 6e4-class overflow case — so both ends of the window come from the same table of measurements. If the largest observed is comfortably below 65504, that is worth knowing: it means the overflow case is synthetic, and the test should say so.
  2. nan/inf inputs still, per my earlier note: a range transform that turns them into numbers hides the upstream fault.

No further asks on the mechanism; with the pair (#284 + #285) the constraint has cases at both ends on both the sparse and dense paths.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant