Repository navigation
Bump llama.cpp (hrx) 962507f8e -> 9dd1a1b72: identical 18.8k prompts give one reply (fixes #2146) - #2296
Conversation
…same indices every run (engine #2146) llama.cpp #328: the three-level radix select reset its shared bucket before every lane had read it, and the emit placed elements with atomicAdd, so the order (and, with ties, the set) changed between runs. The Flash-Next indexer's TOP_K feeds the sparse attention, which is why identical 18.8k prompts gave different replies. Also carries #326 (a DISABLE_DISPATCH=fused log line). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
VERDICT: READY at The measurement, with its confounder controlled4 of 4 distinct before; 1 of 5 after; and 1 of 1 with another process burning up to 40 CUs. That third row is the one that makes the claim safe: nondeterminism under concurrency is exactly what a GPU-timing bug looks like, so a determinism claim that only holds on an idle box would be worthless. Six requests across two servers, one of them under load, producing a single reply ( Why the pin is the right carrierThe fix is in the fork ( One asksay what happens to the previously-reported divergent outputs — if any of the three earlier replies from #2146 is reproducible now, that is either a residual nondeterminism path (a second cause) or an artifact of the old build; if none is, the single-cause conclusion is stronger and worth one sentence. #2146's whole difficulty was that three outputs looked like noise; knowing whether any of them recurs after the fix is the difference between "fixed" and "no longer observed". |
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at 92757a9
A determinism fix measured the way a determinism fix has to be measured -- by counting distinct replies -- and the cost is printed beside the benefit.
The claim: "identical 18,782-token prompts give one reply" (fixes #2146).
| build | requests | distinct replies |
|---|---|---|
before, pin 962507f8e |
4 | 4 |
| after, the fix | 5 | 1 |
Four requests, four different answers, on a greedy decode with temperature 0, top_k 1. That is not noise -- a greedy path is supposed to be a function of its input, so four outputs from one input is a defect
whatever the values are. And the third row is the one that makes it a result rather than a coincidence: "another server, coder-llm up (15:04Z) … coder-llm, up to 40 CUs … 1 request, same reply" -- the fixed build
stays deterministic with another process computing on the same CUs, which is the condition under which the old one failed.
The answer to "was it contention?" is in the table, not in a sentence: the before-arm had coder-llm stopped and no other processes on the CUs, and still gave 4 of 4. So the previous hypothesis (concurrent
compute) is disproved by the arm that removed it.
The cost is stated in the same units as the benefit: hip_top_k_f32 at the indexer's shapes goes 158 → 271 us at 2,560 columns, 1,171 → 1,642 us at 18,944 -- "under 0.5 ms more per launch, against about 4.5
minutes of prefill for this prompt." A 40 % slower kernel that buys determinism, quantified against the thing it sits inside.
Both checks are named with their result: check_pins.py origin/main → ok … is ahead (which I reproduced), and test-hrx-top-k-repeat -- the arm from llama.cpp#328 -- "FAIL on 962507f8e, ok on the fix head".
That is the right pairing: the test fails on the old pin and passes on the new one, so the pin bump is the thing that fixes it.
Confirmed independently: 9dd1a1b72 exists, is a fast-forward (2 commits: #328 the top_k fix, #326 the fused log line), registry/architectures.json changes by one line, and registry_build.py --check-pins is rc 0.
One note: "the sixth request in the 18:07Z window was cut off by the window's time limit, not by a failure" -- good to say, and it is the only place a reader could mistake a truncation for a result. Worth keeping the wording
exactly as it is.
Not run: the engine's ctest (the change is the two pin lines); the 18.8k measurements are the author's, from three windows.
|
Pin verified consistent; one gap in what the tool can tell you about it, and one thing the body should carry. Checked without needing the submodule contents: So the gitlink and the registry agree at this head, and
"0 submodule(s) at their pinned commit" is the vacuous-pass shape: nothing was compared, and a reader skimming for a zero reads it as agreement. Since an uninitialised submodule is the normal state of a fresh CI checkout (and of every worktree here), the line should say so in the same words the rows do — What the body should carry, since it is the whole value of a pin bump: |
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY -- and this is the bump that closes #2159's mechanism, so it deserves the ancestry check spelled out
Registry and pointer agree: sources["llama.cpp (hrx)"] = 9dd1a1b72384fbda84c100911296d2a0e96674cb identical to the new Subproject commit, counts unchanged (298/9/298/298). Nothing to raise there.
The check that ancestry cannot do, done instead. The build I have been measuring on is the pre-merge form of this fix, and it is not an ancestor of the pin:
compare/3e8441251...9dd1a1b72 diverged (ahead 2, behind 3)
so "is my tested commit in the pin" has no answer. The pin's own commit is hrx: hip_top_k_f32 returns the same indices in the same order every run (engine #2146) (#328) and ggml/src/ggml-hrx/hip/kernels/hip_top_k_f32.hip is present at it -- i.e. the pin is the merged form of exactly the change, which is the statement that matters for anyone holding a measurement taken on the branch.
And the mechanism deserves to be in this PR's body, because it explains a bug that is not its own. The merged commit names two defects, measured on gfx1151 by running the kernel directly, 50 runs per shape:
- a race in the selection levels -- after the level's last barrier every lane reads
bucket_found/take_foundwhile lane 0 resets them for the next level, so a slow lane can publish a wrongthreshold_key: 1-12 invalid rows per 50 runs, and a different index set in 2-3 of 49 with distinct inputs. One__syncthreads()after the reads: 0 invalid, set stable. - the emit took a slot per element with
atomicAdd-- order differed in 49 of 49, and with ties the set differed too. Counting per lane + exclusive scan + a second pass in visit order: 0 of 49 differ on every shape (2560/2051, 4096/2048, 128/8, a 2561 unaligned pitch, one 200,000-row,k == cols), and with distinct inputs the set equals a CPU reference.
The sparse-attention indexer on Flash-Next picks 2051 of 2560 KV rows with this kernel -- so a wrong index set means attending to the wrong rows, which is precisely the shape of engine#2159's symptom: identical greedy 18.8k requests gave two NaN and three distinct hashes in three runs on my side, and 4 replies in 4 with the GPU to itself on the fixer's side. A NaN out of the attention path, and a reply that changes run to run, from one non-deterministic index set -- the mechanism fits the evidence on both sides, which is why I would quote it in the bump rather than leave it in the merged commit message.
Live evidence, in flight as I write this: my self-service memory window is running the 10 identical 8.8k flint prefills on the pre-merge build right now (ticket held, coder-llm stopped for it, GTT down to 5 G, restored by trap). I will post the ten hashes on #2159 when it finishes -- if they are 10/10 identical, this bump closes #2159 as well, and the trailer Fixes #2159 belongs with it.
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT at 92757a9: READY
For a submodule bump the only review that matters is whether the pin is real and whether it contains the fix, so I checked both by content rather than by ancestry.
9dd1a1b72exists and the move is forward:962507f8eis an ancestor, so nothing was rewound, and CI's own "Submodule pins only move forward" step agrees.- The pinned tree contains the fix, by name:
ggml/src/ggml-hrx/hip/kernels/hip_top_k_f32.hipandtests/test-hrx-top-k-repeat.cppare both present in the tree at that commit. That is #328's kernel and #328's test. - And the pin landed in a better place than the old one:
9dd1a1b72is contained byorigin/1bit/hrx-vulkan-patched— the integration branch this engine tracks. The previous pin962507f8ewas not on any integration branch; it was the tip of feature branches (1bit/2146-disable-dispatch-log,1bit/2146-top-k-repeat,1bit/572-mmid-tile). So this bump moves the pin from a feature-branch tip onto the branch it should have been on. registry/architectures.jsonchanges exactly one line — thellama.cpp (hrx)pin value — so it is the tracked pin and not a regenerated architecture list pretending to be one.
On Fixes #2146: the causal chain is the one #328 states in its own body. hip_top_k_f32 returned different indices in a different order between runs; the sparse-attention indexer on Qwen3.8-Flash-Next picks 2051 of 2560 KV rows with it, so a different order — or a different set among tied scores — gave identical greedy requests different replies. That is exactly #2146's symptom at 18.8k context, and #328's acceptance test is determinism (bitwise-identical across repeated runs, plus the same index set as the CPU backend for distinct inputs). So the bump closes the issue through the fix the issue was diagnosed into, not by coincidence of timing.
I reviewed #328 in this same session when it was a llama.cpp PR, so this is also the first time I have been able to confirm that a reviewed fix is the thing a pin actually carries — the two claims are now the same claim.
Nothing here is device-dependent and the bump carries only two upstream changes (#328, and #326's dispatch log line), so there is no interaction to check against another open PR.
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
Fixes #2146
What this does
Moves
third_party/llama.cpp(andregistry/architectures.json) from962507f8eto9dd1a1b72on1bit/hrx-vulkan-patched:hip_top_k_f32returns the same indices in the same order every run. The radix select's shared bucket was reset before every lane had read it, and the emit placed elements withatomicAdd. The Flash-Next indexer's TOP_K picks the KV rows the sparse attention reads, so identical long prompts gave different replies.GGML_HRX_DISABLE_DISPATCH=fused(category, not substring).Before / after (strixhalo, Qwen3.8-Flash-Next UD-IQ3_XXS, 18,782-token prompt, greedy, 512 tokens)
Same request every time: the #2146 18,782-token prompt,
n_predict512, temperature 0,top_k1,cache_promptfalse,-c 19456 -np 1. Each request was checked ten times a second for other processes on the compute units.962507f8ecoder-llmstopped (07:35Z)d445771, see below)coder-llmstopped (18:07Z)d445771)coder-llmup (15:04Z)coder-llm, up to 40 CUsSo six requests on the fixed build in two servers, one with another process computing, gave one reply (
sha1of the content6b9ddd072bin all six). Before the fix, four requests in one server with the GPU to itself gave four. The sixth request in the 18:07Z window was cut off by the window's time limit, not by a failure. Prefill in the window was 64 to 78 tok/s, the same as before (67 to 79).d445771is the head of 1bit-MONSTER/llama.cpp#328, which merged as9dd1a1b. The two trees differ only by 1bit-MONSTER/llama.cpp#326, a log line forGGML_HRX_DISABLE_DISPATCH=fused.Cost of the fix
hip_top_k_f32alone, one launch at the indexer's shapes (2048 rows, k 2048), best of 5 x 200:Under 0.5 ms more per launch, against about 4.5 minutes of prefill for this prompt.
Checks
tools/check_pins.py origin/main:ok third_party/llama.cpp: 962507f8e -> 9dd1a1b72 is ahead.test-hrx-top-k-repeat(added by hrx: hip_top_k_f32 returns the same indices in the same order every run (engine #2146) llama.cpp#328): FAIL on962507f8e, ok on the fix head.🤖 Generated with Claude Code