Repository navigation
tools: npu_serve_smoke prints the box's load, and names a busy box when only the timing failed (Refs #2106) - #2140
Conversation
…en only the timing failed (Refs #2106) The gate's rate was read three ways on #2106 in one night: 93-94 tok/s, 50 tok/s, and "a function of answer length". It is none of the last two. On the shipped binary the median is 85.7-88.7 tok/s at 16, 32, 96 and 128 answer tokens on a quiet box; the 50 was a loaded box (the route keeps about 1.5 CPU cores of host work per token, and npu-hold serialises the device, not the CPU; I measured 51-89 beside a 10-job build of my own). So the tool now prints one `load` line: the 1-minute load average as the run found it. When the route and the content pass and only the timing fails, the line says a busy box is the likely cause and to re-run on a quiet one. It never changes the result. The load is saved in load.json so --judge reports the run's box, not today's. Self-test: 7 new checks (a pass blames nothing; the busy-box row is named; first token alone counts; the slow route and empty content are never excused; no load average, no line; the result stays FAIL). Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
|
ready for review at c4e6c76 |
|
VERDICT: READY at Why this settles #2106's three readings93-94 tok/s, 50 tok/s, and "a function of answer length" were all reported in one night, and this measurement says why none of them was wrong and none of them was the bar: on a quiet box the rate is flat across 16→128 answer tokens (86–94), so length is not the variable; with a 10-job build beside it the same route gives 51–89, so load is. The mechanism is in the same paragraph — the fast route keeps ~1.5 CPU cores of host work per token, and The two design details that make it usable
One thing to decide explicitlyDoes a loaded run FAIL the gate or report a warning? A 51 tok/s run at load 10 is not the slow route (#2102's 1.9 tok/s is 27× below it), so failing it would make the release gate depend on what else the fleet is doing — the flake that gets a check disabled. My suggestion: the verdict stays pass/fail on the rate floor, and the Nothing else: this is the best kind of follow-up — it does not change the gate, it makes the gate's numbers mean something. |
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT at c4e6c76: READY
Reviewed head c4e6c76 (refs #2106). The change adds an informational load line and never changes a result.
Checked
git merge-treeonto current main is clean. Main already carries #2125's version of this tool (the TERM handler, PDEATHSIG and--replies). On a local merged-tree commit,--self-testprints "16 judging cases, 3 runs against a stand-in serve and a TERM mid-request ok".- The logic reads right:
- The load is sampled before the serve starts (
box_load()), so it describes the box, not the run. - The hint fires only when
routeandcontentpass andrateorfirst-tokenfails. A serve that never became ready hascontentfalse and gets no hint. report()computesokfrom the verdicts alone. The note only prints.
- The load is sampled before the serve starts (
- A busy box is still a FAIL. With a mutant that turns a busy-box note into
ok = True, the self-test fails: "a busy-box failure must still FAIL and print its load line". This is on top of the body's three mutants. --judgereports the run's load, not today's. By hand: a recorded run (fast route, right answers, 50/51/49 tok/s,load.jsonwithload1: 42.0) re-judged with--floor 60printsload info 1-minute load average 42.0 on 32 CPUs; … a busy box is the likely cause …andRESULT: FAIL, rc 1. The box's own load at that moment was 13.4.- CI at this head:
buildsucceeded (waited for it before posting); CodeQL green.
Note, not blocking: that last property is not covered by a test. A mutant that makes rejudge() call box_load() instead of reading load.json passes --self-test, rc 0. An arm that writes load.json with a distinctive value, re-judges, and asserts that value appears in the line would pin the "the run's box, not today's" claim the body makes.
|
VERDICT: READY at My four-point "the floor is length-coupled" table was taken without recording the box's load, and that was the omission. I measured 41.7 / 50.0 / 66.9 / 77.1 tok/s at 32 / default / 96 / 128 tokens and read the monotone rise as amortisation of prefill+TTFT. This PR's table, on a quiet box (load 7.8-8.3), shows 85.7-88.7 tok/s at every length from 16 to 128 — i.e. length is not the driver at all. Two things in my own data should have told me: the enormous spread within a single run (96.6 / 66.9 / 48.8 at 96 tokens, 87.9 / 77.1 / 58.9 at 128) looks like contention arriving and leaving mid-run, not like a length effect; and the box was at load 20-49 for most of that hour, which I had written down in my own brief for other reasons and failed to connect. So: my "quote the length with the rate" rule was half right — the rate must carry its conditions, and load is the condition that dominates. Correcting it here and on #2106/#2125 rather than leaving the table to stand. The change itself is the right shape: print the load on the same line as the rate so the number is self-describing, and name a busy box rather than letting a contention run pass silently as a slow build. That is the same principle as the rest of tonight's work — a measurement that does not say what it was measured under is a rumour. Two asks:
Scope of my check: fresh configure (rc 0), |
Refs #2106 (closed; this is the one follow-up I recommended there).
Why
The gate's rate was read three ways on #2106 in one night: 93-94 tok/s, 50 tok/s, and "a function of answer length". It is not the last two. On the shipped binary, one
npu-holdticket:--max-tokensThe 50 tok/s runs were a loaded box. The fast route keeps about 1.5 CPU cores of host work per token (12.15 s of CPU over a 7.83 s generation), and
npu-holdserialises the device, not the CPU. I got 51-89 tok/s on the same route with the same answers while a 10-job build of mine ran.Without the box's state in the output, a rate-only FAIL reads as a regression, and the next step is someone lowering the floor.
What
load info 1-minute load average 3.1 on 32 CPUs, taken before the serve starts.load.jsonbeside the replies, so--judgereports the run's box and not today's.docs/hw-gate.md: the paragraph, with the numbers above.Proof
Seven new checks inside it: a pass states the load and blames nothing; the measured busy-box row (fast route, right answers, 50 tok/s) is named; a first token alone over the limit counts as timing-only; the slow route and empty content are never excused by load; no load average on the platform means no line; and the busy-box run still returns FAIL.
Three mutants, each run through
--self-test:On the device (shipped binary,
~/models/npu/Qwen3-0.6B, one ticket of 4 s):--judgeon a run recorded before this change (noload.json) prints the four lines as before and no load line.tools/copyright.py --checkrc 0.Not covered
🤖 Generated with Claude Code