Repository navigation
docs/determinism: all three routes at one budget, thinking off, on a #864 build - #872
Conversation
PR Reviewer Guide 🔍(Review updated until commit 37d1129)Here are some key observations to aid the review process:
|
|
Content looks right and the tool checks out — but it cannot be merged in this shape, for the same reason as #862: it So What I could verify at this head, and it holds: and the page itself is honest in the places that usually are not: the new row names the build it was taken on Rebase it and I re-verdict at the new head — the check is |
…n a #864 build Qwen3-0.6B Q4_K_M, 8 prompts x 64 tokens, greedy, --thinking off, --match-by text, on main@692cfa2 + #864 (ce1af72) — the configuration a numeric gate would be written against, and the first row where the NPU needs neither a pre-#748 build nor an unfixed engine. cpu and hrx are bitwise repeatable over 5 repeats, npu over 2; all three at one budget. Agreement: cpu|hrx token match 0.721 / mean 4.03e-02, cpu|npu 0.331 / 8.62e-02, npu|hrx 0.410 / 8.89e-02, no pair bitwise (NPU exact-bit 0.000). The cpu route was served in both device claims and agrees with itself 128/128 positions, exact bits. Two facts worth the row: the cpu/hrx numbers reproduce the c18 binary digit for digit (so the CPU and HRX decode paths did not move between that binary and main@692cfa2), and thinking off moves the NPU pairs *further* (8.6-8.9e-02 against 5.2-5.5e-02 with thinking on) while raising token agreement, because a reasoning stream is long runs of tokens both routes pick identically. The tool's docstring no longer claims the NPU ignores enable_thinking, and the known-limits line that said the NPU row cannot be reproduced on 44d849d or later now says which configuration is reproducible and how.
38112cd to
37d1129
Compare
|
Persistent review updated to latest commit 37d1129 |
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY
At 37d112923 (rebased onto a77aa662e), superseding the status comment I left on 38112cd64. The stack is gone: this
is a single commit on main, two files, +62/-22 — the same two files I reviewed, blob-identical at both heads
(docs/determinism.md 2432e6ff…, tools/logprob_agree.py 7754840c…), so nothing was rewritten in the rebase and the
#864 commit it used to carry is no longer part of it.
Re-ran the tool at this head: python3 tools/logprob_agree.py --selftest -> logprob_agree selftest: ok, rc 0. CI at
37d112923: build success (02:39Z).
The page's own evidence, which is what makes it quotable: the new "all three routes, thinking off" row names the build it
was measured on (main@692cfa2 + #864, ce1af72) instead of inventing a merge sha, carries the double-served cpu
control (two separate claims agreeing 128/128 positions, exact bits, max |Δlogp| = 0), reproduces the c18
cpu/hrx figures digit for digit (122 aligned, 88 common, 0.721, 0.080, 3.98e-01, 4.03e-02), and says out loud that
thinking-off moves the NPU pairs further away (mean |Δlogp| 8.6-8.9e-02 against 5.2-5.5e-02) rather than picking the
flattering row. The --thinking off --match-by text command in the usage block matches the flag the tool actually takes.
Follow-up to the merged determinism page (#831). It closes the one measurement that page left open, and
it is stacked on #864: this branch's parent commit is
ce1af72, so please merge #864 first and thisdiff becomes the last two files only.
What was missing
docs/determinism.md(merged as #831) stated three things about the NPU route: it is bitwise repeatable,it could only be compared with thinking on, and its row needed a build before
44d849dbecause alater build masks reasoning tokens and
logprobs.contentcame back empty while thinking(#832). #864 fixes #832 by honouring
chat_template_kwargs.enable_thinkingon the forward path, which makes the natural configuration —thinking off, the one a numeric gate would be written against — measurable on every route. This PR
measures it and corrects the page's caveats.
Measured (
main@692cfa2+ce1af72, Qwen3-0.6B Q4_K_M, 8 prompts × 64 tokens, greedy,--thinking off,--match-by text, NPU at--prefix-slots 0)cpuhrxnpu(forward path)cpuvshrxcpuvsnpuhrxvsnpucpuwas served in both device claims (the GPU and NPU queues are separate) and is the control: thetwo
cpureports agree 128/128 positions, exact bits,max |Δlogp| = 0.Two facts the row adds
cpu/hrxnumbers reproduce the c18 binary digit for digit — same 122 aligned / 88 commonpositions, token match 0.721, exact-bit 0.080, max 3.98e-01, mean 4.03e-02 as the first measurement on
this page (
lemonade-onebit-rc-2026-10-31c). The CPU and HRX decode paths did not move between thatbinary and
main@692cfa2.thinking on — while token agreement is higher (0.33-0.41 against 0.26), because a reasoning stream is
long runs of near-deterministic tokens both routes pick identically. Both rows are kept; they answer
different questions.
Also in the diff
tools/logprob_agree.py: the module docstring said "the NPU forward path ignores enable_thinking" — truewhen written, false after npu: forward-serve honours chat_template_kwargs.enable_thinking, so logprobs.content is no longer always empty (#832) #864. Two sentences and one usage example changed, no behaviour.
reproducible and does not need an unfixed engine.
Verified
python3 tools/copyright.py --check→ rc 0;python3 tools/logprob_agree.py --selftest→ ok.51 pages, one<h1>ondeterminism.html, new anchor#all-three-routes-thinking-offpresent.amdxdnaerrors 0 after it, die43 °C before and after.
Nothing is ported from a reference implementation; NOTICE unchanged. Owner merges.