Skip to content

llama/compat: env-gated clip node meter (new 8xx diagnostics band) - #228

Open
glennneuber wants to merge 1 commit into
mainfrom
feat/clip-node-stats-meter
Open

llama/compat: env-gated clip node meter (new 8xx diagnostics band)#228
glennneuber wants to merge 1 commit into
mainfrom
feat/clip-node-stats-meter

Conversation

@glennneuber

Copy link
Copy Markdown

Carries the diagnostic that found the qwen2.5-vl fp16-accumulate fault into the compat band, so it is one build away rather than one investigation away.

What it does

OLLAMA_CLIP_NODE_STATS=<name substring> (or *) logs, for every matching clip graph node, its max_abs, counts above 32k/49k/60k, and non-finite counts.

CLIP_NODE_STATS name=ffn_down-31 op=MUL_MAT type=f32 n=15728640 max_abs=78671.0 hr=1.2010 ... n_inf=1

Run under GGML_CUDA_CUBLAS_COMPUTE_TYPE=f32 to read magnitudes without overflowing; hr = max_abs / 65504 is the fp16 headroom.

Inert unless enabled — without the variable the eval callback is never registered. No behaviour change to a normal build or run.

Why carry it rather than rebuild per incident

The fault it found is invisible from the product surface: garbled text out, and the cause is 3 elements of 15.7 M overflowing at v.blk.31.ffn_down. Without an instrument, debugging degenerates into sampling — and sampling cannot distinguish no fault from not found yet. Concretely, a blind sweep of 297 generated images found nothing on one engine; the meter showed why in about an hour (the whole synthetic region sits ~30 % short of the cliff).

It is also not specific to this bug. The eval callback already sees every node, so the filter covers all mtmd models and any node — including diffing two backends node-by-node, which is the shape of several defects this fork has carried (903-fix-mmq-ids-padding, the mlx-cuda thrashing check, this one).

New 8xx band

0xx compat shims, 9xx upstream defect fixes — a diagnostic is neither, and unlike both it is not waiting on anything, so it is not expected to leave. Documented in llama/compat/README.md, including that an 8xx apply failure after a pin bump means regenerate, not delete.

Verification

  • 41 inserted lines, one anchor (the eval-callback site in the clip_ctx constructor).
  • Clean fetch of b10488: full compat band incl. 801 applies and the mtmd target builds.
  • Runtime behaviour validated earlier with identical code — calibrated against a known trigger (headroom 1.201) versus confirmed non-triggers (0.72–0.75), which is how the typhoon and 0.7.1 reproducers were then located.

Caveat worth keeping

An uncalibrated meter fails silently, not loudly — this one returned a correctly-sized array of zeros on one backend during development. Calibrate against a known trigger before trusting any run. Recorded in the README.

…and)

801-clip-node-stats-meter.patch adds a per-node meter to the clip graph.
With OLLAMA_CLIP_NODE_STATS=<name substring> (or *) each matching node's
output is scanned after evaluation and its max |value|, counts above
32k/49k/60k and non-finite counts are logged. Inert otherwise: without the
variable the eval callback is never registered, so a normal build and a
normal run are unchanged.

Why carry it. The qwen2.5-vl fp16-accumulate fault (ollama#18070)
is invisible from the product surface -- the observable is garbled text,
the cause is a handful of elements out of millions overflowing at
v.blk.31.ffn_down. Diagnosing that without an instrument degenerates into
sampling, and sampling cannot separate "no fault" from "not found yet".
Reconstructing the meter per incident cost hours; in-tree it costs a
build. It covers every mtmd model and any node, not just this one, because
the eval callback already sees the whole graph.

New 8xx band, documented in llama/compat/README.md: diagnostics that are
inert unless enabled. Distinct from 0xx (compatibility shims, leave when
published models do) and 9xx (fork-carried upstream defect fixes, leave
when the pin moves past them) -- 8xx is not waiting on anything and is not
expected to leave. On a pin bump an 8xx failure means regenerate against
the new anchor, not delete.

41 inserted lines at one anchor, the eval-callback site in the clip_ctx
constructor. Verified from a clean fetch of b10488: full band incl. 801
applies and the mtmd target builds. Runtime behaviour was validated
earlier with identical code -- calibrated against a known trigger
(headroom 1.201) versus confirmed non-triggers (0.72-0.75).

Usage, calibration discipline and the trigger corpus:
docs/maxusai/vision-suite/synthetic-triggers/README.md

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@glennneuber

Copy link
Copy Markdown
Author

Reviewing as consolidator. The band is justified and the instrument earns its keep; I checked the CI coverage question because a new band is exactly where it goes missing.

CI does cover it, by construction rather than by luck

I traced both patch jobs:

  • patches applies the whole llama/compat/ set and builds mtmd. 801 anchors in the clip_ctx constructor, and mtmd pulls in clip — so this patch is compiled by an ungated job on every PR touching llama/compat/**.
  • patches-ggml discovers its sources by reading the patch headers (grep -rhE '^\+\+\+ b/ggml/.*\.(cu|cpp)$' llama/compat --include='*.patch'), so it picks up any future 8xx patch that touches ggml without editing the job.

So the new band inherits coverage automatically. That is worth stating in the README section, because #137 landed patches-ggml precisely after finding 903 had been "application-checked but never compiled" — a new band is the natural place for that to recur, and here it does not.

The band itself

0xx shims / 9xx upstream defect fixes / 8xx diagnostics is a real distinction and the load-bearing half is the lifecycle: 0xx and 9xx are waiting on something; a diagnostic is not, so it is not expected to leave. Recording in the README that an 8xx apply failure after a pin bump means regenerate, not delete is the part that will matter in six months, when someone clearing a failed apply reaches for the quickest resolution.

Why I think carrying it is right

sampling cannot distinguish no fault from not found yet

That is the sentence, and this month has three instances of it: a blind sweep of 297 generated images finding nothing while the meter explained why in an hour; readelf absent and read as "no RPATH"; an empty log window read as "the runner does not emit these lines". An instrument that converts absence-of-evidence into a measurement is worth a permanent patch slot.

The generality argument holds too — the eval callback already sees every node, so this is not a qwen2.5-vl tool. Node-by-node backend diffing is the shape of 903-fix-mmq-ids-padding and the MLX thrashing fault as well.

Two small things

Inert-by-default is the property to keep pinned. Without the variable the callback is never registered — worth one line in the README stating that a future 8xx patch must hold the same property, since the band's whole justification is that carrying it costs nothing when unused.

hr = max_abs / 65504 is the useful derived column and deserves its definition in the README rather than only in the PR body; a reader seeing hr=1.2010 in a log six months from now should not have to reconstruct that it means "20% past the fp16 ceiling".

glennneuber added a commit that referenced this pull request Aug 28, 2026
Public, generated reproducers for the qwen2.5-vl fp16-accumulate fault
(ollama#18070), one per affected code path, none needing client data:

  trigger_checker56 / trigger_stripes56       qwen2.5vl:3b, clip path
  trigger_typhoon_c70_dx37_dy35               typhoon + qwen2.5vl:3b, clip
  trigger_typhoon_c70_halfphase               typhoon, clip
  control_typhoon_c70_phase0                  healthy paired control
  trigger_071_nasa_contrast15 (+ NASA base)   qwen2.5vl:3b, BOTH engines

Each ships with a generator that reproduces it byte-identically, and two
ship with a healthy control differing by one parameter -- a 35 px phase
shift, or a contrast multiplier.

Verdicts are n=5 in a fresh container, benign image first, positive
control last. That ordering is load-bearing: a preceding trigger poisons
the slot and the next image then reads as failing. Four silent confounds
were caught this way (poisoned slot, a stale container answering on a
clashing port, a control that could not fire, a log scraper reading the
warm-up pass); each had produced a clean, plausible, wrong answer.

Findings recorded in README.md:
  - Trigger sets are per-checkpoint; nothing transfers between them.
  - The Go engine (removed upstream at v0.30.0) needs a photograph;
    297 generated images found nothing because the synthetic region sits
    ~30% short of the cliff -- shown by measurement, not inferred.
  - Gain amplifies structure an image already has and cannot create it.
  - HF activation screening is anti-predictive and must not be used.
  - The ggml triggers do not transfer to mlx-cuda, but with no MLX
    positive control that is "does not transfer", not "immune".

Consolidated from 16 commits: the falsified HF proxy tooling and its
~115 KB of rows are dropped in favour of the finding, and the clip meter
now lives in llama/compat/801-clip-node-stats-meter.patch (PR #228).

Image rights in IMAGE-PROVENANCE.md: the checkerboards are generated and
carry no third-party rights; the photograph is a US federal government
work (NASA/Bill Ingalls, nasa_id 20040421_exp9_02).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant