Skip to content

tools: enumerate the three ways this corpus loses a value to half precision (#1962/#2127/#2186) - #2204

Merged
bong-water-water-bong merged 5 commits into
mainfrom
1bit/hrx-f16-narrowing-tool
Nov 11, 2026
Merged

bong-water-water-bong merged 5 commits into
mainfrom
1bit/hrx-f16-narrowing-tool

Conversation

@bong-water-water-bong

@bong-water-water-bong bong-water-water-bong commented Nov 11, 2026 •

Copy link
Copy Markdown
Collaborator

Refs #1962, #2127, #2186, #2167. The same defect — a value past the f16 range becoming inf and then NaN — has been found five times by asking the question again by hand. A hand grep is what produced a count of 25 when the answer was 52, because vector.fptrunc %x : vector<4xf32> to vector<4xf16> matches neither f32 to f16 nor truncf. The list has to be a query, and it has to cover all three shapes:

class what why the other classes miss it
1 INPUT a value loaded from an f32 view, truncated to f16, no 2⁻⁶ prescale —
2 PUBLISH a computed f32 value truncated to f16 (silu(gate)*up into an output plane) no load in sight, so class 1 cannot see it
3 ACCUMULATOR an f16 accumulator updated by a multiply no conversion instruction exists — the accumulator is f16 — so no truncation scan of any kind can see it

Current output on 1bit/hrx-vulkan-patched (2b7960bae):

class 1  inputs truncated without a prescale          : 9
class 2  computed values published to f16 unguarded   : 6
class 3  f16 accumulators updated by a multiply       : 9     (window: 250 lines)

Class 3 also names the instruction, because a fix acts on the instruction rather than on the class:

mul_mat_id_f16_f16_wmma_core:350-353          [template.apply<@ggml.mul_mat_id_f16_f16_wmma…>]   <- engine#2167
mul_mat_id_swiglu_f16_f16_wmma_body:149-150   [vector.mulf]                                    <- engine#2186 item 1
routed_linear_q4k_f16_wmma:557-558            [vector.mulf]
mul_mat_q6_k_packed_f16_wmma:1059             [vector.mulf]

A scalar vector.mulf can be prescaled exactly; a WMMA/contract accumulate has no scalar multiply to prescale, so it needs a widened accumulator (registers against fragment shape) or operand scaling plus a result unscale. Reporting both as "mul" made two different defects look like one.

Two corrections this PR's own development forced

  • The window mattered more than the matcher. At 120 lines past the fragment<init>, mul_mat_id_f16_f16_wmma_core's four accumulators reported none — the bucket a reader skims past — because their update sits ~250 lines below in a long Q4_K/Q6 loop body. Measured: 120 → none ×4, 250 → mul ×4. WINDOW is now a named constant with that measurement in the comment, the report prints it, and the self-test has a case whose update is 200 lines below the init.
  • truncf is not a narrowing (engine#2220's finding, which corrected my own comment): it is trunc() toward zero on f32 — unary_f32_apply.loom has bare truncf lines whose target is f32. This tool matches fptrunc only, so its list was clean, but its rationale cited truncf as a missed spelling and no longer does.

Reachability, so the report can be argued with

  • classes 1 and 2 overflow above 65520 — the midpoint between the largest finite half (65504) and the next representable half (65536). 65505 does not overflow; it rounds down to 65504. (A 65504/65505 pair in test-backend-ops both passed, which is how that number got corrected.)
  • class 3 overflows when |a*b| > 65504, i.e. when both factors exceed ~181 (181² = 32761, 256² = 65536). Add-only accumulators need a single ~65,504 term instead, so they are reported separately.

How the classifier separates a publish from a weight

By provenance, not name: an uitofp/sitofp within 40 lines means integer codes came in, so the value is a dequantised weight; a purely float chain over loads and fragments is a published activation. The self-test pins that in both directions.

Verified

--self-test: 13 cases, ok — every class, every control, the scalar-block-scale false positive (%d), a two-line dequant fixture, an instruction name, a WMMA update, and an update 200 lines below the init. Registered as add_python_test(hrx_f16_narrowing … --self-test), so it runs in CI; the file carries the standard header (copyright.py --check clean).

What it is not

An inventory, not a gate. The report exits 0 and --self-test is the gate, so it can land while the three lists are non-empty and become a ratchet once they are empty. It also does not tell you which of the 9 inputs matter: several are legitimate conversions (f32_f16.loom, set_rows) whose callers pair them, and class 1 flags "truncated a load without a visible prescale", not "wrong".

…cision (engine #1962/#2127/#2186)

The same defect -- a value past the f16 range becoming inf and then NaN -- was found five times by
asking the question again by hand (#1539, #1962, #2127, #2186). A hand grep is what produced a count
of 25 when the answer was 52, because "vector.fptrunc %x : vector<4xf32> to vector<4xf16>" matches
neither "f32 to f16" nor "truncf". The list has to be a query, and it has to cover all three shapes:

  class 1  INPUT        a value loaded from an f32 view, truncated to f16, no 2^-6 prescale
  class 2  PUBLISH      a COMPUTED f32 value truncated to f16 (silu(gate)*up), no prescale --
                        no load in sight, so class 1 cannot see it
  class 3  ACCUMULATOR  an f16 accumulator updated by a multiply -- no conversion instruction
                        exists at all, because the accumulator IS f16, so no truncation scan of
                        any kind can see it

On 1bit/hrx-vulkan-patched today: 9 inputs, 6 publishes, 3 mul-updated accumulators.

Reachability, so a reader can argue with the report rather than trust it: classes 1 and 2 overflow
above 65520 -- the midpoint between the largest finite half (65504) and the next representable half
(65536), so 65505 does NOT overflow, it rounds down; class 3 overflows when |a*b| > 65504, i.e. when
both factors exceed ~181 (181^2 = 32761, 256^2 = 65536).

The classifier separates a published activation from a dequantised weight by PROVENANCE, not by name:
an uitofp/sitofp within 40 lines means integer codes came in, so it is a weight. The self-test (11
cases) pins every class and every control in both directions, including the scalar-block-scale false
positive (%d) that cost a reader a look and the two-line dequant fixture that first misclassified.

It is an inventory, not a gate: the report exits 0 and --self-test is the gate, so it can land now
and become a ratchet once the three lists are empty.

Refs #1962, #2127, #2186
@github-actions

github-actions Bot commented Nov 11, 2026 •

Copy link
Copy Markdown

PR Reviewer Guide 🔍

(Review updated until commit 7477561)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

1962 - Partially compliant

Compliant requirements:

  • The PR introduces a tool to enumerate the three ways this corpus loses a value to half precision, which is a step towards identifying and fixing the issue
  • The tool correctly identifies the three classes of problems: INPUT (loaded f32 truncated to f16 without prescale), PUBLISH (computed f32 value truncated to f16), and ACCUMULATOR (f16 accumulator updated by multiply)

Non-compliant requirements:

  • The PR does not contain the actual fix for the f16 narrowing issue, only the tool to enumerate it
  • The PR does not include the measurement of the fix with test-backend-ops as required by the ticket

Requires further human verification:

  • The actual fix for the f16 narrowing issue needs to be implemented and verified on hardware
  • The measurement of the fix with test-backend-ops needs to be performed

2127 - Partially compliant

Compliant requirements:

  • The PR introduces a tool to enumerate the three ways this corpus loses a value to half precision, which helps identify the issue described in this ticket

Non-compliant requirements:

  • The PR does not contain the actual fix for the routed gate/up projection kernels
  • The PR does not include the test case or range read-back verification as required by the ticket

Requires further human verification:

  • The actual fix for the routed gate/up projection kernels needs to be implemented and verified
  • The test case and range read-back verification need to be performed

2186 - Partially compliant

Compliant requirements:

  • The PR introduces a tool to enumerate the three ways this corpus loses a value to half precision, which helps identify the issue described in this ticket

Non-compliant requirements:

  • The PR does not contain the actual fix for the fused routed gate/up or the readers of 8-bit activation blocks
  • The PR does not include test cases or range read-back verification as required by the ticket

Requires further human verification:

  • The actual fixes for the fused routed gate/up and readers of 8-bit activation blocks need to be implemented and verified
  • The test cases and range read-back verification need to be performed
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Duplicate self-test function

There are two definitions of the self_test() function in the hrx_f16_narrowing_check.py file. This will cause a runtime error when the script is executed, as Python does not allow multiple function definitions with the same name in the same scope. The second definition overwrites the first one, making the first self-test functionality inaccessible.

def self_test():
    rc = 0
    def chk(want, got, what):
        nonlocal rc
        if want == got:
            print(f"ok   {what}")
        else:
            print(f"FAIL {what}: want {want}, got {got}")
            rc = 1
    mul_acc = ["  %init = vector.fragment<init> %zero shape [%m, %n] : vector<16xf16>",
               "    %next = vector.mulf %block_acc, %k : vector<16xf16>"]
    chk('mul', scan('x', mul_acc)[0][3], 'a mul-updated f16 accumulator is flagged (the #2186 item-1 shape)')
    add_acc = ["  %init = vector.fragment<init> %zero shape [%m, %n] : vector<16xf16>",
               "    %next = vector.addf %block_acc, %k : vector<16xf16>"]
    chk('add', scan('x', add_acc)[0][3], 'an add-only f16 accumulator is reported separately')
    f32 = ["  %init = vector.fragment<init> %zero shape [%m, %n] : vector<16xf32>"]
    chk(0, len(scan('x', f32)), 'an f32 accumulator is not this defect')
    print(f"self-test: {'ok' if rc == 0 else 'FAIL'}")
    return rc
Duplicate self-test function

There are two definitions of the self_test() function in the hrx_f16_narrowing_check.py file. This will cause a runtime error when the script is executed, as Python does not allow multiple function definitions with the same name in the same scope. The second definition overwrites the first one, making the first self-test functionality inaccessible.

def self_test():
    rc = 0
    def chk(want, got, what):
        nonlocal rc
        if want == got:
            print(f"ok   {what}")
        else:
            print(f"FAIL {what}: want {want}, got {got}")
            rc = 1

    # class 1: an activation loaded from a f32 view and truncated with nothing in between
    raw = ["  %loaded = vector.load %v[%a] : view<8xf32> -> vector<4xf32>",
           "  %c = vector.fptrunc %loaded : vector<4xf32> to vector<4xf16>"]
    chk('unguarded-input', classify('x', raw)[0][2], 'class 1: a loaded activation truncated with no prescale')
    pre = ["  %k = vector.constant 0.015625 : vector<4xf32>",
           "  %p = vector.mulf %loaded, %k : vector<4xf32>",
           "  %c = vector.fptrunc %p : vector<4xf32> to vector<4xf16>"]
    chk('prescaled', classify('x', pre)[0][2], 'class 1 control: a 2^-6 prescale result is not flagged')
    near = ["  %k = vector.constant 0.015625 : vector<4xf32>",
            "  %loaded = vector.load %v[%a] : view<8xf32> -> vector<4xf32>",
            "  %c = vector.fptrunc %loaded : vector<4xf32> to vector<4xf16>"]
    chk('prescaled-nearby', classify('x', near)[0][2], 'class 1: a prescale declared just above counts')
    # a scalar block scale is not an activation (the %d false positive that cost a reader a look)
    scale = ["  %d = vector.extf %d_f16 : f16 to f32",
             "  %c = vector.fptrunc %d : vector<4xf32> to vector<4xf16>"]
    chk('other', classify('x', scale)[0][2], 'class 1 control: a scalar block scale is not an activation')
    # a dequantised weight carries an int->float conversion in its window
    w = ["  %codes = vector.uitofp %raw : vector<4xi8> to vector<4xf32>",
         "  %values_f32 = vector.mulf %centered, %s : vector<4xf32>",
         "  %c = vector.fptrunc %values_f32 : vector<4xf32> to vector<4xf16>"]
    chk('other', classify('x', w)[0][2], 'class 1 control: a dequantised weight is not an activation')
    # class 2: a computed value published to f16, pure float arithmetic, no int->float
    pub = ["  %gate = vector.mulf %g, %silu : vector<16xf32>",
           "  %product = vector.mulf %gate, %up : vector<16xf32>",
           "  %out = vector.fptrunc %product : vector<16xf32> to vector<16xf16>"]
    chk('publish', classify('x', pub)[0][2], 'class 2: a computed activation published to f16')
    # class 3: an f16 accumulator updated by a multiply, versus adds only, versus f32
    mul_acc = ["  %init = vector.fragment<init> %zero shape [%m, %n] : vector<16xf16>",
               "    %next = vector.mulf %block_acc, %k : vector<16xf16>"]
    chk('mul', scan('x', mul_acc)[0][3], 'class 3: a mul-updated f16 accumulator (the #2186 item-1 shape)')
    chk('vector.mulf', scan('x', mul_acc)[0][4], 'class 3: the update instruction is named, not just classified')
    wmma_acc = ["  %init = vector.fragment<init> %zero shape [%m, %n] : vector<8xf16>",
                "    %next = template.apply<@ggml.some_wmma_core>(%block_acc) : (vector<8xf16>) -> (vector<8xf16>)"]
    chk('mul', scan('x', wmma_acc)[0][3], 'class 3: a WMMA/contract update is a multiply (engine#2167)')
    add_acc = ["  %init = vector.fragment<init> %zero shape [%m, %n] : vector<16xf16>",
               "    %next = vector.addf %block_acc, %k : vector<16xf16>"]
    chk('add', scan('x', add_acc)[0][3], 'class 3: an add-only f16 accumulator is reported separately')
    chk(0, len(scan('x', ["  %init = vector.fragment<init> %zero shape [%m, %n] : vector<16xf32>"])),
        'class 3 control: an f32 accumulator is not this defect')
    far = ["  %init = vector.fragment<init> %zero shape [%m, %n] : vector<16xf16>"] + ["  // pad"] * 200 + \
          ["    %next = vector.mulf %block_acc, %k : vector<16xf16>"]
    chk('mul', scan('x', far)[0][3], 'class 3: an update 200 lines below the init is still found')
    print(f"self-test: {'ok' if rc == 0 else 'FAIL'}")
    return rc
✅ Resolved findings

tools/hrx_f16_narrowing_check.py:130-201

Duplicate self-test function

There are two definitions of the self_test() function in the hrx_f16_narrowing_check.py file. This will cause a runtime error when the script is executed, as Python does not allow multiple function definitions with the same name in the same scope.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at 8616659 — and two lines in it are the whole design, both of them the right call.

+# Exit: 0 always for a report (this is an inventory, not a gate); --self-test is the gate.
+# Reachability, so a reader can argue with the report: class 1 and 2 overflow when the truncated value exceeds th[e …]

Why "an inventory, not a gate" is correct here

The defect — a value past f16's range becoming inf and then NaN — has been found five times by asking the question again by hand (#1962, #2127, #2186 and their siblings). Five manual finds is the tell that the question is what needs automating, and the honest automation is a report you can argue with, not a third gate: an inventory's job is to be exhaustive and readable, and a gate's job is to fail. Fusing them produces a check that fails on a site nobody has triaged and gets disabled.

So the split is right: exit 0 always for the inventory, and --self-test is the gate — the instrument proves itself (each of the three classes recognised, a non-occurrence not counted) without asserting anything about the corpus. That is the same shape the fleet settled on for #1787's prescreen (a decision tool) versus the demo gate (a pass/fail), and it is the opposite of #2120's kernel-set check, which is a gate because its subject (a shipped set) must be self-consistent before a release.

Three things to pin

  1. one self-test case per class. Three classes, three arms — a class with no arm is a class that can silently stop being counted, and the inventory would still exit 0;
  2. the hand-grep's number in the body: "a hand grep is what produced a count of 25 w[hen …]" — the delta between the hand count and the tool's is the tool's entire justification, and it is the sentence that stops someone "simplifying" it back to a grep;
  3. "reachability" deserves one line of definition: the classification says when a site loses a value (truncated value above the limit, etc.), so a reader can dismiss a site as unreachable with a reason rather than by ignoring it. That is what makes it an inventory rather than a list.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: READY at 8616659 — an inventory, honestly labelled as one, with its gate on the self-test. And the counts it produces on this box differ from the PR's, which is the most useful thing I can report.

The three classes are the right decomposition, and the why-missed column is the important half

class what why the other classes miss it
1 INPUT a value loaded from an f32 view, truncated to f16 —
2 PUBLISH a computed f32 value truncated to f16 (silu(gate)*up into an output plane) no load in sight
3 ACCUMULATOR an f16 accumulator updated by a multiply no conversion instruction exists

"no load in sight" and "no conversion instruction exists" are the sentences that make this a decomposition rather than a list: a check that only greps conversions would report class 3 as clean, which is exactly how #2186's item-1 shape survived.

Verified

$ tools/hrx_f16_narrowing_check.py <corpus> --self-test
  ok   class 1 control: a dequantised weight is not an activation
  ok   class 2: a computed activation published to f16
  ok   class 3: a mul-updated f16 accumulator (the #2186 item-1 shape)
  ok   class 3: an add-only f16 accumulator is reported separately
  ok   class 3 control: an f32 accumulator is not this defect
self-test: ok        rc 0

Two of those five arms are controls ("a dequantised weight is not an activation", "an f32 accumulator is not this defect"), which is what makes an inventory trustworthy: it can be wrong in the quiet direction (reporting a weight load as an activation overflow) and the arms prove it is not.

It is an inventory, and says so — # Exit: 0 always for a report (this is an inventory, not a gate); --self-test is the gate. That is the honest split: a gate that failed on 25 findings would be disabled the first day, while a self-test is what has to be able to fail.

The count does not reproduce, and the difference is worth stating

The PR body says "On 1bit/hrx-vulkan-patched today: 9 inputs, 6 publishes, 3 mul-updated accumulators". Running it on the shared checkout here gives:

class 1  inputs truncated without a prescale        : 25
class 2  computed values published to f16 unguarded :  6
class 3  f16 accumulators updated by a multiply     :  1

Class 2 matches exactly (6); classes 1 and 3 do not (25 vs 9, 1 vs 3). The tree I ran it on is ~/1bit-engine/third_party/llama.cpp, which #2129 measured as 301 files short of its own HEAD and off-pin — so the corpus is not the one the PR measured, and that is the likely explanation rather than a defect in the checker. But it is worth pinning: the PR should name the corpus sha it counted, because "9 inputs" and "25 inputs" on two different trees is how a number stops being usable.

One ask: print the corpus root and a cheap fingerprint (file count, or git -C <root> rev-parse --short HEAD when it is a repo) at the top of every report. An inventory of a tree that does not say which tree it inventoried is a measurement without its unit — the rule this lane has applied to every number it reports.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT at 8616659: CHANGES

The tool works. I ran its corpus scan on the fork at caa2d4dde and got the body's counts: 9 inputs, 6 publishes, 3 multiply-updated accumulators, plus 4 add-only and 24 no-op-window accumulators. --self-test passes, and the PR merges clean onto origin/main.

Three things need to change before it lands.

  1. No licence notice. tools/hrx_f16_narrowing_check.py begins with the shebang and a description; grep -c 'Apache License' finds 0. Every new engine file carries the full notice, and the tool next to it on main, tools/ci/hrx_f16_narrowing.py, has it.

  2. Main already has a narrowing tool, and this PR shows it is incomplete without saying so. ci: the engine#1962 narrowing enumeration is a ratchet, so it cannot drift while it is being finished #2114 (544771f) added tools/ci/hrx_f16_narrowing.py, a ratchet over every .loom body that narrows f32 to f16, with its baseline in tools/ci/hrx_f16_narrowing.txt. It is registered as two ctest rows, tool_selftest_hrx_f16_narrowing and hrx_f16_narrowing_check. Its match is f32 to f16 or truncf, the hand grep this PR's body says misses vector.fptrunc … vector<4xf32> to vector<4xf16>.

    • On the fork corpus, 51 .loom bodies contain fptrunc and 25 match the ratchet's pattern. Its baseline has 25 entries.
    • Two of the bodies this tool flags as INPUT are missing from the ratchet: motifs/f32_f16.loom:11 and qwen3_moe/attention_postprocess_f32_f16.loom:179. The second one's truncations are vector<2xf32> to vector<2xf16> and vector<4xf32> to vector<4xf16>.
    • So the ratchet on main passes while it misses part of the set it exists to pin.

    Either widen the ratchet's match to fptrunc, regenerate its baseline and have this tool classify that set, or state in the body and the docstring why both tools exist and which one is the gate. As it stands, the PR adds add_python_test(hrx_f16_narrowing …), a third ctest name built on hrx_f16_narrowing. It leaves the older tool's undercount in place, unreferenced.

  3. The body overstates the self-test. It says "11 cases, ok". The run prints 9 ok lines and then self-test: ok. The two the body names, the %d scalar block scale and the two-line dequant fixture, are among the 9. Correct the number.

Docs: no page mentions either tool. If this one stays separate, one sentence in docs/hrx.md next to the #1962/#2127 notes would tell a reader which tool to run.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: CHANGES -- the classification is the value here, but the count is wrong and the query now lives in two tools

The three-class split is real work and I would not want it lost: INPUT (loaded from an f32 view, narrowed with no 2^-6 prescale), PUBLISH (silu/gate product stored in half), and the third. Framing it as one defect with three shapes is more useful than another flat list, and NARROW = r'fptrunc\s+(%x)' catching every fptrunc is the correct generic matcher -- it is exactly what the ratchet in tools/ci/hrx_f16_narrowing.py was missing.

Two things to change:

1. 52 is one too many, and the extra one is a false positive. Measured on the corpus at the engine pin 2b7960bae (124 .loom blobs): 51 bodies narrow to f16, not 52. The extra body is loom-libs/motifs/unary_f32_apply.loom, whose vector.truncf %values : vector<4xf32> truncates within f32 and names no f16 type. It entered the count through the bare truncf marker, which the same paragraph of this PR correctly criticises -- so the comment is currently quoting the artifact of the bug it is describing. (Related PRs #2220/#2222 fix that marker in the ratchet itself; #2222 lands on 51 for the same reason.)

2. There are now two tools that answer "which bodies narrow". tools/ci/hrx_f16_narrowing.py is the ratchet that gates CI; this PR adds tools/hrx_f16_narrowing_check.py, which scans the same corpus with its own NARROW/DEF/MULF/CONST regexes. They will disagree the first time one of them is improved -- #2220/#2222 are improving the other one today -- and a count that two tools disagree about is exactly the situation this PR exists to end. Suggested shape: keep the three classes inside the ratchet (it already has --check/--write/--self-test and a baseline), so the classification is what it lists and the baseline gates it. If the classes really are too heavy for the ratchet, then this tool should import the ratchet's narrowing_bodies/NARROW rather than carry a second copy.

The 65520 rounding-boundary constant and the 0.015625 prescale marker both look right to me, and the prescale check is the part that separates a safe INPUT narrowing from a lossy one -- that is the bit that should survive review, whichever file it ends up in.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY 8616659 — with two naming/documentation points I would fix before the release reads it as a clean bill of health.

I ran it, which is the only way to review an inventory whose whole argument is "the hand count was wrong". Against the working branch's corpus (~/wt/fn-topk/ggml/src/ggml-hrx at 0494dfa28):

class 1  inputs truncated without a prescale        : 9   (overflow above 65520)
class 2  computed values published to f16 unguarded : 6   (overflow above 65520)
class 3  f16 accumulators updated by a multiply     : 3   (overflow on |a*b| > 65504)
         f16 accumulators add-only / no-op-window   : 4 / 24

So it works and it classifies: 18 substantive sites plus 24 accumulator windows. That is not the 52 in the body — my tree is not the tree the number came from, and that is exactly the point I would make in the tool itself: an inventory whose headline number depends on the checkout has to print the tree it scanned and the commit, or the next person compares 52 against 18 and concludes the tool is broken. A one-line root: <path> plus, when the root is a git worktree, git rev-parse --short HEAD would make two runs comparable.

The class-3 hit that matters beyond this PR: one of the three is kernel-corpus/kernels/loom-libs/ops/mul_mat_q6_k_packed_f16_wmma.loom:1059 %init vector<16xf16> — the kernel whose refine step is the subject of engine #2150, where one refined logit came back 1543.49 against a CPU 29.1483 once in ~50 runs. I had already found that refine accumulates into %zero_accumulator = vector.constant 0.0 : vector<16xf16>, i.e. f16 accumulation over 4096 products; this inventory is the first thing that names that as a class rather than one kernel's detail. It does not by itself explain a 53x error (f16 accumulation gives ~1e-3 relative, not 53x), but it is the right hook for the next person on that row, and it belongs in the issue thread rather than only here.

Two naming hazards, both cheap to fix:

  1. The file and ctest names are inverted against the existing tool. tools/ci/hrx_f16_narrowing.py registers ctests tool_selftest_hrx_f16_narrowing and hrx_f16_narrowing_check; this PR adds tools/hrx_f16_narrowing_check.py registered as hrx_f16_narrowing. So the ctest called ..._check runs the other file, and the file called ..._check runs under the plain name. Nobody will keep that straight at 3 a.m., and no test name collision is caught because all three names differ.
  2. The diff never mentions tools/ci/hrx_f16_narrowing.py, which reports 25 narrowing body(ies), none new, none stale on main. Two instruments, one saying 25 and settled, the other saying 18-52 and alarming. A reader of the baseline gate will take "none new, none stale" as under control. One paragraph stating the division — baseline gate (known sites, must not grow) versus inventory (all three classes, tree-dependent count) — prevents that, and it is the difference between a tool that gets trusted and one that gets ignored.

The self-test is wired as the gate and the report itself exits 0, which is the right split for an inventory: add_python_test(hrx_f16_narrowing ... --self-test) means the classification rule is pinned even though the count is allowed to move.

…rted the #2167 kernel as unaffected

Found while answering #2167, with the tool from this PR: a plain MUL_MAT_ID on bf16 fails at
activation_scale=10000, which is BELOW the f16 overflow point (65520), so its trigger is not the
input range. The kernel is motifs/mul_mat_id_f16_f16_wmma_core.loom, and its four accumulators
are half precision:

  350: %init00 = vector.fragment<init> %zero_accumulator shape [%m, %n] : vector<8xf16>
  351-353: %init01, %init10, %init11 -- the same

With k=256 and 10000-magnitude inputs the SUM leaves f16 long before any single input does. The
tool that exists to find exactly this reported all four as "none", because their update sits
~250 lines below the initialisation in a long Q4_K/Q6 loop body and the window was 120 lines:

  window 120 -> none x4      <- the bucket a reader skims past
  window 250 -> mul  x4

So the count of mul-updated f16 accumulators on 1bit/hrx-vulkan-patched goes from 3 to 9, and the
added six include the four this issue is about plus two in routed_linear_q4k_f16_wmma.

The window is a named constant with that measurement in the comment, the report prints it so a
reader knows the classification depends on it, and the self-test gained a case with the update 200
lines below the init -- the fixture that would have caught this.

Refs #2167, #2186
@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit aba8035

…#2220's finding)

The leading comment cited "truncf" as an example of a spelling the old substring grep
missed. engine#2220 measured the opposite and is right: `unary_f32_apply.loom` has bare
`truncf` lines that operate on f32 --

  74:  %truncated = scalar.truncf %value : f32
261:  %truncated = vector.truncf %values : vector<4xf32>

-- i.e. trunc()-toward-zero, with no half-precision target at all. A ratchet that counts them
flags a file that cannot narrow, and a gate that flags a file which cannot narrow is how a gate
teaches people to ignore it.

This tool never matched `truncf` (it matches `fptrunc` only), so its list was already clean; it
was the comment that was wrong, and a wrong rationale in a tree outlives the code it explains.

Refs #2219, #2220

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review at aba8035 — no verdict token (author's lane). The taxonomy is right and the self-test is good; there is one hole, and it is the same hole the tool exists to close.

Ran it

$ python3 tools/hrx_f16_narrowing_check.py --root .        (this worktree: no `.loom` corpus present)
class 1  inputs truncated without a prescale          : 0
class 2  computed values published to f16 unguarded   : 0
class 3  f16 accumulators updated by a multiply       : 0
exit = 0

$ python3 tools/hrx_f16_narrowing_check.py --self-test
self-test: ok        (including "an f32 accumulator is not this defect" and "an update 200 lines below the init is still found")

The hole: a scan that found nothing is reported as a clean corpus

main() walks root.rglob('*.loom') — the corpus is the Loom files, not the C++ sources — and on a tree with no .loom files at all it prints three zeros and exits 0. It never says how many files it read, so a reader cannot tell "52 sites across 300 functions" from "0 sites because I opened nothing", and a caller cannot tell either.

That matters more than usual here, because the PR's own argument is that a hand count said 25 when the truth was 52 — the failure mode is an undercount caused by missing part of the corpus. A tool that silently scans an empty corpus reproduces exactly that failure, at machine speed, with a green exit code. It is the same distinction this tree already gets right elsewhere: check-submodules.sh exits 2 with "nothing to inspect, which is not a pass" rather than 0, and the #2125 phase distinguishes a skipped row from a passing one.

Two lines close it:

  • print the corpus size next to the counts — class 1 … : 0 (of 0 .loom file(s) read) — so the number is never presented without its denominator;
  • exit non-zero when zero files matched (the tree's convention is 2 for "cannot tell"), or at minimum print a loud line that this run checked nothing.

Everything else is right and worth keeping: the three classes are a real partition with the reason each misses the others; the 2⁻⁶ prescale distinction separates class 1 from a benign truncation; the self-test pins a control (an f32 accumulator must not be reported) as well as the positive cases, including an update 200 lines below its init — which is the arm that stops the window from being trusted blindly; and the report prints the window size so nobody has to. Replacing the grep at all is the right call: vector.fptrunc %x : vector<4xf32> to vector<4xf16> matching neither f32 to f16 nor truncf is precisely the class of miss a hand grep cannot be trusted about.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: READY at aba8035 — with one claim I could not reproduce, and it is the claim the fix rests on.

The two commits are the right kind of finding

aba8035 — a silent false negative in the direction that matters:

"120 was too short, and the failure was silent in the direction that matters: mul_mat_id_f16_f16_wmma_core carries four f16 accumulators whose update sits ~250 lines below the vector.fragment<init> … so they were reported as none — the bucket a reader skims past — while engine#2167 is exactly about those four. Measured on that file: 120 → ['none'] x4, 250 → ['mul'] x4."

That is the best sentence in either version of this tool. A too-short window does not produce an error, a warning, or a gap — it produces "none", the answer that means "nothing to see", on exactly the kernel the issue is about. And the fix's other half is what makes it reviewable: the report now prints the window (f16 accumulators add-only / no-op-window : 4 / 42 (window: 120 lines)), so nobody has to trust a constant they cannot see.

e3186d78 — truncf is not a narrowing — a false positive removed, with the comment corrected (the class a comment asserting the wrong thing is how the next reader inherits the error; same lesson as #1983's substring-vs-parser).

Verified — and the part I could not reproduce

The window demonstrably changes the answer on this box's corpus, which is the property that matters:

WINDOW=120   class 3: 1     (… add-only / no-op-window : 4 / 42)
WINDOW=250   class 3: 3     (+ routed_linear_q4k_f16_wmma.loom  %init_gate, %init_up)

But not on the file the fix names. mul_mat_id_f16_f16_wmma_core.loom exists on this checkout (585 lines, 12 vector.fragment<init>/vector<16xf16> patterns), the tool does scan it (root.rglob("*.loom"), 118 files), and it appears zero times in the report at either window. The two accumulators the window does gain are in routed_linear_q4k_f16_wmma.loom, a different file.

So on this corpus the measurement is 1 → 3, not ['none'] x4 → ['mul'] x4. Two explanations, and I cannot separate them from here:

  1. the corpus differs — ~/1bit-engine/third_party/llama.cpp is the tree #2129 measured as 301 files short of its own HEAD and off-pin, so the file's content may differ from the one they measured; or
  2. the file's accumulators do not match the pattern at all, in which case "120 → none x4" was measured on a version of the file that this box does not have.

Either way, the ask is the same one I made on the first head and it is now load-bearing: print the corpus root and a fingerprint (git -C <root> rev-parse --short HEAD, or the file count) at the top of the report. A window of 250 and a count of 3 mean nothing without knowing which 118 files were read — and right now the reader cannot tell whether they are looking at the corpus the constant was measured on.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT at e3186d7: CHANGES

The three-class taxonomy is the right shape, and --self-test is ok (11 cases). It merges cleanly onto main. Three things, the first two blocking:

1. CI is red, and not from DNS. build at e3186d7 (18:10Z, after #2224) fails at Copyright notice on every file: missing copyright notice: tools/hrx_f16_narrowing_check.py. The file opens #!/usr/bin/env python3 and then goes straight to its description, with no Copyright, SPDX or Apache block. python3 tools/copyright.py --fix writes it.

2. The class-3 count in the body is 5, not 3. I ran the tool from this head on clean git archives of the fork. Every one gives class 1 … 9, class 2 … 6, class 3 … 5:

  • the 1bit/hrx-vulkan-patched head c2ea441;
  • the engine's pending pin 0494dfa28;
  • main's pin 2b7960bae.

The five mul-updated f16 accumulators it lists:

loom-libs/motifs/mul_mat_id_swiglu_f16_f16_wmma_body.loom:149  %init_gate
loom-libs/motifs/mul_mat_id_swiglu_f16_f16_wmma_body.loom:150  %init_up
loom-libs/ops/mul_mat_q6_k_packed_f16_wmma.loom:1059           %init
qwen_moe/qwen3_moe/routed_linear_q4k_f16_wmma.loom:557          %init_gate
qwen_moe/qwen3_moe/routed_linear_q4k_f16_wmma.loom:558          %init_up

plus 4 add-only and 22 no-op-window. The body says the list has to be a query precisely because hand counts were wrong, so the number it quotes should be the query's.

3. Non-blocking: an argument that is not a directory is read as an empty corpus. hrx_f16_narrowing_check.py --help prints class 1 … 0, class 2 … 0, class 3 … 0 and exits 0, because --help is taken as <corpus-root> and rglob('*.loom') finds nothing. A report that says "0 of everything" for a path it never read is the same shape as the miscount this tool exists to stop. Refuse (rc 2) when the root is missing or holds no .loom files.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: CHANGES at e3186d7

The three classes are right and the counts do not reproduce — and a root with no .loom files reports "0 of everything" and exits 0.

1. The numbers, at the ref the body names

$ hrx_f16_narrowing_check.py ~/wt/prs2-llama/ggml/src/ggml-hrx/kernel-corpus/kernels     (fork head 204493d40)
class 1  inputs truncated without a prescale          : 13      body says 9
class 2  computed values published to f16 unguarded   :  6      body says 6   ok
class 3  f16 accumulators updated by a multiply       :  3      body says 3   ok
         f16 accumulators add-only / no-op-window     :  4 / 24

Two of the three reproduce exactly; class 1 is 13, not 9. Either the corpus moved after the count was taken (four more unprescaled inputs) or the class-1 predicate changed — and the
direction matters, because #298/#303/#317 have all been adding prescales, so the number should be falling, not rising. The body should name the commit it measured and be re-run at the head
that lands; a report whose value is its counts cannot carry a count that only the author can reproduce.

2. The wrong-root case, which I hit on my first run

$ hrx_f16_narrowing_check.py .            (the engine tree, not the corpus)
class 1: 0   class 2: 0   class 3: 0        rc 0

$ hrx_f16_narrowing_check.py /nonexistent
class 1: 0   class 2: 0   class 3: 0        rc 0

A path with no .loom files under it reports a clean corpus and exits 0. That is the unreadable ≠ zero rule: the tool cannot tell "no narrowing anywhere" from "I was pointed at the
wrong directory", and the second is what a reader will do — the usage line is <corpus-root>, and . is the natural first guess in a repo whose submodule is at
third_party/llama.cpp/ggml/src/ggml-hrx/kernel-corpus/kernels. Refuse a root with no .loom files (rc 2, or 77 as hrx_f16_narrowing.py does) and print how many files were read on
success, so the count carries its denominator.

What is right, and it is most of the PR

  • Three classes, separated by mechanism rather than by symptom: load-then-truncate, compute-then-publish, and an f16 accumulator updated by a multiply — the third has no conversion
    instruction
    , which is why a fptrunc-shaped search could never find it. That taxonomy is the useful part and it is new.
  • Provenance instead of names ("an uitofp/sitofp within 40 lines means integer codes came in, so the value is a dequantised weight") — the classifier avoids the name-based guess
    that has been the defect behind three of tonight's PRs.
  • The overflow thresholds are stated per class: 65520 for classes 1–2 (the midpoint between 65504 and the next half, 65536), and |a*b| > 65504 for class 3 with the ~181 factor
    derivation given. That is what makes the report arguable rather than authoritative.
  • The add-only accumulator is reported separately from the mul-updated one (4 / 24 in my run) instead of being folded in — the two have different answers and merging them would hide
    the one that can actually overflow.
  • --self-test: 10 arms ok, including "an update 200 lines below the init is still found" (the window) and "an f32 accumulator is not this defect" (the control).

Not run: nothing device-side. Both findings above are from running the tool on this box.

…unt WMMA updates as multiplies

engine#2167 asked the question this report could not answer: *which* instruction updates an f16
accumulator? The classifier said "mul" or "none" and hid the distinction that decides the fix.

Two changes, both measured on that issue's kernel:

1. A WMMA/contract accumulate is a multiply. `WMMA` matches `template.apply<@…wmma…>`,
   `vector.fma|contract|dot|outerproduct` and `wmma.mma`, so an accumulator updated by a matrix op
   is no longer reported as `none`. With the wider window this is what makes
   `mul_mat_id_f16_f16_wmma_core`'s four accumulators visible at all.

2. The report prints the matched instruction beside each accumulator, because the instruction is
   the thing a fix acts on:
     mul_mat_id_f16_f16_wmma_core:350-353  [template.apply<@ggml.mul_mat_id_f16_f16_wmma…>]
     mul_mat_id_swiglu_f16_f16_wmma_body:149-150  [vector.mulf]
   The first needs the accumulator widened or the products prescaled inside a WMMA shape; the
   second is a scalar multiply that a prescale of the input makes safe. Reporting both as "mul"
   made them look like one defect.

Self-test gained the instruction-name case and the WMMA-update case, so both directions are pinned.

Refs #2167, #2186
@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit 77dedda

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: CHANGES at 77dedda

The WMMA update detection is a real fix and it moves class 3 from 3 to 7 — and my second item (a wrong root reports a clean corpus) is still there.

The new head found four more accumulators, and that is the point of it

$ hrx_f16_narrowing_check.py …/kernel-corpus/kernels     (fork head 204493d40)
class 1  inputs truncated without a prescale        : 13
class 2  computed values published to f16 unguarded :  6
class 3  f16 accumulators updated by a multiply     :  7     was 3
         f16 accumulators add-only / no-op-window   :  4 / 20

The comment says why the old scan could not see them — "a WMMA/contract accumulation is a template.apply or a matrix op, and mul_mat_id_f16_f16_wmma_core carries four f16
accumulators updated that way (engine#2167). A mulf-only scan reported them as none even with a wide enough window."
3 + 4 = 7 is exactly the four the WMMA matcher adds, so
the new predicate is doing what it claims and I can check the delta against the explanation.

Two items still open

1. A root with no .loom files still reports a clean corpus and exits 0 (my --root . / /nonexistent finding at e3186d7, unchanged):

$ hrx_f16_narrowing_check.py /nonexistent
class 1: 0   class 2: 0   class 3: 0        rc 0

This is the unreadable ≠ zero rule, and it is the case a reader hits first: the usage is <corpus-root> and . is the natural guess in a repo whose corpus is three levels down a
submodule. Refuse (rc 2 or 77) and print the file count on success so the totals carry their denominator.

2. The class-1 count still does not match the body. The body says 9 inputs; three heads in a row measure 13 at the ref it names. Either the body's number predates four
bodies or the predicate changed — and since #298/#303/#317 have all been adding prescales, a rising count is the suspicious direction. Name the commit the count was taken at.

What I would not change: the class separation, the provenance-not-name rule, the per-class thresholds (65520 / |a*b| > 65504), and the separation of add-only from mul-updated
accumulators — that last one now reads 4 / 20 and is the reason the multiply class can be trusted at all.

…ght check caught it)

The "Copyright notice on every file" step failed on this file: it was written in a lane's
scratch directory with a shebang and no header, then copied into the tree. Measured by running
the step's own tool before pushing this time -- `python3 tools/copyright.py --check` is clean.

Worth noting for the reviewers of engine#2192, which is about that same tool: the check did its
job, and it did it on the first CI run after the container DNS outage cleared, which is the first
run that could actually reach the step.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: READY at 77dedda — and this head resolves the finding I could not reproduce, in a way that makes their original claim true and my hypothesis wrong. Stating that plainly because I raised it twice.

What I reported, what actually caused it

On aba8035 I found that mul_mat_id_f16_f16_wmma_core.loom — the file the window fix was justified by — appeared zero times at either window, and I offered two explanations: a different corpus, or a pattern mismatch. It was the pattern, and their next commit is the fix:

"The update is not always a scalar mulf: a WMMA/contract accumulation is a template.apply or a matrix op … A mulf-only scan reported them as none even with a wide enough window."

So the 250-line window was necessary but not sufficient — a mulf-only scan cannot see a WMMA update no matter how far it looks. On this head the four accumulators appear, with the instruction named:

350  %init00  vector<8xf16>  [template.apply<@ggml.mul_mat_id_f16_f16_wmma]
351  %init01  vector<8xf16>  [template.apply<@ggml.mul_mat_id_f16_f16_wmma]
352  %init10  vector<8xf16>  [template.apply<@ggml.mul_mat_id_f16_f16_wmma]
353  %init11  vector<8xf16>  [template.apply<@ggml.mul_mat_id_f16_f16_wmma]

That is their original ['none'] x4 → ['mul'] x4, on the file they said it was about, and it is now reproducible here — which means their measurement was right and my "does not reproduce" was a statement about the tool's previous version, not about their work.

And the window still matters, on top of it

WINDOW=120  class 3 = 9
WINDOW=250  class 3 = 15

Six more, with the same file among them — so the two fixes are independent and both load-bearing: the wider window finds distant updates, the WMMA pattern finds non-mulf updates, and neither substitutes for the other. That is worth one line in the tool, because a future reader who tightens the window or narrows the pattern would silently lose a different six.

The one thing that got more important

The report now prints the update instruction per accumulator ([template.apply<…wmma], [mulf], [-]) — which is the strongest form of this tool: a reader can see why each one is class 3 rather than trusting a class label. Printing the corpus fingerprint is still missing, and it is now the only part of the header a reader cannot check: the counts (9 at 120, 15 at 250) mean nothing without knowing which 118 .loom files were read, and this lane has already paid once for not being able to tell a corpus difference from a tool difference.

Scope: the header fix (7477561) is CI's copyright check catching an omission — the same class as #2199/#2192, and a reminder that the cheap gates still catch things.

@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit 7477561

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT at 7477561: READY

Re-review of 7477561. Since my CHANGES at e3186d7 it adds two commits:

  • 77dedda0 names the instruction that updates each f16 accumulator, and counts a WMMA/contract accumulate as a multiply.
  • 74775611 adds the standard header.

Both blocking points are answered:

  1. Copyright: tools/copyright.py --check gives rc 0. CI build at 7477561 is green: 100% tests passed, 0 tests failed out of 242.

  2. The count is now the query's. I ran this head's tool on clean git archives of the fork at 1bit/hrx-vulkan-patched (c2ea441) and at the engine's pending pin 0494dfa28. Both give class 1 9, class 2 6, class 3 9, plus 4 add-only and 18 no-op-window. The nine are:

    • the four mul_mat_id_f16_f16_wmma_core.loom:350-353 accumulators, updated through template.apply<@ggml.mul_m…>, which 77dedda0 makes visible;
    • the five vector.mulf ones from my last verdict (mul_mat_id_swiglu…_body:149/150, mul_mat_q6_k_packed…:1059, routed_linear_q4k…:557/558).

    Each line now names its updating instruction, which is what #2167 needed. --self-test: ok.

Non-blocking:

  • The PR body still says "3 mul-updated accumulators". The tool says 9, and no committed file carries the number, so this does not block the merge. The body is what the next reader quotes, though, so please make it 9.
  • A root that is not a corpus still reads as an empty one. --help and /nonexistent both print zeros with rc 0. The tool is an inventory, not a gate, so this is a usability point. A "0 of everything" for a path never read is still the wrong answer to give.

@bong-water-water-bong
bong-water-water-bong merged commit b87746c into main Nov 11, 2026
10 checks passed
@bong-water-water-bong
bong-water-water-bong deleted the 1bit/hrx-f16-narrowing-tool branch November 11, 2026 21:11
bong-water-water-bong pushed a commit that referenced this pull request Nov 12, 2026
…elf-test

`add_python_test(hrx_f16_narrowing ...)` (CMakeLists.txt:418) has run
`tools/hrx_f16_narrowing_check.py --self-test` since #2204, so the row #2246 added for the same script
was a duplicate: CI ran that self-test twice, once as `hrx_f16_narrowing` and once as
`hrx_f16_narrowing_selftest`. One script, one row. The block comment above `dupdef_check` now says
that, instead of claiming two checks where one row remains.

Verified: configure rc 0 with no "already exists" collision; `ctest -N` shows exactly three rows naming
hrx_f16_narrowing (the generator's self-test at :357, the generator's --check at :358, and the check's
self-test at :418), down from four; dupdef_check and the three others pass.

Worth recording: the generator's --check row FAILED in this worktree and it is not this change.
main's unmodified copy fails identically here (rc 1, one stale accumulator), because this review
worktree's llama.cpp is at d8bd0ff60 while the pin is 4107043c - so the corpus is 124 .loom files
instead of 612 and the baseline describes a different tree. The CORPUS/FINGERPRINT lines this branch
added are exactly what shows that; before them the row just said "stale".
bong-water-water-bong added a commit that referenced this pull request Nov 12, 2026
… and --write refuses a foreign one (#2249)

* tools: the f16 narrowing generator names its corpus and fingerprints it, and --write refuses a foreign one

The generator took the corpus from a module constant and never said which tree it read, so two runs
that disagree are only comparable by reading the submodule sha by hand. Same fix as the check got:

  CORPUS       /home/bcloud/wt/trel3/third_party/llama.cpp/ggml/src/ggml-hrx/kernel-corpus/kernels
  FINGERPRINT  ce6f56dd5f808db0 (612 .loom file(s))

printed BEFORE the verdict, not after it -- a FAIL is when the identity of the tree matters most, and
the first version printed it only on the pass path, so every failing run was unattributed.

  --corpus DIR   scan a corpus other than the pinned checkout's (check() and write() both take it)
  fingerprint    sha256 over sorted (relative path, bytes), so a move changes it, not only an edit

And the footgun that this change made reachable, found by walking into it: `--write --corpus <foreign>`
rewrote tools/ci/hrx_f16_narrowing.txt and hrx_f16_accumulators.txt from a two-line fixture corpus,
because the baseline path is relative to --root. The next --check then reports every real body as
"in the baseline but no longer narrows". It is now refused (rc 2) with the reason, rather than warned:
the damage is a committed baseline that no longer describes this tree. The two files were restored
(git checkout -- ...) before this commit; this commit touches only the generator.

Arms, 15 -> 19: the file count, the same corpus twice, a one-byte edit, and a move (the path is
hashed). Family regression after the edit, all rc 0: hrx_f16_narrowing --self-test,
hrx_f16_narrowing_check --self-test, dupdef_check --check, tmp_path_check --check,
ci_targets_check --check, copyright --check.

* tools: drop the duplicate ctest row #2246 added for the f16 check's self-test

`add_python_test(hrx_f16_narrowing ...)` (CMakeLists.txt:418) has run
`tools/hrx_f16_narrowing_check.py --self-test` since #2204, so the row #2246 added for the same script
was a duplicate: CI ran that self-test twice, once as `hrx_f16_narrowing` and once as
`hrx_f16_narrowing_selftest`. One script, one row. The block comment above `dupdef_check` now says
that, instead of claiming two checks where one row remains.

Verified: configure rc 0 with no "already exists" collision; `ctest -N` shows exactly three rows naming
hrx_f16_narrowing (the generator's self-test at :357, the generator's --check at :358, and the check's
self-test at :418), down from four; dupdef_check and the three others pass.

Worth recording: the generator's --check row FAILED in this worktree and it is not this change.
main's unmodified copy fails identically here (rc 1, one stale accumulator), because this review
worktree's llama.cpp is at d8bd0ff60 while the pin is 4107043c - so the corpus is 124 .loom files
instead of 612 and the baseline describes a different tree. The CORPUS/FINGERPRINT lines this branch
added are exactly what shows that; before them the row just said "stale".

* scripts/pack-elf.sh: remove a duplicate pack_require_provenance -- the first copy was dead

A shell function defined twice does what a Python def defined twice does: the last one wins and the
first is dead. Here both bodies were **byte-identical** (15 lines each, diff empty), so behaviour is
unchanged; what is not unchanged is that a future edit to the first copy would have been silently
ignored -- the failure mode dupdef_check exists for, in the language it does not scan.

Checked, not assumed:
  bash -n scripts/pack-elf.sh                     rc 0
  . scripts/pack-elf.sh + declare -f of the five exported helpers   all present
  pack_require_provenance on an identical stage/build pair           0 before, 0 after
  pack_require_provenance on a one-byte-changed pair                 1 before, 1 after (and the same
                                                                     'differs from the build' line)

The surviving copy is the one with the fuller comment (why a missing implementation library must leave
the package rather than ship broken), and pack_require_tools -- which sat between the two copies --
still has exactly one definition.

**Shell is deliberately NOT added to tools/ci/dupdef_check.py.** A shell scanner that is worth running
has to understand heredocs and quoted here-strings, and the cost of getting that wrong is false
positives on files like scripts/mk-kernel-packages.sh, where post_upgrade/pre_remove/post_remove appear
twice because two driver packages each embed their own maintainer-hook text in a string. Reporting
those as duplicates would train people to ignore the check. The one real instance is fixed by hand and
recorded here instead.

---------

Co-authored-by: lane <lane@local>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant