Repository navigation
npu plan: the KV cache contract (P7a), and the arithmetic it rests on gated in CI - #1273
Conversation
… gated in CI Scoped first step for #806 (plan P7). #806's own last comment names the gap: the design audit lives in a private brief and "the plan has no kv.dtype field yet" (docs/npu-plan.md, its own words under "Not yet"). The rest of P7 needs P4 and the private kernel repo; the contract and the arithmetic do not, and they are the part that can be wrong silently. CPU-only: no device, no ticket. plan.h grows `kv {key, value, hadamard {block, signs}}`, defaulting to f16 with no rotation so every plan written before the field keeps loading and means what it meant. int8 means one fp32 scale per (head, token) taken after a per-head Hadamard rotation. Refusals name the field, in the loader's existing style: unknown dtype, a rotation block that is not 0 or a power of two in [2, 65536], a signs entry that is a path instead of a bare name, signs with no rotation block, and a non-string dtype (a type error is a refusal here, not a JSON exception escaping the loader). kv_int8() is what P4's target-capability check will read. Measured, tests/npu_kv_int8_test.cpp (ctest -R npu_kv_int8, no model, no device) -- attention-weight KLD against the f16 cache, head width 256, 512 keys, 8 draws per cell: KV vectors f16+rot int8 K unrot int8 K rotated rotation's worth uniform 3.9e-15 2.5e-05 2.4e-05 1.0x 8x outliers 8.6e-15 4.5e-04 4.7e-05 9.6x 30x outliers 4.5e-14 8.2e-03 1.5e-04 54.5x Two readings. The rotation is free: (QH)(KH)^T = QK^T to 1.6e-09 relative, which is 0.01x one f32 dot product's own rounding -- a rotated f16 cache is indistinguishable from today's, not approximately equal to it. And the rotation is what keeps int8 inside the plan's 1e-3 gate: at 30x outliers the unrotated cache is an order of magnitude outside it (8.2e-03) and the rotated one is inside (1.5e-04). The control row says where it does not matter -- at 8x the unrotated cache already passes -- and on uniform vectors the rotation measures 1.0x, so nothing is claimed on data that has no outliers. Thresholds are set from those columns, not picked: the benefit must grow with outlier severity, must claim nothing on uniform data, and at 30x must straddle the gate. What this does not establish is the gate itself: the vectors are synthetic and generated to the outlier structure docs/npu-plan.md describes, and #806's KLD is taken on real KV from a real model on the device. The test bounds the mechanism; the model run decides. tests/npu_plan_test grows a case per refusal; tools/npu_plan.py now states the contract it writes (f16, no rotation) instead of leaving it to a key's absence; docs/npu-plan.md gains the schema rows, the refusal rules, the measured section and a P7 entry that says what landed and what is left. Does not close #806. Checks: ctest -R 'npu_kv_int8|npu_plan' 4/4 pass (including npu_plan_generate), the CI add_dependencies binding verified in the generated build graph, tools/copyright.py --check rc 0.
|
#1273 ready for review at 94f80f8 Scoped first PR for #806 (plan P7), CPU-only: 0 s GPU, 0 s NPU, no tickets — rule 1 respected, c31 had the box. Adds the
|
|
/review |
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
…rved as f16 (#1320) Follow-up to #806 (plan P7 part A) now that P4 has landed. #1273 put `kv {key, value, hadamard}` in the plan and promised, in docs/npu-plan.md, that "a target without the capability must refuse `int8` by name, never serve f16 in its place". That was prose. #808 (P4, A1) landed npu/target/target.h with `Caps::int8_kv`, and the xdna target reports it -- and nothing read it. So a plan asking for an int8 KV cache loaded clean and opened clean, and would have been answered out of an f16 cache: the same text from a different cache contract, which is the failure this layer exists to remove and the one that cannot be noticed from outside. Closing it before a writer exists, because afterwards the fallback would be born masking a real bug. - target.h: check_kv_caps(key, value, caps, err), the rule in one place, taking the plan's two dtype strings so the header stays free of nlohmann/json. A function rather than a per-target check because xdna is not the last target (P5 adds Hexagon) and a rule re-remembered per target is one that will be missing from a target. - target.h also gains <cstdint> for uint8_t in TileFn, which it needed all along and only got when an includer happened to pull it in first. Found by compiling a TU that includes this header alone. - xdna_target.cpp: open() calls it before the device checks, so the answer does not depend on whether a device is attached. - tests/npu_target_caps_test.cpp: device-free, and end to end from a plan file to the refusal. Pins both directions -- too little (int8 K, int8 V and int8/int8 each refused, message naming the plan fields and the missing capability) and too much (a plan with no kv block, i.e. every container shipped so far, is accepted with no error text; refusing a working model would be the worse regression) -- plus that a target reporting int8_kv = true accepts all three. - docs/npu-plan.md: the schema row says where the enforcement lives, the refusal list carries the target-side rule (today every target reports int8_kv = false, so every int8 plan is refused, because the writer does not exist yet), P7's remaining list drops this item, and P4's stale "in PR #808 (open)" line is corrected to A1 landed / A2 remaining. Checks: ctest -R 'npu_target_caps|npu_kv_int8|npu_plan' 5/5 pass through CMake in a fresh tree (including npu_plan_generate), the ci.yml add_dependencies binding verified in the generated build graph, tools/copyright.py --check rc 0. Stated limitation: xdna_target.cpp needs XRT headers and this host has none, so that TU is not compiled here. The line added there is fully qualified, the function it calls is proven by the new test, and the exact call-site namespace context was probed by a throwaway TU -- but I cannot rule a compile error out locally, so it is flagged rather than assumed. No device: 0 s GPU, 0 s NPU, no ticket taken. Co-authored-by: agent <agent@local>
P7a groundwork: the KV cache contract in the plan, and the arithmetic it rests on measured in CI
Scoped first PR for #806 (plan P7:
kv.dtype int8+ per-head Hadamard, chunked NPU prefill) — per the plan-layer rule in #799, a roadmap issue gets a scoped first step plus a comment on the issue listing what remains. This does not close #806.Rule 1 of tonight's brief says c31 gates come first, and this PR is deliberately CPU-only: no GPU ticket, no NPU ticket, no device time at all.
Why this is the right first step
#806's own last comment names the gap: the design audit "lives in a private brief (
~/briefs/DESIGN-P7-…md), not in this repository", and the plan layer can't describe an int8 KV cache becausedocs/npu-plan.mdsays in its own words "the plan has nokv.dtypefield yet". P7's remaining work needs P4 (NpuModel/NpuTarget, PR #808) and the privatenpu-kernelsrepo, neither of which this lane can land tonight. What it does not need any of that for is the contract and the arithmetic — that is, the part that can be wrong silently.What lands
npu/plan/plan.h—kv {key, value, hadamard {block, signs}}, defaulting tof16/no rotation so every plan written before the field keeps loading and means exactly what it meant.int8means one fp32 scale per (head, token) taken after a per-head Hadamard rotation. Validated with named refusals in the loader's existing style: an unknown dtype, a rotation block that is not0or a power of two in[2, 65536], asignsentry that is a path rather than a bare name,signswith no rotation block, and a non-string dtype (so a type error is a refusal, not a JSON exception escaping the loader).kv_int8()answers the target-capability question P4 will ask.tools/npu_plan.py— the generator states the contract it means (f16, no rotation) instead of leaving it to a key's absence. Nothing enables int8: that is a per-target decision whose rotation and scale sidecar live next to the kernels.tests/npu_plan_test.cpp— a case per rule above, including the two that matter for safety:../elsewhere.signsis refused (a plan file must not point the loader outside the directories the command line named), and a non-string dtype is refused by name.tests/npu_kv_int8_test.cpp— the measured proof, below. Registered asctest -R npu_kv_int8, header-only, no model and no device.docs/npu-plan.md— schema rows, refusal rules, a measured section, and P7's "Not yet" entry rewritten into what landed and what is left.Measured proof
Both claims P7a rests on are arithmetic rather than device behaviour, so they are gated in CI. Head width 256 (what both models this phase targets carry), one scale per (head, token), 512 keys, 8 draws per cell; KLD of the attention weights against the f16 cache the plan calls the reference — the same statistic #806's gate names:
(QH)(KH)ᵀ = QKᵀholds to 1.6e-09 relative, which is 0.01x one f32 dot product's own rounding error — a rotated f16 cache is not approximately today's cache, it is indistinguishable from it. If that were only approximately true, P7a would be a different model that answers the same text most of the time, which is the worst failure shape available.What this does not establish: the gate itself. The vectors are synthetic, generated to the outlier structure
docs/npu-plan.mddescribes; #806's KLD is taken on real KV from a real model on the device, and that run is what remains.Thresholds in the test are set from these columns, not chosen: the rotation's benefit must grow with outlier severity (54.5x > 9.6x > 1.0x), it must claim nothing on uniform data (<1.5x, measured 1.0x), and at 30x outliers the unrotated cache must be outside the gate while the rotated one is inside.
Checks
ctest -R 'npu_kv_int8|npu_plan'— both green (plus the whole-tree target build via CMake, so the registration and the CIadd_dependenciesbinding are exercised end to end, not just the standalone compile).tests/npu_plan_generate_test.py— green, so the generator change keeps every plan it writes valid.tools/copyright.py --check— rc 0; the new test carries the full Apache notice.~/GPU-TOKENand the NPU keeper were never touched, and no ticket was taken.What remains for #806 (also posted on the issue)
kvis a description until something reads it. In the order it can be taken: the cache writer/reader in the forward (int8 K store + scale sidecar, with the prefix-cache snapshot's byte accounting moving with it), the KLD gate on real KV on the device, then part B — prefill in 1024/2048-token chunks with block-fp expansion — which waits on P3's on-core expansion, P4'sNpuModel, and theM>16decision that part B needs and the decode path must not get.