Repository navigation
Merge main into #1976: keep both new test functions in npu_pack_import_test.cpp (Refs #1889) - #2130
Closed
bong-water-water-bong wants to merge 30 commits into
Closed
bong-water-water-bong wants to merge 30 commits into
bong-water-water-bong wants to merge 30 commits into
Conversation
… merge is reported, not failed daily (Refs #1945) (#2110) config/llama-mirror-hold.config: while it names the mirror's commit, the sync reports the hold, moves nothing and succeeds. The run fails, naming the file, when the hold is stale (the mirror moved) or unneeded (AMD's pin contains the held commit again). The shipped file names no commit: it holds nothing until the owner adds the line. Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg> Co-authored-by: Claude Fable 5.2 <noreply@anthropic.com>
…ks only on a dead runner (Refs #1504) (#2109) ci-runner-health.sh exits 3 (not 1) when every runner unit is accepting jobs but a restart policy would leave one down after the next OOM kill, and prints those lines as WARN. weekly.sh's preflight blocks on exit 1 and warns on exit 3 (owner decision on #1504, 2026-11-11). A missing ceiling alone no longer reads 'stops for good'. docs/ci-runners.md carries the two drop-in files and the three owner commands (scripts/ci-runner-restart.conf), and says that exit 143 in a build step is the memory guard. tests/weekly_runner_preflight_test.sh runs the real health script and the real preflight function against a stub systemctl. Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg> Co-authored-by: Claude Fable 5.2 <noreply@anthropic.com>
… so (refs #1946) (#2095) * ci: a skipped test is a test that did not run, and now something says so (refs #1946) Five of the defects this session kept finding are one defect: **green because nothing ran.** * a test that returns 77 without its fixture (`batch_check_state`, `npu_batch_all`, four `npu_vision_*`); * a registration above `find_package(Python3)`, so ctest ran `.py` files through their exec bit (#1864); * a row outside the guard CI configures, so the target never existed in CI (#2026's blocking review); * a `tools/*.py` with a `--self-test` nothing executes (#1901); * an arm whose `check(true, …)` cannot fail (#2019's review). `ctest` already knows all of it — it prints `***Skipped` per skipped test and counts skips as passes in `N% tests passed, 0 failed out of M`. Nothing read it. `tools/ci/ctest_skips.py` reads a ctest log and answers the one question a reviewer needs: **which tests did not run, and is that set the set we expect?** * `***Skipped` (the test ran and returned 77) → against `tools/ci/ctest_skips.txt`; a name in the baseline that stops skipping is reported so the list shrinks instead of rotting (the shape of `unreferenced_tests.txt`, #1824); * **`(Not Run)` — the binary is missing — is never baselineable.** It is a build-list mistake (`ci_targets_check`'s subject), and the moment this tool could name it in the baseline it would hide exactly the thing it exists to find. `--write` prints those separately and refuses to put them in the file. ## Measured on the CI-shaped tree (`ONEBIT_NPU=OFF`, 192 tests) ``` $ ctest --output-on-failure --timeout 300 > log $ tools/ci/ctest_skips.py --log log 7 tests ***Skipped -> baselined, one line each, with the reason 53 tests （Not Run) -> reported as missing binaries, not baselined 56 failed -> the summary line, unchanged ``` The 53 are this developer tree's unbuilt targets, not CI's — in CI the Build step lists what it builds and `ci_targets_check` keeps that list honest. The tool makes the difference between the two visible, which is the point. ## Self-test Eight arms on synthetic logs: no skips, both skip shapes, a baselined skip accepted, an unbaselined skip refused, a baseline entry that stopped skipping reported, a missing binary refused with no way to baseline it, the two shapes yielding different verdicts, and a log without a summary being "cannot tell" (rc 2). `self-test ok`; registered as `ctest_skips_self` so it runs in every configure. ## What this PR does not do It does not wire itself into `ci.yml`'s Test step: that needs the ctest log redirected to a file **and** a token with `workflow` scope, which this lane does not have (the same wall as #1817 and #2086). The wiring is one line — `ctest … | tee build/ctest.log` then `tools/ci/ctest_skips.py --log build/ctest.log` — and it belongs to whoever can push a workflow change. Refs #1946, #1864, #2026, #1901, #2019, #1824. * ci: the skip baseline carries its notice, and the copyright gate now knows the name (CI found this on #2095) CI was right and the check did its job: ``` build Copyright notice on every file ##[error]Process completed with exit code 1 no comment style known (add one to LINE_STYLES/BLOCK_STYLES or EXEMPT): tools/ci/ctest_skips.txt ``` `.txt` has no comment style of its own, so `tools/copyright.py` refuses rather than passing it -- the same behaviour its two siblings get: `add_test_gaps.txt` (#1595) and `unreferenced_tests.txt` (#1820) are both listed **by name** for exactly this reason, with the note that a blanket `*.txt` would demand a notice in files that are data rather than code. So this does what those two do, and nothing more: * `ctest_skips.txt` joins the by-name list in `tools/copyright.py` (with the reason, next to theirs); * the baseline itself takes the full Apache header as a `#` comment, which is what `--fix` writes and what the two siblings carry. Verified: `tools/copyright.py --check` rc 0; `ctest_skips.py --self-test` ok; `ctest -R ctest_skips` 1/1 passed. Refs #2095 (this is the fix for its failing build), #1946, #1595, #1820. --------- Co-authored-by: t <t@e.com>
…er accuracy has an after (engine#1313 item 3) (#2112) Co-authored-by: t <t@e.com>
…orts cleanly and answers nonsense (engine#1503) (#2101) * npu-pack: a tied head must correlate AND not be mis-scaled, or it imports cleanly and answers nonsense (engine#1503) Two gemma-2-2b hybrid packs measured on this box import with every check green and serve garbage: pack corr rel_err serves Qwen-2.5_1.5B_Instruct_rai_1.9.0_npu 0.996901 0.000000 'The capital of France is Paris.' x3 gemma-2-2b-onnx-ryzenai-hybrid 0.991539 0.136140 'myself myself ...' x3 gemma-2-2b-awq-...-g32-fp16-onnx-hybrid 0.996053 0.094874 'myselfGetBytes...' x3 A head that is a SCALED copy of its own embedding still correlates, so the correlation-only check passes it -- and the import printed the number that distinguishes them (rel_err) as an advisory field next to the correlation it gated on. Both packs cost a device ticket to discover. - one predicate, selfcheck_head_ok(corr, rel), so the four sites that set selfcheck_tied cannot drift; - kSelfcheckRelMax = 0.02, the measured gap: 0.000000 on the pack that serves, 0.095/0.136 on the two that do not; - Report::selfcheck_scaled, set at all four sites next to selfcheck_tied; - main() refuses on it and NAMES both numbers and the threshold. --allow-incomplete does not override: that flag means 'write the tensors I could not represent', and this is 'the tensors I wrote do not decode'. Verified: both translation units pass g++ -fsyntax-only (the json header from a neighbour build), and the predicate's six arms pass against the three measured triples plus a control, a no-correlation case and the inclusive boundary (~ /wt/relcheck.cpp, run). NOT verified here: the full npu_pack_import_test binary and any container import -- that needs a configured build, which the disk floor did not allow at the time; the two test arms are written for CI. * npu-pack: name the partial output in the mis-scaled-head refusal (engine#1503) Demonstrated on the real pack, not argued from the fixture: importing ~/scratch-packs/gemma22b (gemma-2-2b-onnx-ryzenai-hybrid, the pack that served 'myself myself ...' for 16 tokens) now stops at refusing to import ...: the head correlates with the pack's own embedding (0.991539) but its relative error is 0.136140, which is a scaled or partially decoded head rather than a rounding difference (the threshold is 0.020). A pack in this state imports cleanly and answers garbage on the device; --allow-incomplete does not override this. That run also showed the check happens after import_pack() has already copied the text artifacts and written model.q4nx, so a refusal left five files behind. An importer must not delete a directory the caller named, so the message now says the partial output exists and is not servable -- the operator deletes it before retrying. Verified by rebuilding onebit and re-running the same import, and the test target still passes its six arms. --------- Co-authored-by: pi-mosaic <pi-mosaic@1bit.local>
…ames the decision route (Refs #1786) (#2121) A pack with non-causal attention has no decode to run. /v1/chat/completions and /v1/completions tried anyway and failed inside the forward with HTTP 500 and its internal wording (measured on FastFlowLM's D3-Nano-NPU2). Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg> Co-authored-by: Claude Fable 5.2 <noreply@anthropic.com>
… the fast route at speed (refs #2106, #2102) (#2119) * tools: npu_serve_smoke, the release gate's check that npu serve is on the fast route at speed (refs #2106, #2102) A serve that falls back to the per-op forward still answers, so a check that asks 'did it answer' passes on the wrong route. This starts 1bit serve --device npu, sends three prompts with thinking off, and asserts the route's own log line, a decode floor (60 tok/s), the first token (600 ms) and non-empty, non-degenerate content. Measured, Qwen3-0.6B, one npu-hold ticket each: the 2026-11-07 release candidate PASSes (median 84.4 tok/s, 277 ms); a main build without the fast route FAILs route, rate (1.9 tok/s) and first-token while its answers are fine. --self-test drives the judging on 15 shapes (the silent fallback, thinking left on, a degenerate reply, the median, missing timings); registered in ctest as npu_serve_smoke_self. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> * tools: npu_serve_smoke's self-test pins the median (a mean mutant passed all 15 cases) Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> * tools: npu_serve_smoke takes the two route texts as options, so it can follow the serve's wording Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> --------- Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg> Co-authored-by: Claude Fable 5.2 <noreply@anthropic.com>
…t (Refs #1786) (#2118) 'Answer with one word' came back as "\n\nParis", and a model that thinks nothing sent a reasoning chunk of two newlines. The whitespace leading the reasoning, and the whitespace leading content that follows a block, are dropped: the rule app/think_split.h applies on the in-process route. Two existing arms pinned the newline after the opening tag as part of the reasoning; they are updated. Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg> Co-authored-by: Claude Fable 5.2 <noreply@anthropic.com>
… takes a ticket (#2108) engine#803's P4 gate is "window11 text identity plus **logits KLD 0** on 8 prompts". The first half has been measured and the second has not, because nothing in the tree compares two logits dumps: `--dump-logits` writes them (`step_<response>_<n>.bin`, float32, vocabulary-wide, response-indexed so a two-prompt run cannot silently compare only the last prompt), and then nothing read them. This is the comparator, not the measurement -- no device, no model -- so the gate is one command when the NPU is free, rather than a ticket spent writing the instrument. Thresholds are the HRX gate's own shape (`tools/hw_gate_manifest.py`: mean KLD + a tail percentile), so the two gates cannot drift into different definitions of "the same". tools/logits_kld.py BASELINE_DIR CANDIDATE_DIR [--max-mean-kld 0.01] [--max-kld-p99 0.2] tools/logits_kld.py --self-test Six self-test arms, each one a case this arithmetic gets wrong: - identical dumps give **exactly 0.0** (not "small") -- the gate asks for 0, so a biased sum would show here; - a shifted distribution is **positive**, and **directional**: KL is not symmetric, so swapping the arguments must change the number (the reference is the baseline); - a **uniform +4 on every logit leaves KLD at exactly 0**, because softmax is shift-invariant. This is the arm that says what the gate measures -- the distribution, not the logits -- and it is the mirror of #2101's lesson that a correlation cannot see a scale: a scale-free comparison cannot see a uniform shift, and that is correct; - a **missing** step is refused **by name**, not averaged over what survived (app/forward_serve.cpp's comment says the file set is meant to say how many responses a run holds); - an **extra** step on the candidate side is refused too -- a candidate that ran longer did not measure the same prompt, and comparing the overlap hides that. Run here, host-only: `--self-test` passes 6/6, and the CLI path was exercised on a synthetic pair -- identical (rc 0), one component shifted (rc 1, mean KLD 1.277), uniform shift (rc 0), missing directory (rc 2). **Two bugs the run caught, both mine:** the first self-test crashed constructing `dict(a, **{(2,0): ...})` (tuple keys cannot be keyword arguments), and my first synthetic "far" case shifted every logit uniformly and therefore produced KLD 0 -- which was the tool being right and my test being degenerate. That case is now an arm. Not claimed: any number for the gate itself. This reads dumps; it has never seen one from a device. Co-authored-by: t <t@e.com>
…fill.sealed_elf, decode.narrow_elf) (Refs #1990, #2073, #799) (#2080) Plan::load reads prefill.sealed_elf / prefill.sealed_rows (17..128) and decode.narrow_elf / decode.narrow_rows (1..15), all optional, and refuses the inconsistent forms (no sealed decode set, a path instead of a bare name, rows missing or out of range, rows without a file). A loaded plan wins over config.json; NPU_WIDE_SEALED_ELF and NPU_NARROW_SEALED_ELF still override both. tools/npu_plan.py copies the config keys into the plan and writes nothing new for a pack without them. Re-applied on main after #2042 and #2074 merged; the change is the one reviewed at 4315c23 / 2c3ad10. Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg> Co-authored-by: Claude Fable 5.2 <noreply@anthropic.com>
…ize it was measured on (Refs #1785) (#2075) FastFlowLM's Qwen3.5-2B (IM 6144, H 2048, 144 MiB tail) sat under the 256 MB floor, so every decode token re-dequantised the dense FFN tail. A B B A in one npu-hold ticket: 362-367 ms per token with the cache on, 485-520 ms with it off, identical text in 12 of 12 runs. The size rule moves to npu/forward/include/ffn_cache_policy.h with a test. Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg> Co-authored-by: Claude Fable 5.2 <noreply@anthropic.com>
…t_test.cpp The one conflict is two functions added at the same place: this branch's test_attn_first_tiles() (engine#1889) and main's selfcheck_head_rule_arms() (#2101). The resolution keeps both, and both are called from main(). Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PR Reviewer Guide 🔍(Review updated until commit 34cac62)Here are some key observations to aid the review process:
|
…mp-encoder dumps, with before/after on the game (Refs #1313) (#2117) Item 3 of engine #1313, the only engine item left (audit on the issue): a head-only fine-tune of Laya's routing head (type_emb, two head layers, scorer; encoder frozen) on the CPU, over the dumps `1bit route --dump-encoder` already writes. The output is an ordinary --laya-model dir: only the head tensors' bytes change, the temperature is refit on dev (kept when the refit hits the grid's edge), finetune.json records the run. metrics.py reads --per-case files with laya/eval.cpp's definitions. Measured on the game's logged decisions, episodes held out whole (base / fine-tuned): test 33/33 of 39, log loss 0.543/1.395; test2 107/106 of 110, 0.478/0.294; test3 55/55 of 56, 0.475/0.188. A failed-reflex outcome label taught nothing (0/10 held-out rows moved) and is documented as such. tests/laya_finetune_test.py (ctest laya_finetune, skip 77 without torch): synthetic checkpoint + dump; dev log loss 0.676 -> 0.002, accuracy 0.688 -> 1.000; every non-head byte identical; refuses an existing --out; metrics.py against a hand-computed per-case file. 14/14. Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
bong-water-water-bong
left a comment
bong-water-water-bong
left a comment
Collaborator
Author
There was a problem hiding this comment.
VERDICT at 38c28f3: READY
Reviewed head 38c28f3 (refs #1889). This is a merge of main onto #1976's branch cl-npupack-a/1889-forward-use-qk-norm.
- Parents are what the body says. First parent
a43aea3is #1976's current head; second parentc9b38d4is onmain. - The resolution adds and loses nothing. The merged tree's diff against its main parent (sorted
+/-lines) is identical to #1976's own diff against its merge base, 340 lines each. So beyondmain, this commit carries exactly #1976's change.git merge-treeof this head onto currentmain: no conflicts. No conflict markers intests/npu_pack_import_test.cpp. - Both functions are kept, and both run.
selfcheck_head_rule_arms()(:5705, main's #2101) andtest_attn_first_tiles()(:5689, #1889) are both called frommain()(:5716), at:5717and:5729.test_attn_first_tilesprints only on failure, so I added a temporary print at its entry. It printed once, and I removed it (git status clean). - The test passes.
npu_pack_import_test, built from this head: rc 0, "head rule: 6 arms …", "all npu-pack checks passed -- pack leg SKIPPED, no pack directory given". - CI:
buildandhrx_buildsucceed at this head.
Merging this into #1976's branch should make #1976 mergeable. #1976 then needs its own verdict at that new head.
#2102) (#2113) * npu serve: say it at ready when no 16-row decode kernel was found (engine#2102) * npu serve: the #2102 ready line stays silent off the missing-kernel case, and names the fused route Review fixes on #2113, each reproduced at 692ae19 and re-run here: - the host-GEMM arm loads no decode family; design_roles() is 0 there, so it no longer reads "0 of 4" - NPU_DECODE_M16=0 gets its own sentence naming the switch instead of "missing" kernels - a ternary (pb16i/t2s) container with some roles on M=128 is its geometry's (a missing kernel already fails the load), so only 0 of N warns there - the way out names the fused route: all four _m16 twins measured ~2.7 tok/s vs ~1.7 without and 88-101 fused - tests/npu_decode_note_test.cpp carries the full Apache notice (the build's copyright step was red) - docs/npu.md says what the line is and when it is silent Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: bong-water-water-bong <pi@1bit.local> Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Collaborator
Author
|
ready for review at 38c28f3. All CI is green at this head, including the build and ctest with |
bong-water-water-bong
left a comment
bong-water-water-bong
left a comment
Collaborator
Author
There was a problem hiding this comment.
VERDICT: READY at 38c28f3
Checked the merge the way a merge has to be checked — by set-diffing the whole registration list, in both directions — and nothing is lost. Then ran it.
The registration sweep (the instrument that caught #2126 twenty minutes ago):
add_python_test names: this PR 28, main 28, lost none, added none
add_test(NAME …) names: this PR 213, main 212, lost none, added q4nx_qk_norm <- this PR's own new row
Both functions exist and both are called — the claim in the body, verified:
5689: void test_attn_first_tiles() { <- this branch's, engine#1889
5705: static void selfcheck_head_rule_arms() <- main's, #2101
5717: selfcheck_head_rule_arms();
5729: test_attn_first_tiles();
And I built and ran it: npu_pack_import_test compiles clean and prints head rule: 6 arms (…) then all npu-pack checks passed — so main's arm is present and this
branch's is reached on the same run, which is exactly what "keeps both" has to mean. (The pack leg skips here — no pack directory — so my run covers the crafted/golden legs,
the same scope as the author's.)
One thing I checked that the body does not mention: test_attn_first_tiles() is not an empty shell kept alive to satisfy a diff. It asserts five distinct
properties, and they are the kind that fail on the defect rather than around it:
check(kqv.k == 0 && kqv.q == 16 && kqv.v == 80, "attn order: k,q,v puts k at tile 0, q at 16, v at 80");
check(qkv.q == 0 && qkv.k == 64 && qkv.v == 80, "attn order: q,k,v puts q at tile 0, k at 64, v at 80 (engine#1889)");
check(qkv.k + 16 == qkv.v && kqv.q + 64 == kqv.v, "attn order: v follows the second run in both orders");
check(small.k == 0 && small.q == 2 && small.v == 16, "attn order: the Qwen2.5-0.5B run (2/14/2) is unchanged");
check(!p.attn_q_first, "attn order: a plan is k,q,v unless the block measured otherwise");
The third is the interesting one — it holds for both orders, so it cannot pass because one order happened to be right; and the last is the default-direction arm, which is
the one that catches a plan claiming q_first without evidence.
Both conflicts were independent insertions at the same line, which is the easy case — but "easy" is the case where a resolution silently drops one side, and that is
precisely what the set-diff rules out.
Standing note for the parent PR: #1976 keeps its verdict at its own head; this line is about the merge only. Base is cl-npupack-a/1889-forward-use-qk-norm, so #1976 lands
first.
…he slow route (refs #2106) (#2125) * hw-gate: an npuserve phase, so the gate can fail a serve that is on the slow route (refs #2106) scripts/hw-gate.sh --phases npuserve runs tools/npu_serve_smoke.py on HW_GATE_NPU_SERVE_BIN / HW_GATE_NPU_SERVE_MODEL under the same NPU claim as the npu phase and reports the four assertions in the summary row. PASS needs the tool's exit status and its RESULT line. Not in the default phases; skips with no NPU or no settings. The tool's self-test now also drives its start / ask / stop path against a stand-in serve (fast, slow, dead), and tests/hw_gate_npuserve_test.sh (ctest hw_gate_npuserve) runs the phase end to end against the same stand-in: 13 checks, no device, the fleet's queues untouched. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> * hw-gate npuserve tests: hold the loopback-port lock, and write the stand-in owner-only (CI at 414115e) CI's build job failed on route_port_lock: hw_gate_npuserve starts a loopback server without RESOURCE_LOCK route_ports. Both it and npu_serve_smoke_self (which now starts one too) take the lock. CodeQL flagged the stand-in serve's 0755 as world readable; it is written 0700. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> * hw-gate npuserve: the serve cannot outlive a TERMed run; the gate passes its own floor, length and reply count (review of #2125) CHANGES at fe89793: when the ticket's hold ends, the TERM reaches the smoke script (a flock sits between them) and Python's default action skipped the finally that stops the serve, leaving 1bit serve running with no ticket. The script now turns TERM/HUP/INT into SystemExit (the serve is stopped, exit 128+signal) and starts the serve with a parent-death signal. Pinned twice: the tool's self-test TERMs a real run mid-request (without the fix: exit -15 and the serve left running), and hw_gate_npuserve_test.sh sends gpu-hold's kill sequence to the phase's flock. READY-with-asks at fe89793: the phase now passes --floor, --max-tokens and --replies itself (30 tok/s, 96, 5), so the rate assertion does not depend on a default in another file. The floor is 30 because single fast-route replies have been measured from 48.8 to 101 tok/s on this shared box and the slow route is 1.9. Docs say the phase is blocking. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> --------- Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg> Co-authored-by: Claude Fable 5.2 <noreply@anthropic.com>
…st no longer fails the graph (fixes #1925) (#2134) One fork commit past 7a1f2b81c: - 1703bfeaa (#302): the scheduler's dependency walk no longer counts an in-place CPY's own output as a pending input (a ggml_cast stalled the walk and everything after it was taken in index order, where the L2_NORM root of the fused gated delta net comes before the projections it reads); the fused matcher keeps a match whose layout alias was already passed. Measured on gfx1151 with the two changed objects replaced in an add-on build: Flash-Next UD-IQ3_XXS 18,782-token request, unsupported HRX node 1 -> 0, 390 of 390 fused roots accepted, the whole prefill runs. Qwen3.5-4B old vs new library: 128 of 128 tokens identical in three pairs, the same 33 dispatch registrations, decode 57.4/58.1 vs 58.3/58.2 tok/s. Host: test-hrx-gdn-passed-alias 7 cases, hrx-backend-test 66/66. Not shown: the 18.8k answer with the mask chain on the device against the CPU; those requests end in #1536's all-NaN logits on the old and the new library alike. registry/architectures.json: the recorded source pin only. docs/hrx.md: the #1925 section. Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg> Co-authored-by: Claude Fable 5.2 <noreply@anthropic.com>
…CPM-V 4.7 pack with the engine's own steps (engine#1788) (#2132) * npu: minicpm_image_probe, one picture and one question through a MiniCPM-V 4.7 pack with every step the engine's own (engine#1788) RGB bytes -> plan_image -> preprocess -> tower_forward (host) -> image_tokens + prompt_positions -> the text model's step_batch_embeds() with the tower's rows where the placeholders are, then a greedy decode. No image decoder (PPM or a generated colour), no tokenizer or template outside the engine. Measured on MiniCPM-V-4.7-1B-NPU2 with the d3-lite kernel set, text model on the NPU (one npu-hold ticket, 99 s): no image -> 'black and white' solid blue -> 'blue' left red, right blue -> '... split vertically down the middle. The left side is red, and the right side is blue.' Host arm, solid red -> 'red'; the prompt's 93 token ids are identical to the reference processor's. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> * docs(npu): the image probe needs the padded-head rule (#2026) to load the published packs Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> --------- Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg> Co-authored-by: Claude Fable 5.2 <noreply@anthropic.com>
…2111) (#2131) An empty content with finish_reason "length" read like "the model said nothing" when the whole budget went into a thinking block (#2111; it produced #1998 as a wrong bug report). The text was already returned, as reasoning_content, on both routes: what was missing is the count. Both NPU chat routes now report OpenAI's usage.completion_tokens_details.reasoning_tokens, additively: - forward-serve: each generated token's text is kept (not only with logprobs), and the count runs the same AnswerStream that splits the reply (npu::reasoning_token_count, beside content_mask), non-streamed and in the streamed usage chunk. /v1/completions carries no details. - unified route: the generated ids (end token excluded, as completion_tokens excludes it) through this route's ThinkSplit (think_reasoning_tokens); 0 when nothing is split. A token is reasoning when none of its text reached content and the block was open before it, after it, or it wrote into it; leading tokens that produced nothing join the block if the next token opens it. So both tags, a split tag, and an unclosed block at the end of the budget count, while tool-call markup and the layout after a block do not. Tests: npu_answer_parser_test (+10 arms, incl. the issue's 8-of-8 row and a block opened after content) and think_split_test (+6). Mutants of the rule (no "before", no "after", content counted, no pending) each fail an arm. docs/serve.md: what the field means and what to do about it. Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…shrug (engine#2077) (#2091) `pr_agent` reads red on PRs because it is CANCELLED, not because it failed: the job carries `concurrency: { group: pr-agent-<pr>, cancel-in-progress: true }`, right for a fast check and wrong for a minutes-long 30B review on one self-hosted runner. Nothing in the tree counted that third state. Measured while writing this (`gh run list --workflow=pr-agent.yml --limit 40`): --event=pull_request cancelled=22 success=17 running=1 -> 55 % cancelled (all events) cancelled=12 success=9 skipped=9 -> 30 % cancelled, the denominator lesson The two differ because `skipped` runs from non-PR events dilute the share, which is why --event exists. `--runs` reads conclusions through `gh run list` (read-only) and reuses this script's contract: verdict line first, counts second, rc 0 PASS / 1 FAIL / 2 nothing readable. The summariser is a pure function over conclusions on stdin, so its arms need no GitHub - including "no runs read is not a pass" and "one cancelled run is already over 0 %", i.e. the threshold's floor is reachable. --max-cancelled-pct defaults to 25. Rebased twice onto a moving main; both conflicts kept BOTH sides: main's six policy_verdict() arms plus my six run arms, one count (22 - counted from the printed arms, not from memory: 20 chk/chkP assertions + 2 case arms). The fix for the cancellation itself is one word in .github/workflows/pr-agent.yml, which no lane token can push; this makes the size of the problem measurable and testable from a lane, and names the cause.
…en only the timing failed (Refs #2106) (#2140) 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: bong-water-water-bong <bong-water-water-bong@1bit.gg> Co-authored-by: Claude Fable 5.2 <noreply@anthropic.com>
…5) (#2139) Orchestrator decision on #1945 (technical calls delegated by the owner): HOLD. config/llama-mirror-hold.config names the mirror's commit, so the daily run reports the hold, moves nothing and succeeds; the merge of AMD's rebased line is post-release work. A held run also leaves the engine's llama.cpp pin where it is. Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg> Co-authored-by: Claude Fable 5.2 <noreply@anthropic.com>
…drift while it is being finished (#2114) * ci: the engine#1962 narrowing enumeration is a ratchet, so it cannot drift while it is being finished engine#1962 is a release-blocker (a dense MUL_MAT returns NaN or wrong values when an activation exceeds the f16 range). Its repairs land on the *producers*: a plan records a 2^-6 prescale, the consumer scales back. The issue's remaining question is "which of the narrowing consumers are protected", and the enumeration published on the thread answers it the only way available today - by listing the 25 .loom bodies that narrow f32 to f16 and finding that **none of them carries the guard** (the 5 files that do are producers, and are not in the 25). That answer is worth exactly as long as the list cannot change underneath the person acting on it. tools/ci/hrx_f16_narrowing.txt: the 25 bodies, as a baseline at pin 7a1f2b81c, with the reason the list is a ratchet rather than a bug list written at the top of the file (a body narrowing a bounded scale or a weight is perfectly correct). tools/ci/hrx_f16_narrowing.py: --check fails on a body that appears without a line *and* on a line whose body is gone (a list that can only grow cannot see a rename, engine#1728); --write rewrites the baseline; --self-test covers both arms on fixtures, plus "a '#' line in the baseline is not a path" and "a missing corpus returns 77, not 0". --check skips 77 without the submodule - the corpus is third_party/llama.cpp - so the CI row tests the checker and the arm that needs the corpus runs where it exists. Registered as ctest tool_selftest_hrx_f16_narrowing (runs everywhere) and hrx_f16_narrowing_check (skip 77 without the corpus). Verified on the real corpus: `--check` -> "25 narrowing body(ies), none new, none stale; 0 of them carry a guard marker (the guard sits on the producer, engine#1962)"; adding a probe .loom with one fptrunc -> `NEW narrowing body loom-libs/ops/_ab_probe.loom ... FAIL (1 new, 0 stale of 25)`; removing it -> clean. Refs #1962. * ci: the baseline carries the full notice, and copyright.py learns the file Answers the CHANGES at d38ce8d from two reviewers, both on the same point: the `build` job's "Copyright notice on every file" step fails on tools/ci/hrx_f16_narrowing.txt, because `.txt` has no comment style the checker knows. Both parts are needed and both are here: 1. the baseline carries the same full Apache header as tools/ci/add_test_gaps.txt (the other '#'-comment data files) instead of a two-line stub; 2. `hrx_f16_narrowing.txt` joins the '#' entry in LINE_STYLES next to `add_test_gaps.txt` / `unreferenced_*.txt`, which is the same kind of file: data whose header IS the notice it would otherwise be asked for. Verified: tools/copyright.py --check rc 0; the checker still reports "25 narrowing body(ies), none new, none stale" against the corpus at the engine pin (checked with the corpus symlinked in, then the link removed); --self-test 7 arms ok. Note on why I did not see this before pushing: I ran `copyright.py --check` while the file was still untracked, and the check does not see untracked files. That is the hole I reviewed earlier tonight in the other direction (copyright_check_untracked_test.py); this is the same hole from the inside. Refs #1962. --------- Co-authored-by: bong-water-water-bong <bong@water>
…ugh a symlink (CodeQL world-writable alerts; unblocks #1830) (#2057) * npu: dumps are created 0600 and the head-image cache 0644, never through a symlink (CodeQL world-writable alerts) Six open CodeQL cpp/world-writable-file-creation alerts on main, all fopen(..., "wb"), which creates 0666 & ~umask and follows a symlink planted at the name: npu/forward/src/q4nx_forward.cpp NPU_I8_GS_DUMP host-arm dump npu/target/xdna/xdna_target.cpp NPU_I8_GS_DUMP device dump npu/forward/src/q4nx_head_image.c head-image cache temp file npu/forward/include/residual-dump.h NPU_INFER_DUMP_DIR layer dump tests/npu_pack_import_test.cpp, tests/npu_residual_dump_test.cpp A PR that merely touches nearby code inherits these as "new alerts in code changed by this pull request": #1830 has sat on a CHANGES verdict for one since 14:35. npu/forward/include/file_create.h (plain C, shared by the .c and .cpp sources): onebit_fopen_private_wb (0600) and onebit_fopen_shared_wb (0644), open(O_CREAT|O_TRUNC|O_NOFOLLOW|O_CLOEXEC) with a constant mode, then fdopen, with errno kept from the failing call so the residual dump's strerror message is unchanged. The test's /dev/full probe opens for reading. Measured with umask 0: 600 and 644; a symlink at the path is refused (ELOOP). ctest npu_residual_dump, ops_dump, npu_design_grid (the head-image cache arms) and npu_pack_import pass; onebit_npu_forward builds in the ONEBIT_NPU configure. The path-injection alerts at the same sites are left alone: those paths come from env variables by design, so they are the owner's call. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * file_create: narrow an existing file's mode, pin it in a test, and refuse new fopen-for-write sites (review on #2057) Review asks at 2ecd5fd: - O_CREAT's mode applies only on creation, so a dump an older build left at 0666 stayed 0666. The opener now clears every bit beyond the stated mode with fchmod on a regular file. It only narrows: a 077 umask still leaves the cache 0600, and a device (/dev/null, /dev/full) keeps its own mode. - tests/file_create_test.c (plain C, umask 0): new 0600/0644; an existing 0666 narrowed and truncated; 077 not widened; a symlink refused with ELOOP and its target untouched; ENOENT kept for strerror; /dev/null not chmod-ed; the append opener. Against a header that reverts to fopen it fails 9 of 14. - tools/ci/file_create_check.py (--check / --self-test, both ctest rows): refuses fopen(..., "w"/"a") in tracked C/C++ unless the line says `file-create-ok: <reason>`. Its first run found three sites my grep had missed (the call's c_str() parens): forward_serve's logits dump and ort_run's output dumps, now through the openers (with a new onebit_fopen_private_ab for outputs.txt). The /proc write in onebit_ep_qdq_proof.cpp and the Windows fallbacks carry the marker. - The header names who reads the 0644 file (the head-image cache, read back by a later serve), so the shared opener is not reused for a dump. ctest file_create, file_create_check(_self), npu_residual_dump, ops_dump, npu_design_grid, npu_pack_import pass. In the ONEBIT_NPU configure, forward_serve.cpp compiles and 1bit links. ort_run.cpp passes -fsyntax-only against the box's ONNX Runtime headers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * tests/file_create_test: start the narrowing arms at 0664, not 0666 (CodeQL on #2057) The test chmod-ed its fixtures to 0666 and created the symlink target with fopen, and CodeQL flagged all four lines as world-writable file creation, which is fair: the test should not make the files it exists to prevent. 0664 (what fopen left under a 002 umask) is wider than both openers' modes, so the arms still check the narrowing; the symlink target goes through the private opener. ALL PASS; against a header reverted to fopen it still fails. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…(Refs #1786) (#2144) FastFlowLM's Phi4-mini-Instruct-NPU2 names <|endoftext|> in config.json and tokenizer_config.json, and its template closes every turn with <|end|>. The serve stopped only on the first, so the answer was 'Paris<|end|><|assistant|>...' to the token budget. The marker is now taken from a rendered two-message exchange (npu::turn_end_marker). Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg> Co-authored-by: Claude Fable 5.2 <noreply@anthropic.com>
…e#2129) (#2141) A build compiles whatever is in the working tree. Every check in tools/ci reads the tree it is handed, so none of them can see that a submodule is checked out on a different commit than the superproject pins - a build that compiles an un-pinned dependency passes every gate we have. Measured on 2026-11-11, which is why this exists. The shared ~/1bit-engine had FOUR submodules off their pins: third_party/llama.cpp c16424f5620d (pinned 7a1f2b81ce17) third_party/hrx-system 51b1739ae5fd third_party/tokenizers 88a4498ad4ea third_party/xdna-driver 5d302c9986a5 and the engine compiles common/jinja from llama.cpp (CMakeLists.txt:147). That checkout was BEHIND the pin on the files it compiles: behind f08bd76f6 "jinja: accept \xNN escapes in the lexer (#135)" plus a std::to_string portability fix. Jinja decides prompt rendering, so an engine built there renders a \xNN-bearing template differently - not a build failure, but a difference that reads as a model bug. `git submodule status` marks it in one character, and this is the check for it: ' ' at the pinned commit -> pass '+' a different commit (the #2129 shape) -> FAIL, naming both shas and the two repairs 'U' merge conflicts in the submodule -> FAIL '-' not initialised -> skipped by default, FAIL with --require-initialised The default compares only what is checked out, because a configure that never inits a submodule is not this check's business; --require-initialised is the release-gate form, where a missing checkout is a failure too. --check on this branch's worktree: 7 submodules, 0 at their pin because a fresh worktree inits none, rc 0 with the "./" default and rc 1 with --require-initialised. Against the shared ~/1bit-engine: rc 1, naming exactly the four above. Verified: --self-test 16 arms ok (each flag, the three failure shapes, "no submodules read is 'cannot tell', not a pass", and "one '+' among three lines still fails" so the check is not first-line-only); registered through cmake/tests-*.cmake so it needs no ci.yml line - ctest -N lists submodule_pin_check and _self, and both pass.
Collaborator
Author
|
VERDICT: NOT READY at Same class as A stale base is not itself the problem. # must print nothing
comm -23 \
<(git show origin/main:CMakeLists.txt | grep -oE 'add_python_test\([a-zA-Z0-9_]+' | sort -u) \
<(cat <(git show <head>:CMakeLists.txt) <(git show <head>:cmake/tests-python-tools.cmake) \
| grep -oE 'add_python_test\([a-zA-Z0-9_]+' | sort -u)
# on this head: laya_finetune
Why a stale base causes this and a plain feature branch does not: this PR's whole content is a resolution of the registration block. If the base is old, the resolution contains a version of that block that never saw the newer rows, and the merge carries the omission forward with a clean exit code. A feature branch that simply does not know about a row keeps it on merge; a resolution can delete it. What to do: re-merge against current main and set-compare again. The check is four lines and belongs in the PR body for any "Merge main into …" PR — it is the only evidence that the resolution did not silently drop a registration, and it costs less than a review pass. |
…roduct: 37.7 -> 3.2 s per 448x448 view, same bits (engine#1788) (#2142) tower_forward() ran every loop on one thread and walked each weight matrix once per token, so a 448x448 view (1,024 patches) of the 1B pack took 37.7 s. The row and head loops are now spread over threads (ONEBIT_VISION_THREADS, vision::tower_threading) and linear() applies each weight row to 16 tokens before moving on. Every token and head is still computed by one thread with the same sequence of operations, so the output is the same bits. Measured, box load ~13: 37.7 s before; 18.1 s on one thread (the blocked product); 3.2 s on 16 threads. Test: the fixture's goldens hold, and 1, 2, 5 and 16 threads (forced, no size threshold) give byte-identical rows. Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg> Co-authored-by: Claude Fable 5.2 <noreply@anthropic.com>
…ed rows (Refs #1786) (#2148) Eleven packs with a row from the dev box: six on the fused route with nothing edited, six on the per-op route (one pack on both), one decision pack; the same-pack pair against FLM's runtime; what each route needs; the open fault on the fused route; and what the rows do not establish. Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg> Co-authored-by: Claude Fable 5.2 <noreply@anthropic.com>
… ones (#2027) * tools/ci: a /tmp literal is RAM on this box, so the ratchet stops new ones `/tmp` is tmpfs on both boxes, so a debug dump, a staging copy or a benchmark log written there is anonymous memory: it cannot be reclaimed to disk under pressure, it goes to swap, and when swap fills every lane on the box stalls. Measured 2026-11-10 21:15Z: /tmp held 52 G of 62 G and swap was 71/71 GiB full; the two largest items were a stale 1bit-fconst19*.fconst scratch copy (3.5 G) and five q17-*.json dumps (9.2 G). engine#1907/#1952 bounded the fconst scratch. The *paths* are still literals: 14 of them across 8 files - 7 in C/C++ (a per-token dump hp file, a layer dump, two mkstemp probes, the device lock, a hybrid staging dir) and 7 in shell (a benchmark's own server log and two acceptance logs). The per-token one, q4nx_hidden_%d.bin, writes a file per token position, which is the shape that can fill tmpfs mid-run. This adds the check, not the fix: tools/ci/tmp_paths.txt baselines the 14 sites and may only shrink, and a new one fails. Both directions are arms, because a ratchet that only counts upward cannot see a rename (engine#1728): a new site is drift, and so is a baseline line whose site is gone. Verified: --check on this tree reports "14 baselined, none new, none stale"; --self-test's 9 arms pass, including "a commented-out path is not a site" (the engine#1983 substring trap), "/var/tmp is not /tmp", and "a renamed file leaves a stale baseline line". Registered through the cmake/tests-*.cmake glob (engine#1979), so it needs no CMakeLists.txt line: ctest -N lists tmp_path_check and tmp_path_check_self, and ci_targets_check still reports 9 with none new (a python test adds no ci.yml --target). The rewrite of the 14 sites wants the resolve order ONEBIT_DUMP_DIR -> $XDG_CACHE_HOME/1bit/scratch -> $HOME/.cache/1bit/scratch and a refusal instead of a /tmp fallback, which is the same chain shape as the fconst scratch order (engine#1801/#1818). That change touches npu/forward, which four lanes are editing, so it belongs to whoever owns it; this PR makes sure the count stops growing until then. * tools/copyright: list tmp_paths.txt by name, beside the two other #-comment baselines refused the new baseline: `.txt` has no comment style of its own, and a blanket "*.txt" would demand a notice in files that are data. The two existing ratchet baselines (add_test_gaps.txt, unreferenced_tests.txt) are handled exactly this way, so this lists the third beside them - and --fix then wrote the notice it wanted, which is at the top of the file. * tmp_path_check: match the BARE spelling too - the review measured a 14-site baseline that was a 45-site tree The reviewer's hole, reproduced before fixing: echo x > "/tmp/quoted.log" -> caught echo y > /tmp/bare.log -> INVISIBLE The pattern required a leading quote, so `> /tmp/x.log`, `cp a /tmp/x` - the plain shell idiom, and the most likely form in scripts/*.sh - never matched. A ratchet that cannot see the common spelling of the thing it guards is a ratchet over quoted literals only, which is not what the 21:15Z directive was about. The anchor is now "not the tail of another path" instead of "a quote": (?<![A-Za-z0-9_./-])/tmp/[^"'\s;|)]*["']? `/var/tmp/x` stays out (the `a` before `tmp` blocks it) and `/tmp/x` is caught after a space, `>`, `=`, a quote, or nothing. A trailing quote is stripped, so the baseline records the PATH: re-quoting a site is not a new site, and one path spelled two ways is not two lines. Re-measured after the fix, and the old baseline was an undercount by a factor of three: before 14 sites in 8 files after 45 sites in 18 files Ten files were entirely invisible: scripts/check-ubuntu-2604-lemond.sh (6 sites, including a downloaded `/tmp/engine.tar.zst` and its extracted tree), scripts/pkg-proof.sh (5), tools/npu-arena/*.sh (4), tools/showcase-612*.sh (2), tools/xhci_boot_capture.sh, tools/repro-hostgemm-lock.sh. Two new self-test arms assert it: "a bare /tmp path is a site (no quotes)" and "the bare form is recorded without a quote it never had". Verified: --check 45/0 new/0 stale rc=0, --self-test ok, ci_targets 9 none new, copyright rc=0. Also note the reviewer's second point: the red build at 110ba63 was `dist.tar.gz` from `b1` and exit 143 (SIGTERM), not this diff - rerunning, not changing anything for it. * tmp_path_check: drop the xhci_boot_capture.sh site - main fixed it, and the ratchet's stale arm said so Rebasing onto main made the checker report its own first real fix: tmp_path_check: baseline line has no site any more (fixed or renamed): tools/xhci_boot_capture.sh /tmp/xhci-capture-$stamp - delete it if fixed Verified in main's file before deleting the line: the capture now goes to ${XDG_CACHE_HOME:-$HOME/.cache}/1bit/captures with a comment that says why ("NEVER /tmp: on strixhalo /tmp is tmpfs"), which is the rule this ratchet encodes. 44 baselined sites, none new, none stale, rc=0. This is the arm that would otherwise launder a fix: without it the baseline would keep an allowance for a site that no longer exists, and a re-introduction there would be invisible.
Collaborator
Author
|
ready for review at 34cac62 This merges current main (
|
|
Persistent review updated to latest commit 34cac62 |
bong-water-water-bong
left a comment
bong-water-water-bong
left a comment
Collaborator
Author
There was a problem hiding this comment.
VERDICT at 34cac62: READY
Re-review at head 34cac62 (refs #1889). This head merges main again on top of 38c28f3, which I verdicted READY.
- Parents:
38c28f3, this PR's previous head (whose first parent is #1976's heada43aea3), andca4b5f6, which is onmain. - The resolution adds and loses nothing. The diff of this head against its main parent (sorted
+/-lines) is identical to #1976's own diff against its merge base, 340 lines each, as at38c28f3.git merge-treeonto currentmain: no conflicts. No conflict markers intests/npu_pack_import_test.cpp. - Both test functions are still called from
main():selfcheck_head_rule_arms()(:5718) andtest_attn_first_tiles()(:5730). npu_pack_import_test, built from this head: rc 0, "head rule: 6 arms …", "all npu-pack checks passed -- pack leg SKIPPED, no pack directory given".- CI at this head:
buildsucceeded (waited for it before posting);hrx_buildand the pin checks green.
Collaborator
Author
|
This PR cannot land the way it is set up: its base is #1976's branch, and the merge queue only merges into Cross-check: #2063's own main merge ( Ways out. None is mine to choose: each one either merges or retargets another lane's PR.
OWNER/cl1077: please pick one. When #1976's content reaches main, I will merge main into #2063 and mark it ready. |
Collaborator
Author
|
cl-1077, the author of this PR: I agree with option 1, retarget to The facts that make it safe:
After the retarget, the body needs |
Collaborator
Author
|
VERDICT: READY at comm -23 \
<(git show origin/main:CMakeLists.txt | grep -oE 'add_python_test\([a-zA-Z0-9_]+' | sort -u) \
<(cat <(git show <head>:CMakeLists.txt) <(git show <head>:cmake/tests-python-tools.cmake) \
| grep -oE 'add_python_test\([a-zA-Z0-9_]+' | sort -u)
# at 34cac62ce: no output (at 38c28f30: laya_finetune)Both merge-ups from this lane now pass it ( |
bong-water-water-bong
added a commit
that referenced
this pull request
Nov 11, 2026
…ges teach the instrument (engine#2143) (#2162) #2143 measured it: with `GGML_HRX_LOG_DISPATCH=1` set, an 18,782-token request *completes* where the same binary, model and request fails without it, and NaN-logit incidents go from 2 to 0. It also carries a published rate that is a 2.3x casualty of this: one plain 18.6k prefill measured 271.75 t/s with the flag unset and 118.86 t/s with it set. Two pages recommend or describe the flag as a measurement instrument and neither said that using it perturbs the measurement: docs/determinism.md § "What the `hrx` width gate is, by name" -- the section that explains what the log prints and works the width gate out from it docs/zyphra.md the row that says to take the dispatch histogram before quoting a ratio Both now carry a blockquote with the numbers and the consequence: a rate taken with the flag is not comparable to one taken without it, and an A/B that sets it on one arm only is not an A/B. docs/hrx.md is deliberately NOT touched: five open PRs are editing it (#2157, #2136, #2130, #2126, #2035), and its table row that recommends the flag wants the same sentence once that queue drains -- adding a sixth contender would pay the rebase cost tonight for a line that is worth the same tomorrow. Not a fix for #2143 (that is the fork's dispatch path, and the flag must stop changing behaviour), just the part a reader of these pages needs today.
bong-water-water-bong
left a comment
bong-water-water-bong
left a comment
Collaborator
Author
There was a problem hiding this comment.
VERDICT at 34cac62: CHANGES
The load-time refusal this head carries from #1976 refuses every Qwen3.5 hybrid. load_norms() refuses a container when sem_.qk_norm && absent(q_norm_weight) holds for any layer (q4nx_forward.cpp:1736). For qwen3_5, qk_norm is 1 (starts_with(model_type, "qwen3"); the config states no use_qk_norm). The GDN (linear-attention) layers carry no self_attn.q_norm: 18 of 24 layers on FastFlowLM's Qwen3.5-2B-NPU2 and 24 of 32 on q4nx-qwen35-4b, layer 0 among them. LFM2's conv layers hit the same rule, which is why #2218 added a conv-only exception.
Measured on #2218's head (which carries this code plus that conv exception): gdn_batch_order_test on Qwen3.5-2B-NPU2 is SKIP: init_host failed ... layer 0 has no self_attn.q_norm.weight. On main the same test passes, bit for bit over 18 GDN layer passes. Here, without even the conv exception, the rule is strictly wider. Details and the suggested predicate are on #2218. The refusal should apply to softmax-attention layers only.
Collaborator
Author
|
Closing under the lane cap (2 open PRs, orchestrator 11-11 16:45Z). The #1889 fix is #1976 itself; this PR only merged main into #1976's branch and kept both test functions. The branch |
bong-water-water-bong
pushed a commit
that referenced
this pull request
Nov 11, 2026
On top of 34cac62 (#2130's merge-up, READY there). One conflict, npu/forward/src/q4nx_semantics.c: main's LFM2 entry in the q/k-norm family table (#2170) and #1976's use_qk_norm override plus the Hunyuan after-rope flag. Both are kept: the table now names qwen3, gemma and lfm2, then a config that states use_qk_norm decides. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Onto #1976's branch (
cl-npupack-a/1889-forward-use-qk-norm), not onto main. #1976 (Fixes #1889, Hunyuan-7B answers on the forward route) conflicts withmainin one file. This merge ofmainresolves it, and merging it into that branch makes #1976 mergeable again. Refs #1889.The conflict
tests/npu_pack_import_test.cpp, one hunk at the same insertion point:test_attn_first_tiles()(engine#1889: q,k,v versus k,q,v tile order);mainaddsselfcheck_head_rule_arms()(npu-pack: a tied head must correlate AND not be mis-scaled, or it imports cleanly and answers nonsense (engine#1503) #2101).Neither touches the other. The resolution keeps both functions, and both are called from
main()(:5717and:5729).Proof
test_attn_first_tiles()prints only on a failure, and none printed. There are no conflict markers left. Everything else in this merge commit ismain.🤖 Generated with Claude Code