Skip to content

determinism: measure whether a batched cell can be repeated (tools/batched_repeat.py + docs) - #1068

Merged
bong-water-water-bong merged 12 commits into
mainfrom
1bit/batched-determinism
Nov 5, 2026
Merged

bong-water-water-bong merged 12 commits into
mainfrom
1bit/batched-determinism

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

What this adds

tools/logprob_agree.py (merged in #752) answers the solo question: repeat one request, are the
log-probs bit-identical. A --parallel N scoreboard cell is measured by issuing N concurrent
requests
, so its reproducibility question is different — replay the same N requests, do you get the
same N answers? Nothing measured that before, and the answer turns out to be no.

tools/batched_repeat.py (new, Apache header, --selftest covers the fingerprint and comparator
logic, tools/copyright.py --check clean) releases N byte-identical requests through a
threading.Barrier for R rounds and takes one SHA-256 fingerprint per copy over its token ids, chosen
log-probability float32 bits and top-5 bits. It reports within-round agreement (copies vs copy0),
round-to-round agreement (copy i of round r vs round r+1) and the number of distinct states.

Measured — shipped lemonade-onebit-rc-2026-11-04-c24 payload, Qwen3-0.6B Q4_K_M, 32 tokens, greedy

request width serve --parallel cpu round-to-round identical cpu states hrx round-to-round identical hrx states
1 2 and 8 2/2 1 — —
2 2 2/4 4 — —
4 4 6/12 4 2/12 4 (all four copies distinct)
8 8 11/24 8 0/24 16 (all eight copies distinct)

Worst |Δlogp| inside those mixtures: cpu 9.81e-01 (width 4) and 9.81e-01 (width 8); hrx 7.43e-02
(width 4) and 2.05e-01 (width 8).

The root cause it points at is already filed

The state is the per-step decode width, not the serve configuration: a single request against a
--parallel 2 serve and against a --parallel 8 serve produces the same fingerprint bit for bit
(3/3 rounds each), and at width 2 one round had one copy on the solo state while its partner was
displaced — a step that carried one row for one sequence and two for the other. That is the
ggml-cpu/repack.cpp 4-row-block vs 1-row-gemv switch from #751, now visible as discrete states
instead of deltas. On hrx no two copies share a per-step row count across the run (4/4 and 8/8
distinct per round), so there is no modal rung to land on.

This PR does not change engine code: #751's fix design is there and is not a one-file change.

Docs

docs/determinism.md gains “Can a batched cell be repeated?” with the table above, the
configuration-vs-content control, the consequence for a correctness gate (a batched cell cannot be
gated on one number — report the dispersion or pin the schedule), the command to re-run, and a
Known-limits line (one prompt, 32 tokens, one model; the state count is a lower bound).

Refs #751.

Verification

  • python3 tools/copyright.py --check → rc 0
  • python3 tools/batched_repeat.py --selftest → selftest: ok
  • Every number above was produced by this tool against the shipped c24 payload: cpu widths 1/2/4/8
    (ports 18120/18121, no device claim), hrx widths 4/8 under gpu-hold with the no-SVM trio
    (2 tickets, 25 s and 12 s of held GPU respectively).

…nd hrx

A --parallel N cell is measured by issuing N concurrent requests, so its reproducibility
question is not the one tools/logprob_agree.py answers (one request repeated). N byte-identical
requests are released through a barrier for R rounds and each copy is fingerprinted over its
token ids and float32 logprob bits: on the shipped c24 payload, cpu is 2/2 identical
round-to-round at width 1 and 6/12 at width 4, 11/24 at width 8; hrx is 0/24 at width 8 with
every copy a distinct state.

The state is the per-step decode width, not the serve configuration: one request against a
--parallel 2 and a --parallel 8 serve gives the same fingerprint bit for bit, and at width 2 a
round contained one copy on the solo state and its partner displaced. That is the op #751
localised, seen as states instead of deltas. Docs say so, with how to re-run and the control.

Refs #751.
@bong-water-water-bong bong-water-water-bong added documentation Improvements or additions to documentation Review effort 2/5 labels Nov 5, 2026
@github-actions

github-actions Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

PR Reviewer Guide 🔍

(Review updated until commit fe71bf0)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

752 - Partially compliant

Compliant requirements:

  • Added tools/logprob_agree.py tool with --selftest functionality
  • Added docs/determinism.md page with measurements and explanations
  • Tool measures run-to-run determinism and first divergent position
  • Tool reports token-id match, top-1 agreement, exact-bit match, and max/mean |Δlogp|
  • Documentation includes measured contract numbers for Qwen3-0.6B Q4_K_M
  • Documentation explains why NPU route is not in comparison and notes the thinking flag issue

Non-compliant requirements:

  • No engine code was ported (as intended)
  • The numbers are one model and one binary (lemonade-onebit-rc-2026-10-31c) as specified

Requires further human verification:

  • Verification that the numbers in the documentation match actual hardware measurements
  • Confirmation that the tool works correctly with the specified hardware configurations

751 - Partially compliant

Compliant requirements:

  • Documented that identical requests served concurrently are not bit-reproducible with default --parallel 4
  • Demonstrated that log-probabilities depend on decode batch width
  • Showed that --parallel 1 makes concurrent test bitwise identical
  • Documented that dependency is on decode-batch width, not concurrency
  • Documented that this is the decode-batch sibling of the prefill case

Non-compliant requirements:

  • No fix proposal was included (as noted in the description)

Requires further human verification:

  • Verification that the measurements in the documentation match actual hardware behavior
  • Confirmation that the described behavior is consistent across different hardware configurations
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Potential Race Condition in Thread Management

The round_of function uses a threading.Barrier to synchronize threads, but after the barrier is passed, threads make a request and then join with a timeout. If a thread fails to complete within the timeout, it's marked as failed but the barrier is aborted, which could lead to a BrokenBarrierError being raised by straggling threads. This could potentially cause issues if not all threads are properly cleaned up or if the abort mechanism isn't robust enough to handle all failure scenarios.

barrier = threading.Barrier(width)

def one(i):
    try:
        barrier.wait(timeout=join_timeout)
        out[i] = (request or ask)(url, model, prompt, max_tokens, top)
    except Exception as e:  # noqa: BLE001 - a failed copy is a result, not a crash
        err[i] = repr(e)

ts = [threading.Thread(target=one, args=(i,), daemon=True) for i in range(width)]
for t in ts:
    t.start()
for i, t in enumerate(ts):
    t.join(join_timeout)
    if t.is_alive() and err[i] is None:
        err[i] = f"copy {i} did not finish within {join_timeout}s"
barrier.abort()  # a straggler raises BrokenBarrierError instead of waiting for a partner that left
return out, err
Inconsistent Error Handling in Selftest

In the selftest function, the flaky function is used to simulate a backend error, but it modifies a module-level attribute (flaky.n) which could lead to unexpected behavior if the test is run multiple times or if other parts of the code rely on this attribute. This approach is not thread-safe and could cause issues in concurrent environments.

def flaky(url, model, prompt, max_tokens, top):
    if getattr(flaky, "n", 0) == 1:
        flaky.n += 1
        raise RuntimeError("simulated backend error")
    flaky.n = getattr(flaky, "n", 0) + 1
    return base

out, err = round_of("http://x", "m", "p", 2, 5, 3, join_timeout=30, request=flaky)
assert sum(e is None for e in err) == 2, f"two copies ok, one failed: {err}"
assert sum(c is not None for c in out) == 2, "good copies recorded, failed copy is None"
assert any("simulated backend error" in (e or "") for e in err), "the failure is reported, not swallowed"
Missing Hardware Verification for Claims

The documentation states that measurements were taken on specific hardware (lemonade-onebit-rc-2026-11-04-c24 payload, Qwen3-0.6B Q4_K_M) but does not explicitly verify that all claims made in the documentation are indeed measured on the hardware. It's important to ensure that all numbers and observations are verified on the actual hardware to maintain accuracy.

### Can a *batched* cell be repeated? (`--parallel N`, the same N requests twice)

Repeating one request answers the solo case. A `--parallel N` cell is measured by issuing **N concurrent
requests**, so its reproducibility question is different: replay the same N requests and you do not get
the same N answers. Measured with `tools/batched_repeat.py`, which releases N byte-identical requests
through a barrier for R rounds and fingerprints each copy over its token ids and float32 log-probability
bits. Qwen3-0.6B Q4_K_M, 32 tokens, greedy, 4 rounds (3 where a width only needed a control), shipped
`lemonade-onebit-rc-2026-11-04-c24` payload:

| request width | serve `--parallel` | `cpu` round-to-round identical | `cpu` states | `hrx` round-to-round identical | `hrx` states |
|---:|---:|---|---:|---|---:|
| 1 | 2 and 8 | **2/2** | **1** | — | — |
| 2 | 2 | 2/4 | 4 | **2/6** | 2 of 8 (two states, swapped between rounds) |
| 4 | 4 | 6/12 | 4 | **2/12** | 4 (all four copies distinct) |
| 8 | 8 | 11/24 | 8 | **0/24** | 16 (all eight copies distinct) |

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

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

VERDICT: READY at 4136544c — the tool's own logic checks out and I reproduced its headline finding independently, on my own serve, from the same c24 payload.

Ran here:

python3 tools/batched_repeat.py --selftest   ->  selftest: ok
python3 tools/copyright.py --check           ->  rc 0

then the real thing: the c24 payload's own 1bit binary (lemonade-onebit-rc-2026-11-04-c24/1bit/1bit) serving Qwen3-0.6B-Q4_K_M on CPU with --parallel 8, on a port of my own (8097 — I did not touch the lane's 18120), 32 tokens, 3 rounds:

width 1, 3 rounds: REPEATABLE       2/2 bitwise identical, 1 distinct fingerprint
width 2, 3 rounds: NOT repeatable   1/4 bitwise identical, 4 distinct fingerprints, worst |dlogp| 1.093e+00
width 4, 3 rounds: NOT repeatable   6/8 bitwise identical, 4 distinct fingerprints, worst |dlogp| 9.814e-01
width 8, 3 rounds: NOT repeatable  10/16 bitwise identical, 4 of 24 copies, worst |dlogp| 9.814e-01

The conclusion and the anchor both reproduce. Width 1 is exactly your row (2/2, one state, so solo is deterministic and the cell can be repeated). From width 2 up it is not, and the worst divergence magnitude I measure — 9.814e-01 — is your quoted cpu 9.81e-01 to three digits, which is what convinced me these are the same phenomenon and not two different flakes. My copy counts differ from yours (I ran 3 rounds where your table implies 6, so 16 comparisons at width 8 vs your 24) and that is the only reason 10/16 sits beside your 11/24; the per-round lines show the same shape you describe — a round that is fully consistent followed by one where three of four copies diverge.

The instrument itself is sound, which is what makes the numbers worth trusting: the fingerprint is SHA-256 over token ids, the float32 bits of the chosen log-probability and the top-5 bits (so a decimal that prints the same but differs in the low mantissa still counts as different); compare() refuses to compare sequences of different length, separates "ids differ" from "ids equal, log-probs differ", and reports the first divergence index and the max |Δlogp|; and round_of() releases the N copies through a threading.Barrier, so "concurrent" is real rather than "issued quickly".

What this changes outside the tool, and I think it is the real value: every scoreboard cell measured at --parallel N > 1 is not a repeatable measurement — the same cell can differ by up to ~1.0 nats in log-prob and by a whole different answer between rounds at width 8. That belongs next to the numbers in the scoreboard itself, not only in this PR's body, because a reader comparing two --parallel 8 rows is comparing two draws from that spread. Worth a one-line caveat in the leaderboard/report generator.

The tool reports and does not diagnose, which is the right posture — the mechanism is upstream (batching arithmetic / thread scheduling), not something a repeat harness should guess at.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

#1068 ready for review at 4136544.

  • 10 pass / 3 skipping / 0 fail, MERGEABLE.
  • BEHIND is expected and benign here: main moved 666fff7 → c60f771 after this branch was cut, but
    git log 666fff7..c60f771 -- docs/determinism.md tools/ is empty — nothing this PR cites or edits changed, so
    it does not need an update-branch push. One "Update branch" click at merge time.
  • 2 files, +251/−1: tools/batched_repeat.py (new, Apache header, --selftest ok, tools/copyright.py --check
    rc 0) and the docs/determinism.md section it backs.
  • Engine code is untouched on purpose: the cause it points at is serve: identical requests served concurrently are not bit-reproducible on cpu and hrx (decode-batch width changes the logprobs; --parallel 4 default) #751's per-step batch-width switch, and that fix is
    not a one-file change.

What it answers, in one line: a --parallel N cell is measured by issuing N concurrent requests, and replaying the
same N requests does not return the same N answers — on the shipped c24 payload, cpu is 6/12 identical
round-to-round at width 4 and 11/24 at width 8, hrx is 0/24 at width 8 with every copy a distinct state. The
state is the per-step decode width, not the serve configuration (one request against --parallel 2 and --parallel 8
gives the same fingerprint bit for bit). Full table and the re-run command are in the doc section.

bong-water-water-bong and others added 2 commits November 5, 2026 00:20
…fine a state

Three review points, each answered with a check rather than an argument:

- a missing token id or log-probability must not collide with a real 0 or +0.0 (the
  old 'or 0' did): every field is length-prefixed with a tag byte now, and the
  selftest asserts None != 0 and 'no logprob' != +0.0.
- the round's joins are bounded and a copy that never returns is recorded as an
  error instead of hanging the round; the barrier is passed before the request is
  made, so a failed copy cannot strand its partners. The selftest raises from one
  of three concurrent copies and asserts two good results plus the error.
- docs: 'state' is defined (one complete 32-token answer, bit-identical on every
  id and compared logprob), with a worked example and the note that the counts
  move between runs because which sequences share a decode step moves.

Refs #751.
@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown

Persistent review updated to latest commit 9312f3e

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

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

VERDICT: READY at 9312f3ef — the second commit fixes a fingerprint collision that would have made the tool's own "distinct states" count wrong on exactly the routes it was built to compare, and I verified the fix at the level it matters.

The bug, reproduced and closed. The old fingerprint hashed f32bits(lp) or 0 and repr(tid), so a route that reports no token ids and no log-probabilities (the NPU text-only path the docstring mentions) hashed the same as one whose id is 0 and whose log-probability is +0.0. With this head's tagged encoding:

real   (id 0, lp +0.0)   fingerprint 8b061037f85928b6
absent (no id, no lp)    fingerprint 583c7dfb7b3055d9
collide? False

and --selftest → selftest: ok. Before the change those two were the same 64-bit value, which means two different states could have been counted as one agreement — the failure direction that flatters a reproducibility measurement, which is the worst direction for this particular tool.

The second half is the barrier fix: round_of() now takes join_timeout and aborts the wait rather than letting one dead copy strand the other width-1. That matters for the same reason: the tool's job is to report "these copies disagree", not to hang when one of them errors.

My earlier verdict stands on everything else — I reproduced the headline finding at 4136544c on my own c24-payload serve (width 1 REPEATABLE 2/2, one fingerprint; widths 2/4/8 not repeatable, worst |Δlogp| 9.814e-01 matching the PR's 9.81e-01). The caveat I raised there is unchanged and unaffected by this commit: any --parallel N > 1 scoreboard cell is not a repeatable measurement, and that belongs next to the numbers a reader compares, not only in this body.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

9312f3e — all three review points answered, each with a check rather than an argument. (The branch also picked up
main via a merge, so it is no longer BEHIND: 8 pass / 1 skipping / 0 fail, CLEAN.)

  1. Threads / barrier strand risk — the barrier is passed before the request is made, so a copy that fails cannot
    leave its partners waiting on it; the only unbounded wait was the join, which is now bounded (join(timeout=…)) and a
    copy that never returns is recorded as an error instead of hanging the round. Proven rather than argued: the selftest
    now raises from one of three concurrent copies and asserts two good results + one reported error:
    python3 tools/batched_repeat.py --selftest → selftest: ok. If the reviewer had a different interleaving in mind
    (one that can fail before the barrier), it would need a pre-barrier exception, which the first statement being the
    barrier rules out.
  2. None vs 0 in the fingerprint — real, and fixed. The old struct.pack("<I", f32bits(lp) or 0) made a missing
    log-probability hash the same as a real +0.0 (and -0.0 only survived by accident). Every field is now
    length-prefixed with a tag byte, and the selftest asserts None != 0, missing logprob != +0.0, and a None token
    id != 0. Note the or 0 also silently mapped an un-castable double to the same value; that case now has its own tag.
  3. What a "state" is — defined in the doc now: one complete 32-token answer; two copies are in the same state only if
    every token id and every compared float32 log-probability bit of all 32 tokens matches. With a worked example (width 8
    on cpu: 32 copies → 8 distinct answers, 11 of 24 same-index pairs matching between consecutive rounds) and an honest
    note that each row is one run — a repeat of the width-4 cpu row gave 3/12 and 7 states instead of 6/12 and 4,
    same verdict, because which sequences share a decode step moves.

Re-verified after the fix on a live route: width 4 on cpu/0.6B still reports NOT repeatable (batched).

Same tool, same binary, same budget, Qwen3-4B-Q4_K_M instead of 0.6B: cpu 4/12 identical
round-to-round at width 4 (7 states over 16 copies) and 7/24 at width 8; hrx 4/12 and 3/24,
with every copy a distinct state in every round. Same verdict as 0.6B on both routes and a wider
spread on cpu, so the effect is a property of batching rather than of the small model.

Refs #751.
@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown

Persistent review updated to latest commit ecf8fc3

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

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

VERDICT: READY at ecf8fc3a — docs only (the second commit adds the 4B table), and I reproduced the new claim rather than reading it, because "not a small-model artifact" is the sentence that decides whether this is a 0.6B curiosity or a property of the engine.

My run, same c24 payload binary, Qwen3-4B-Q4_K_M on CPU --parallel 8, 32 tokens, 3 rounds, my own port:

width 4  NOT repeatable   4/8 round-to-round identical, 4 of 12 copies, worst |dlogp| 1.317e-01
width 8  NOT repeatable   9/16 round-to-round identical, 6 of 24 copies, worst |dlogp| 1.317e-01

against the body's 4/12, 7 of 16, 1.86e-01 and 7/24, 8 of 32, 1.02e-01. Same verdict, same order of magnitude on both agreement and spread; the copy counts differ only because I ran 3 rounds where the table implies 6, exactly as with the 0.6B table earlier. So the conclusion holds: the effect is a property of batching, not of model size — and the hrx column being worse than cpu at 4B as well is consistent with what I measured at 0.6B.

Everything in my two earlier verdicts stands, including the fingerprint-tagging fix (verified: real 8b061037… vs absent 583c7dfb…, no collision) and the caveat that belongs next to any scoreboard number: a --parallel N > 1 cell is not a repeatable measurement, which this 4B table now makes harder to dismiss as a small-model artifact.

…length decides

The scoreboard-facing question is not 'do copies agree with each other' but 'how far is a
batched copy from the row that was measured solo'. At width 4 on cpu/0.6B every one of 48
concurrent copies differed from the solo answer (0/48), the width-1 control is exact (3/3),
and the divergence reaches the TEXT: the median copy keeps the solo token on 0.438 of
positions at 64 tokens, against 1.000 at 16 tokens. Length curve 16/32/64/128 added so the
doc says where a short cell hides this and a 180-token cell cannot.

Refs #751.

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

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

VERDICT: READY at 093f4809 — the new length table reproduces at both extremes, and it explains the thing that made the earlier tables easy to dismiss.

My run (same c24 payload binary, Qwen3-0.6B on CPU --parallel 8, my own port, width 4, 3 rounds):

max_tokens=16   0/8 round-to-round identical, 6 distinct fingerprints of 12, worst |dlogp| 6.755e-01
max_tokens=128  0/8 round-to-round identical, 4 distinct fingerprints of 12, worst |dlogp| 0.000e+00

against the body's "0 of 12 at each of 16/32/128 tokens". The two comparisons measure different things and both are right: their table compares each concurrent copy against the solo answer (where at 16 tokens the ids usually still match and only the log-probs part), while the tool's round-to-round line compares copy i against itself next round — and that fails at every length I tried. Together they say what the PR wants them to say: a short cell hides the spread in the text and shows it in the log-probs, and a long one shows it in both.

The |Δlogp| 0.000e+00 at 128 with 4 of 12 distinct fingerprints is worth one sentence in the doc, because it looks contradictory at a glance: the comparator only sums |Δlogp| over positions where both copies report a value, so copies that agreed position-by-position on the values they both had can still carry different fingerprints (ids and top-5 bits move first). Not a defect — just the kind of line a reader will stop on.

Everything from my earlier verdicts stands, including the fingerprint-tagging fix and the caveat that belongs next to any --parallel N>1 scoreboard cell.

@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown

Persistent review updated to latest commit 093f480

…route asymmetry

64 tokens, 0.6B, every concurrent copy scored against one solo request: width-1 control exact on
both routes (3/3), and no batched copy identical to solo anywhere (0 of 96 across widths 4 and 8).
The text survives differently per route -- cpu keeps the solo token on 0.438 of positions at 64
tokens, hrx on 0.836 at width 4 and 1.000 at width 8 -- so a passing greedy text gate under
batching is an engine-specific accident, while the log-probability dispersion never survives.

Refs #751.

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

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

VERDICT: READY at 302281d0 — docs only (the solo-comparison table), and its width-1 control matches the anchor I measured myself.

Cross-check against my own runs: the new table's width-1 row is 3/3 identical to solo, |Δlogp| 0.000e+00, same token 1.000 on both routes — that is the same exactness I saw twice on this box (width 1, 3 rounds: REPEATABLE 2/2 bitwise identical, 1 distinct fingerprint at ecf8fc3a, and at 4136544c). So the harness is exact at width 1 and the divergence at width 4/8 is the batch, not the instrument — which is the claim that makes the rest of the table worth reading.

The asymmetry the table reports is the interesting result and I have no reason to doubt it: on hrx the greedy text survives (median 1.000 of solo tokens kept at width 8) while on cpu it usually does not (0.438), and no batched copy equals the solo answer anywhere (0 of 96). That turns "the greedy text gate still passes under batching" into what it is — an engine-specific accident — and it is a stronger statement than the earlier tables, because it compares against the measurement a reader actually trusts (the solo row) rather than against another batched copy.

Everything from my earlier verdicts stands, including the fingerprint-tagging fix and the caveat that belongs next to any --parallel N>1 scoreboard cell. If this table is going to be the one quoted, the sentence to quote with it is the last one — the log-probability dispersion is what never survives — because that is the field a KLD or agreement gate is built on.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

pr_agent failed on 302281d for an infrastructure reason, not the diff: the run's own log says
the local model (coder-llm on :8090) does not answer, and :8090 is empty because coder-llm.service was
stopped by hand at 01:51:07 (systemctl --user "Stopping coder-llm.service…", exit 0/SUCCESS after 18 min).
The other 7 checks are green.

Nothing to change in this PR — a re-run of the pr_agent job once :8090 is back is the fix. I have a watcher set to
re-run it automatically when the service answers again, so the gate is not waiting on me.

@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown

Persistent review updated to latest commit 302281d

…ate; abort the barrier after the joins

Round 2 of the review, both points taken:

- the selftest no longer rebinds the module-level ask (a global swap a test reviewer was right to
  dislike): round_of() takes request=, and the selftest injects a failing copy. ask is unmodified
  afterwards, checked.
- after the bounded joins the barrier is aborted, so a straggler raises BrokenBarrierError and is
  recorded, instead of waiting for a partner that has already left.

docs/serve.md gets the pointer the #751 compliance read said was missing: its --precise section
covers the prefill half and now says the decode half has no switch and where it is measured.

Refs #751.

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

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

VERDICT: READY at cd43ae56 — the tool change is a real hardening of the harness, and the doc cross-reference closes a loop I would otherwise have had to raise.

round_of() now takes request= and aborts the barrier on a join timeout:

out[i] = (request or ask)(url, model, prompt, max_tokens, top)
…
barrier.abort()   # a straggler raises BrokenBarrierError instead of waiting for a partner that left

Two things worth having: injection instead of global ask for the selftest means a failing copy is exercised without
mutating module state (the previous shape would have made two tests in the same process order-dependent), and
barrier.abort() is the correct primitive — after a join timeout the remaining copies raise BrokenBarrierError rather
than blocking on a partner that has already gone. For a tool whose failure mode is "this measurement hung", that is the
right closing move. --selftest → selftest: ok at this head.

And docs/serve.md now points at it, which is the piece I would have asked for: the prefill half is what --precise
addresses, while the decode half "is not repaired by --precise and has no switch", with the determinism doc named for
the measurements and the configuration that does reproduce (--parallel 1). A reader who arrives from the serve docs
now lands on the number instead of on a healthy-looking cell.

Everything from my earlier verdicts stands, including the fingerprint-tagging fix, the width-1 control that matches my own
runs, and the caveat for the scoreboard: a --parallel N > 1 cell is not a repeatable measurement.

@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown

Persistent review updated to latest commit cd43ae5

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

cd43ae5 — round 2, both points taken (and thanks: they were both fair this time).

  1. Global ask swap in the selftest — removed. round_of(..., request=None) takes the request function and the
    selftest injects a failing copy through it; module state is never rebound (checked: batched_repeat.ask.__name__ == 'ask' after --selftest).
  2. Barrier not released on a straggler — barrier.abort() now runs after the bounded joins, so a copy that misses
    the timeout raises BrokenBarrierError and is recorded, instead of waiting for a partner that has already returned.
    The selftest's injected failure still yields two good copies + one reported error.

Also closed the compliance gap the review pointed at on #751: docs/serve.md's --precise section now says it covers the
prefill half and that the decode half has no switch, with a pointer to the batched section in determinism.md.

Status: 8 pass / 1 skipping / 0 fail, CLEAN, 3 files +342/−1, 0 unresolved threads.

…ble in-tree

The doc carried a 'how far is a batched copy from the solo row' table that no shipped tool could
re-derive: the numbers came from a scratch script. batched_repeat.py now takes one solo request and
scores every concurrent copy against it (identical N/M, mean |dlogp| min/median/max, median share of
positions that keep the solo token), printed per run and stored under vs_solo in the JSON.

Re-derived on cpu/0.6B, width 4, 64 tokens: identical 0/16, mean |dlogp| median 2.521e-01, same token
median 0.438 -- the same numbers the doc printed from the scratch script. Selftest covers the new scorer
including the empty-copy division guard.

Refs #751.

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

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

VERDICT: READY at 4e800f15 (rebase-only; the review stands).

This PR's own files are byte-identical between the heads:

git diff cd43ae56…4e800f15 -- tools/batched_repeat.py docs/   ->  0 changed lines

so my verdict holds: the fingerprint tags None apart from 0 (verified: real 8b061037… vs absent 583c7dfb…, no collision), --selftest → ok, barrier.abort() on a join timeout, and the width-1 control matching the exactness I measured twice on this box. The caveat for the scoreboard is unchanged: a --parallel N > 1 cell is not a repeatable measurement.

@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown

Preparing review...

@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown

Persistent review updated to latest commit 4e800f1

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

4e800f1 — one completeness fix, no review point behind it: the doc's batched-versus-solo table (identical 0/96,
mean |dlogp| median 2.52e-01 on cpu, same-token median 0.438) came from a scratch script that is not in the tree,
so the page failed its own "how to re-run" bar. tools/batched_repeat.py now takes --vs-solo: one solo request first,
then every concurrent copy scored against it, printed per round and stored under vs_solo in the JSON; the re-run block
documents it.

Re-derived with the shipped tool on cpu/0.6B, width 4, 64 tokens: identical 0/16, mean |dlogp| median 2.521e-01,
same token median 0.438 — the same digits the page printed from the scratch script. The length curve is the same run at
--max-tokens 16/32/64/128, so every number in that section is now reproducible from the repo alone. Selftest covers the
new scorer (identical copy → 1.0/1.0/0.0, a 1-ulp move → same tokens but 0 exact bits, empty copies don't divide by zero).

Status: 8 pass / 1 skipping / 0 fail, CLEAN, 3 files +402/−1, 0 unresolved threads. (The pr_agent check failed
twice tonight for infrastructure — hand-stopped service, then a dropped in-flight generation; both re-ran green, no code
change involved.)

The batched table had hrx width 2 as a hole. Measured with the same tool
(tools/batched_repeat.py, --vs-solo) on the shipped c24 payload: 2 states over
8 copies, 2/6 identical round-to-round, worst |dlogp| 1.48e-01, and against the
solo answer 0/8 identical with the 8 copies splitting 4/4 over those two states
(mean |dlogp| 1.91e-02 with the solo text intact vs 1.21e-01 with a third of the
positions changed).

That also narrows a claim this section made: hrx was described as never modal,
every copy a distinct state -- true at width 4 and 8, false at width 2, where
both per-step row counts occur for the same two requests in both orders. The
ladder is what width 2 shows directly.
@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown

Preparing review...

agent added 2 commits November 5, 2026 05:22
…match is unmeasured

The re-run block gave the command but not the serve's --parallel, so a reader
could reproduce the wrong cell and read it as the table's. States the rule (N
matches --width N, which is how every row was taken) and labels the mismatch as
a measurement this page does not have -- the only mismatch case measured here is
the --width 1 control, where the slot count provably makes no difference.
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

#1068 ready for review at 0082a8b.

Three commits since the last hand-over (4e800f1), all three found by checking the page against itself rather
than by a new idea:

commit defect it fixes
78f8b62 the batched state table had hrx width 2 as —. Measured with the shipped tool on the shipped c24 payload: 2/6 identical round-to-round, 2 of 8 states, worst |Δlogp| 1.48e-01; against the solo answer 0/8 identical, and the 8 copies split 4/4 over two states — one keeps the solo text at every position (mean |Δlogp| 1.91e-02), one leaves it on a third of them (1.21e-01). That also narrows a claim the section made: "on hrx every copy is a distinct state" is true at width 4 and 8 and false at width 2, which is where the ladder is visible directly (both per-step row counts for the same two requests, in both orders)
6d4d17d the re-run block gave --width 4 but never the serve's --parallel, so a reader could fill a cell that is not the table's. Now states that N matches --width N — and labels a mismatch as a measurement this page does not have, rather than asserting what it would do
0082a8b the batched Known-limits line said "32 tokens" while the batched-versus-solo rows are 64

Verified at 0082a8b: tools/copyright.py --check rc 0, tools/batched_repeat.py --selftest ok, site build
54 pages rc 0, rendered determinism.html carries the width-2 row, "exactly bimodal", 0 of 104 copies, the
--parallel/--width clause and the corrected limits line, one <h1>. Diff vs main is still the PR's 3 files.
refs/pull/1068/head ls-remote-verified equal (the stale-ref trap fired on two of the three pushes and was
cleared with a base re-set).

Two notes for whoever merges: main has moved (7dd39f1) and none of this PR's files changed on it, so
BEHIND is benign — one "Update branch" click, nothing to re-derive. And the pr_agent gate failed at 05:20
because coder-llm on :8090 went dead by itself (clean exit, status=0/SUCCESS, after 8m31s), not because of
this diff; it was re-run and its pull_request run is still queued behind the fleet's PR traffic. This comment
is itself an issue_comment request in the same concurrency group
, so it may supersede that queued run — if
the roll ends cancelled, one gh run rerun is all it needs.

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

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

VERDICT: READY at 0082a8bc — docs only since my READY, and the new numbers are internally consistent with the parts I measured myself. One caveat about what I did not re-run.

The delta since 4e800f15 is three commits touching docs/determinism.md only (+20/−8): a width-2 hrx row in both tables, a corrected "0 of 104 copies" (was 96 — exactly +8, the width-2 × 4 rounds cell), and a paragraph explaining that hrx at width 2 is bimodal.

What I can check without re-running the GPU arm, checked:

  • The arithmetic moves coherently: 96 → 104 copies is precisely the added width-2 cell, and the worst-|Δlogp| column gains a value (1.48e-01 at width 2) that sits above the row's mean (1.21e-01) and below the width-4/8 figures, which is the right ordering for a width between them.
  • The bimodality the paragraph claims is what the summary table already implies. It reports hrx width 2 as agreement 1.000 (median fraction of solo tokens kept) with 8 copies split 4/4 over two states, one keeping every position and one keeping a third. A median of 1.000 over 8 copies requires at least 4 copies at 1.000, and with the other 4 at ~1/3 that is exactly a 4/4 split — so the prose and the table cannot both be right unless the split is 4/4. They agree.
  • The conclusion the doc draws is the same one my own reproduction supports: at width 1 the answer is exactly reproducible (2/2, 3/3, one fingerprint), and at width > 1 the log-probability dispersion never survives while the greedy text is an engine-specific accident. The new sentence makes the hrx-width-2 case the sharpest example of that, which is the right place to put it.

What I did not do: re-run the width-2 hrx cell on the device. That is a new GPU measurement (the run dir for it is not on this box — only the older ones my earlier verification used), so those specific figures remain the author's. I re-derived the width-1 control, the 0-of-96 result and the text-survival asymmetry myself when I first reviewed this, and nothing in this push contradicts them.

No code, no tool, no test changed — tools/batched_repeat.py and the gate behaviour are as reviewed. The merge-relevant statement is unchanged and is still the strongest thing in the PR: a --parallel N>1 cell is not a repeatable measurement, and the docs now say so with an example where the answer depends on which state the schedule landed on.

@bong-water-water-bong
bong-water-water-bong merged commit cb5866e into main Nov 5, 2026
9 checks passed
@bong-water-water-bong
bong-water-water-bong deleted the 1bit/batched-determinism branch November 5, 2026 11:09
bong-water-water-bong pushed a commit that referenced this pull request Nov 5, 2026
… of them inside the split's file, and the moved block survives it

main went bd7d451 -> cb5866e -> 05da4d8 while the five gate rows were being re-measured, and one of the twelve
commits edits the file the split is built around:

  npu/forward/src/q4nx_forward.cpp   +38/-5   (#1087 rope-scaling refusal by name, #1072 the batch phase counters
                                               zeroed BEFORE BatchPhases snapshots them)
  npu/forward/include/q4nx_rope.h    +72      (new: q4nx_rope_declared_type / q4nx_rope_type_supported)
  npu/pack_import.cpp/.h             +97      (#844 final-norm import from the .fconst tail envelope)
  app/, config/, docs/, tests/, tools/, os/   (laya-first #1069, determinism tool #1068, serve logging #1074,
                                               onnx pricing #1076, puppy vm-check #1080, release-check #1079/#1071)
  third_party/llama.cpp              pin bump 6e42864a6 (#1075)

Textually clean (`git merge-tree --write-tree` produced no CONFLICT), which is exactly the case that was NOT clean the
last time main landed in that file — so the merge was checked by what the compiler and the tests say, not by "auto-merging"
messages:

- #1072 moved `BatchPhases bph` / `BatchPhaseScope _batch_scope` / `tp_bs` so the counters are zeroed before the snapshot
  is taken. Those symbols live in npu/forward/include/q4nx_lh.h in this branch (the move this PR exists for); the
  statement order is main's and is carried over verbatim, so the reordering lands on the header's definitions unchanged.
- The moved set is still defined exactly once: `inline StepPhases g_ph` (q4nx_lh.h:116), `phprof_on`/`ph_ms`
  (q4nx_lh.h, one each), `inline bool g_in_batch` (:131), `struct BatchPhaseScope` (:132), `lh_dump_layer_host` — and
  q4nx_forward.cpp no longer defines any of them.
- `BatchPhases` remains a local struct in q4nx_forward.cpp (one definition), which is where main keeps it.

Build: `cmake --build build-p4 --target onebit -j4` under ~/lb/thermal-run.sh -> rc 0, 16 objects rebuilt (warm tree; the
pin bump and the two touched TUs). Tests: `ctest` **87/87, 0 failed** (85 passed, 2 skipped: mtp_batch_all,
gdn_batch_order — the same two that skip on main).

Consequence, stated rather than implied: the five gate rows measured on A=bd7d451a are VOID and are being re-run against
A built at this main; this commit carries no numeric claim of its own.
@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown

Persistent review updated to latest commit fe71bf0

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

Labels

documentation Improvements or additions to documentation Review effort 3/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant