Repository navigation
measure(arcspec): per-sequence KV advance does not generate at batch — root cause, and the instruments that found it - #114
Conversation
Code Metrics Report━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ Language Files Lines Code Comments Blanks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ C Header 5 305 210 52 43 CSS 2 1181 1036 34 111 CUDA 73 25129 18043 4319 2767 Dockerfile 1 39 22 8 9 JavaScript 16 3546 2676 482 388 Jinja2 7 694 656 5 33 JSON 74 4600 4597 0 3 Makefile 1 6 5 0 1 Metal Shading Lan| 33 12224 9431 1142 1651 PowerShell 1 300 227 30 43 Python 145 15139 12482 811 1846 Shell 37 8630 5732 2323 575 Plain Text 4 3801 0 2479 1322 TOML 33 1498 1294 54 150 YAML 3 25 23 2 0 ───────────────────────────────────────────────────────────────────────────────── HTML 4 2687 2604 43 40 |- CSS 2 543 479 37 27 |- JavaScript 1 1233 1215 12 6 (Total) 4463 4298 92 73 ───────────────────────────────────────────────────────────────────────────────── Jupyter Notebooks 4 122 83 23 16 |- Markdown 1 60 30 22 8 |- Python 1 122 113 1 8 (Total) 304 226 46 32 ───────────────────────────────────────────────────────────────────────────────── Markdown 203 44777 0 34735 10042 |- BASH 72 1654 1202 331 121 |- C 3 17 17 0 0 |- CUDA 2 84 56 16 12 |- JSON 18 708 708 0 0 |- PowerShell 1 1 1 0 0 |- Python 23 1008 787 113 108 |- Rust 66 2051 1716 77 258 |- TOML 6 207 164 0 43 |- YAML 5 41 36 5 0 (Total) 50548 4687 35277 10584 ───────────────────────────────────────────────────────────────────────────────── Rust 671 324690 280167 16004 28519 |- Markdown 489 27687 471 23888 3328 (Total) 352377 280638 39892 31847 ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ Total 1317 484809 346283 87048 51478 ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ |
|
ArcGate: HELD — parked above #104, and the reason is now specific rather than open-ended. Full analysis on #104. Short form: #104 and #122 (landed as master
#104's Two things that are not true, both checked rather than assumed:
This is scope, not a verdict. Nothing here is ranked down or closeable; the blocker is now one A/B measurement — does #104's design buy anything #122's does not, priced against its per-step rebuild. Also note: this PR currently shows 1 check, the comment bot — zero CI lanes have ever run on it, because it predates the stacked-PR trigger fix that is now on master. Any push starts the full 16 lanes. Nothing deleted; branch untouched. |
…TN_BACKEND)
Expert parallelism was priced on the belief that building with NCCL silently
disables flash attention for V4 — `attention/mod.rs`'s `use_nccl() =>
naive_sdpa` gate — which would make an EP=2-vs-EP=1 comparison a comparison of
two different attention kernels rather than of expert parallelism.
Reading the dispatch says otherwise for V4, in three steps: V4 loads
`attn_sink`, so `sdpa_params.sinks` is `Some`; `Sdpa::run_attention` therefore
diverts to `sinks_attn` on its first line, before `can_use_flash` and long
before the `use_nccl()` gate, which lives in `run_attention_noflash`; and flash
was never reachable at head_dim 512 regardless (`FA2_MAX_HEAD_DIM` is 256, FA3
takes {64,128,256}, `flash_sinks_ok` takes {64..256}).
That is a code read, and a $9.22/hr 2xH100 pair is too expensive to book on
one. These two `OnceLock`-gated lines make it an observation: one names the
backend `sinks_attn` chose, the other fires only if a model actually reaches
the `use_nccl()` gate. For V4 the second must never appear — presence and
absence are the assertion, rather than a comment asserting it.
No numerics change: two log lines, each emitted once, off the hot path.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…st the grant `kv_advance()` logs "per-sequence KV advance is ON" once, at init, when the mode is GRANTED. That line has been this project's proof of a live treatment arm all evening. It does not prove the mode did anything: `per_seq_advance` (`mtp_pipeline.rs`) is recomputed every step from `assembled_here && post_op == Out && kv_advance() == PerSequence`, and can be false on every step while the grant stands. The gap is not hypothetical. A proposed optimisation — gate the batched-cache re-assembly on `rows_uniform()` — would have made `assembled_here` false on every steady-state step, so the mode would have been permanently inert. Cost would have dropped to zero at every batch size, the throughput chart would have read as a clean win, the init line would still have said ON, and every engagement check in the harness would have passed. It was withdrawn only because the semantics were read before it was written. So the counter is the assertion: `per_seq_steps` on the `MTP[...]` marker says how many decode steps actually applied per-sequence advance. Granted-but-inert is now a visibly zero field rather than an absence, which is the same shape of fix as naming the attention backend. Appended at the end of the marker so parsers that look up named fields are unaffected; 68 mtp tests pass unchanged, including both CPU token-identity tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`arcspec_perseq_ab.sh` — per-sequence KV advance ON vs OFF, `tok_per_step` differenced out of the engine's cumulative `MTP[agg]` counters across a per-cell wall-clock fence. Two arms in one binary, plus a drift-control arm that re-runs the control after the treatment so the comparison is bounded for time drift. `arcspec_token_identity_b8.sh` — greedy token identity at B=8 against a strictly sequential B=1 reference, in both arms. The OFF arm is the control and is not optional: if batching alone changes output, no ON divergence is attributable. Both refuse rather than report when they cannot measure honestly, because every guard here was earned by something that went wrong on hardware tonight: * provenance — the running server's baked `git revision:` must equal the ref the script built. A sibling harness built `-p arc-cli` (binary `arc`) and launched `target/release/mistralrs`, a stale binary from an unrelated build, guarded only by `[ -x "$BIN" ]`. For an A/B that is the worst failure available: both arms run the same stale code, the ratio is ~1.0, and it reads as an honest refutation. * counter monotonicity — a negative delta means the parse crossed a restart, so it reports NA rather than the number it would otherwise produce. * crash watchdog — a cell that crosses a CUDA fault is `VOID`, never a number. Note the shape of `crash_count`: `grep -c` prints `0` AND exits 1, so the obvious `|| echo 0` emits "0\n0", the integer test errors, and the guard is silently dead while looking installed. Found by testing the guard, not reading it. * owner-matched lock release — a lock is only removed if it carries this process's own pid. Tonight I deleted another chain's live GPU lock because I saw the file, assumed it was my leak, and did not `cat` it first. The owner tag was already there; nobody was checking it. * completion graded independently of text — a diverging control invalidates comparing *what* was generated, not *whether* anything was. The first version returned "uncontrolled" for everything and so misclassified a one-token failure to generate as an unattributable text difference. Driver counts streamed SSE deltas, not completed requests: at B>=32 V4 finishes no request inside a 60s window, and a completion-counting driver reports 0.00 tok/s with zero errors, which reads as a crash and is not one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…apart from text Two defects found by running these harnesses on a shared box, both of the same shape: a guard that is safe in isolation and wrong in company. **The lock could deadlock the box permanently.** Owner-matched release — only remove a lock carrying your own pid — stops one chain deleting another's lock, which I did tonight by seeing a file, assuming it was my leak, and not reading it first. But it cannot self-heal: a lock whose owner died, or one created empty with no owner tag at all, is unremovable by every well-behaved participant. That is not hypothetical either — a size-0 lock appeared, its presumed owner was dead, the H200 sat at 0% for six minutes, and the run waiting on it would have aborted after 40 minutes having measured nothing. `lock_is_stale` reclaims, but only on evidence and never on one signal alone: older than a grace period (so a lock caught mid-write is never stolen), AND empty or its named pid is dead, AND the GPU is idle by both memory and compute processes. Tested across all five cases, including the two that would do harm: a lock owned by a live pid is never taken, and a dead owner with a busy GPU is never taken. **The comparator misclassified the failure it exists to catch.** It graded only text identity, so a diverging control returned "uncontrolled" for everything — including an arm that returned one token with no `finish_reason` while its own B=1 reference returned full-length. A diverging control invalidates comparing WHAT was generated; it does not invalidate comparing WHETHER anything was. Completion is now graded on its own axis and reported first. Also worth recording, because `bash -n` did not catch it: an earlier attempt to lift the lock block between scripts by regex deleted `acquire_lock`'s definition while leaving its call site. The script parsed clean — `bash -n` checks syntax, not name resolution — and would have failed at runtime on the box. Verified here by running the script far enough to acquire and release. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
612b3a1 to
c2e2b16
Compare
|
Rebased onto Verified independent of #104: neither (The two Two conflicts resolved — both because master moved the code this instrumented
|
This PR was still in the false-green state after being rebased. Now fixed — and it exposes a limit of the new guard.Rebased onto master and retargeted, but its checks were: That was the only one. No WhyTwo events had to line up:
Fixed by close/reopen, which fires The limit this exposes, worth stating plainlyThe The thing that actually protects the merge is branch protection requiring So the two layers are complementary and neither is redundant:
The residual gap is visual, not procedural: a reviewer scanning the checks list still sees one green check and no red. Worth a follow-up if that matters — but it cannot be closed from inside |
1 participant
Measured on hardware first, then written up. H200 SXM5, V4 + qtip2b UQFF, ref
be29f397, provenance asserted (the running server's bakedgit revision:matched the ref this run built, checked per arm).The headline: per-sequence KV advance does not generate at batch
ARC_V4_XS_PER_SEQ=1 ARC_MTP_PER_SEQ_KV=1at B=8 returns exactly one token per request,finish_reason: None, on two independent runs. Its own B=1 reference returns full length, and the control at B=8 generates normally.Root cause —
kv_cache/xs_rolling.rs:427,set_row_lens:tail_width()isself.tail.dim(1)— the raw window is a dense[B, W, …]tensor with one W, sized for the longest row. The compressed rows are genuinely per-row (tokens/base); the raw end-anchored window is not. The moment rows diverge, the shortest hast < w,clone_in_cache'sset_row_lensrefuses, the completion step errors, and the sequence dies after its first commit.This explains every observation with no further assumption: B=1 works (one row,
t == w); B=8 with the flags off works (cohort rollback keeps rows uniform); B=8 with the flags on dies immediately when prompts differ in length; and it is deterministic because it is an invariant check, not a race.The trigger is prompt-length diversity, not generation length. An earlier sweep drove one identical 40-word prompt to all 8 workers, so rows started uniform and drifted slowly — 2,288 chunks before a row fell behind. The identity harness sent 8 prompts of different lengths and died at step 1.
Fix direction (not in this PR). The invariant is correct; the representation cannot express what per-sequence advance produces. Ranked:
[B, W]tail and carry per-row valid widths, with the reader masking. This mirrors whatfront_pad_kv_cache/drop_dead_prefixalready do for Normal slots, so the shape of the solution is already in the tree. It is a narrower first step and it is not the general answer.Do not relax the check. Serving a short row's window from columns that are not its own is a wrong answer nothing downstream catches — the FP8-KV failure class this project has already paid for once.
Retracted: the performance numbers from this chain are void
Stated plainly rather than quietly dropped. All of the following were measured against a treatment arm that was erroring its sequences, and none should be carried forward:
tok_per_step+13.2% at B=128; −20.3% aggregate at B=32; +156 ms/forward; and the diagnosis that unconditional per-step cache re-assembly (issues_cache_in's|| granted) was the cost. Thelead_paddecoupling that followed from it is moot until the chain generates.Three further retractions from the same session, kept because the reasoning is the useful part:
I killed my own headline. The +13.2% at B=128 rested on
batch_steps=2— two engine forwards in a 60 s window — and on a slice where MTP accounted for 15% of the tokens actually emitted (driver 50.0 tok/s vs engine 7.33). The drift control agreeing to −0.3% proves the control is reproducible, not that the row means anything.I killed the MTP-skip hypothesis I was about to build the story on. The arms log different skip reasons and the ON arm's is per-sequence-specific, which looked like the mechanism.
drafted_steps/stepsis 100% in every arm at K=1/8/32, so drafting was never skipped; the warning is once-per-process, so its count measures nothing.I proposed a fix that would have silently disabled the feature. Gating cache re-assembly on
rows_uniform()looked like a pure optimization.per_seq_advancerequiresassembled_here, set only on theInarm, so skipping assembly on a uniform batch means the cohort rule runs, rows never diverge, and the gate never opens. Cost would drop to zero at every batch size, the chart would read as a clean win, and the init line would still say ON.That third one is why this PR adds
per_seq_steps.What this PR contains
ARC_ATTN_BACKEND— names the attention backend once per process. Settles an open question about expert parallelism on hardware: V4 loadsattn_sink, soSdpa::run_attentiondiverts tosinks_attnon its first line and never reaches theuse_nccl() => naive_sdpagate; flash was unreachable at head_dim 512 anyway. Measured:sinks_attn head_dim=512 flash_sinks_ok=false => unfused matmul + softmax_with_sinks, with therun_attention_noflash REACHEDmarker absent. An EP=2-vs-EP=1 comparison on V4 is therefore not a comparison of two attention kernels, and does not need a 2xH100 rental to establish that.per_seq_steps— counts decode steps on which per-sequence advance actually ran, versus the once-at-init line that only says it was granted. Granted-but-inert becomes a visible zero rather than an absence.arc-tools/arcspec_perseq_ab.sh,arc-tools/arcspec_token_identity_b8.sh) that produced these numbers. Both refuse rather than report when they cannot measure honestly: provenance assertion against the server's baked revision, counter-monotonicity, a crash watchdog that marks a cellVOID, owner-matched lock release with evidence-based stale reclaim, and completion graded independently of text.The token-identity harness is the acceptance gate for the fix: it reproduced this twice, deterministically, in ~8 minutes per run, and passes only when the ON arm generates.
Also found, and independent of this chain
The engine is not batch-invariant with the flags off. Control, greedy,
temperature 0, identical prompts:OFF b=8diverges fromOFF b=1on 4/8 and 5/8 prompts across two runs — full-length generations both sides, different text. Some of this class is ordinary GPU batch-invariance, but it means greedy output is not reproducible across batch composition on this build, and any A/B comparing text at batch is unsound until it is characterised. Needs its own owner; it never touches the per-sequence path (zero occurrences in the control log).The long-prompt death does not reproduce. 40/128/512/1100-word prompts all streamed clean (0.8/1.2/3.2/6.6 s), past the ~1,055-word point recorded in FACTS. Cost 12 seconds of box time.
Why the CPU tests did not catch any of this
batched_ragged_accept_is_token_identical_to_the_b1_referenceandper_sequence_advance_is_token_identical_to_the_b1_referenceboth pass, unchanged, alongside all 68 mtp tests. They areSimSeq/sim_steparithmetic: fabricated tokens through a scheduling simulation that never builds a KV cache, never calls attention, and never runs the model. They are structurally incapable of observing a ragged-KV defect, so their passing is not evidence. That gap is itself a finding.