Repository navigation
npu: the t2s leaderboard rows say one run, and what the pair did not test - #1284
Conversation
…test Two corrections to the rows merged in #1276, both raised in review: - 'repeats': 7 read as seven repeats of the measurement, but the 7 is the number of steady-state tokens inside ONE request whose median is the recorded value. repeats is now 1 and the instrument says 'one run per arm ... median of its 7 steady-state tokens', which is what was done. (Same mismatch pi-prs flagged on #1206's pooled MTP rows.) - the t2s2 row's notes claimed 'greedy text identical on the three-prompt window'. That window was never run for this pair: what was measured is role-level byte equality with the pb16i path (6/6 roles at layer 0, 8/8 at layer 3, three seeds) plus text equality on the one prompt both arms were asked. The 2026-11-06 re-take that would have closed it was blocked by an unrelated serve holding the engine's device lock, so the notes now name that as the open acceptance step instead of implying it was done. Numbers unchanged: 313 vs 410 ms/token, 1.31x, 1.42x on the device wait, 2.12x on bytes. docs/npu-leaderboard.md regenerated (21 records valid).
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at 0622b039 — this is the repeats point I raised on #1276, fixed the right way, plus an overstatement in the same record that I had let pass.
What it changes: both rows "repeats": 7 → 1, and the instrument now says "one run per arm: the serve's own [ph] tok wall= line, median of its 7 steady-state tokens". That is the distinction the field was carrying wrongly: the seven tokens are inside one request, not seven measurements. bench/npu-leaderboard/README.md only requires a positive integer, so 1 is valid and says what happened.
And it retracts something I should have caught in #1276. My review quoted the t2s2 record's notes as agreeing with the page ("greedy text is equal … on the prompt both were asked") — the record itself then went further, claiming "greedy text identical on the three-prompt window". This PR replaces that with the truth plus the reason: equality is on the one prompt both arms were asked (' Paris.\nThe capital of Germany is'), and "A three-prompt full-model text window on this pair was NOT run — the 2026-11-06 re-take was blocked by an unrelated serve holding the engine's device lock — so that remains the open acceptance step." That is the correct shape for a record: the acceptance it did pass (every decode role byte-identical, 6/6 at layer 0 and 8/8 at layer 3 on three seeds) stays, and the one it did not is named as open instead of implied.
Checked here:
tools/npu_leaderboard.pyregeneratesdocs/npu-leaderboard.mdbyte-identical at this head (git statusclean after the run), and--checkreports 21 record(s) valid — so the table'srepeatscolumn showing1for both rows is the generator's output, not a hand edit.- The generated bullet in the doc carries both corrections (grep:
one run per arm×2,was NOT run×1), and the old wordinggreedy text identical on the three-prompt windowis now absent from bothdocs/npu-leaderboard.mdanddocs/npu.md. - The measured values are untouched (313.0 / 410.0 ms/token), so the 1.31x the row exists for still follows: 410/313 = 1.3099.
- Three files,
+8/-8, records + generated doc in one PR; no code, no test, no device.
Nothing outstanding: the value, the unit, the provenance and now the caveat all describe one adjacent-window pair.
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at f7b68ec0.
Both of the things review left open are closed, and nothing else moved. With a current main (f2755a1d, which is an
ancestor of this branch) the whole diff is three files and +8/−8, exactly as the body says — the two records and the
generated page:
tools/npu_leaderboard.py --check -> 21 record(s) valid rc 0
regenerate vs committed docs/npu-leaderboard.md -> BYTE-IDENTICAL
docs/npu-leaderboard.md diff -> 4 changed lines, all Bonsai, 0 others
numbers -> 313 vs 410 ms/token, 1.31x, 1.42x, 2.12x (unchanged)
(the docs/hetero-prefill.md in my first stat was my own stale origin/main, not this PR — re-checked against a fresh fetch.)
repeats now says what was done: 1, with an instrument that begins "one run per arm … median of its 7 steady-state
tokens". That is the honest reading, and fixing it here is the same discipline the #1206 pooled rows needed.
The acceptance claim is now the one that was measured, which is the part that mattered: byte-exactness on every decode
role (6/6 at layer 0, 8/8 at layer 3, three seeds) plus greedy text equal on the single prompt both arms were asked, with the
three-prompt full-model window named as not run and the reason given. That is stronger than a text diff on the layer
evidence and narrower on the model evidence, and the record now says exactly which is which.
One nit, in the same spirit: the t2s2 record's source still reads "three npu-hold tickets (19:47, 19:48, 19:49 AST)",
while the instrument now says one run per arm — a reader can take three tickets for three repeats, which is the confusion this
PR just removed from the repeats column. "three tickets: one per arm plus the byte-exactness run" would close it, or leave
it if the third ticket's job is already obvious from the source line.
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at 96c9da98 — main merges only; the reviewed content is byte-identical to the head I verdicted.
Checked rather than assumed: the PR's own added lines (own diff vs merge-base, which excludes everything main brought in) hash to 41513ca735d3 at the head I reviewed and to the same value here, so no line of this PR's change moved — only its base did (the branch now sits on newer main).
Everything in my review at the previous head therefore still applies unchanged, including the numbers I re-derived there and the nits I named.
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
Follow-up to #1276 (merged as
b24eb570), for the two things review left open on those rows. Records and the generated leaderboard only — no prose, no numbers change.1.
repeatsnow says what was done. It was7, which reads as seven repeats of the measurement; the 7 is the number of steady-state tokens inside one request whose median is the recorded value.repeats: 1plus an instrument that begins "one run per arm … median of its 7 steady-state tokens" says it without needing the reader to decode the column. This is the same mismatch flagged on #1206's pooled MTP rows, whererepeatsdescribed how many windows were pooled rather than how many runs.2. The t2s2 row no longer claims an acceptance it did not have. Its
notessaid "greedy text identical on the three-prompt window". That window was never run for this pair. What was measured is stronger in one sense and narrower in another:' Paris.\nThe capital of Germany is');[npu] waiting for the exclusive device lock— an unrelated orphaned host-GEMM serve holding the lock while touching no array (reported on npu plan P3: t2 on-core expansion kernel for the sealed M=16 family (XDNA) - halve the weight stream #802 and to the orchestrator). The notes now name that as the open acceptance step.Both rows keep their numbers: 313 vs 410 ms/token, 1.31x per token, 1.42x on the device wait, 2.12x on bytes.
tools/npu_leaderboard.py --check→ 21 records valid;docs/npu-leaderboard.mdregenerated from the records, and the diff is the two Bonsai rows only.Refs #802