docs(compat): 004 is not redundant at b10864, and why - #309
Conversation
The v0.34.1 fold moved llama.cpp b10630 -> b10864, and at b10864 upstream changed its own gemma4 vision defaults from set_limit_image_tokens(40, 280) to (70, 1120) — exactly the endpoints of the ladder 004 enforces. That invites the conclusion that the patch has been upstreamed. It has not, and the next payload bump will invite it again, so the answer belongs in the file rather than in a thread. Checked at b10864: - Those bounds are inputs to calc_size_preserved_ratio, which rounds to align and then clamps down over max_pixels or scales up under min_pixels. It neither snaps to the ladder nor fills to the budget: an under-budget image keeps its natural rounded grid. 004 replaces that call for gemma4 with calc_size_budget_fill. Upstream changed the bounds fed to an algorithm; the patch changes the algorithm. - 004 never touches set_limit_image_tokens. That line appears in the patch only as context, which is also why the patch still applies cleanly. - The fork passes --image-min-tokens 70 --image-max-tokens 1120 explicitly on every gemma4 launch, so the payload's own defaults never bound anything here regardless of what they are. Also records that the MLX path does not use this patch at all — the mlx-metal profiles carry patchset = [] — and gets the same geometry from llm.BudgetFillSize, which must stay in lockstep with the C++ function. Changing one and not the other splits GGUF and MLX onto different grids, and only the GGUF half is covered by this patch's tests. No behaviour change. go test ./llm/ gemma4 budget tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A b9888..b10864 diff of the gemma4 branch shows image_resize_algo moving to RESIZE_ALGO_BICUBIC alongside set_limit_image_tokens moving to (70, 1120), which reads as two simultaneous b10864 changes. Only the token limits are. b10630 — the payload behind 0.33.2 — already set BICUBIC, so the resize kernel is identical on both sides of any 0.33.2-vs-0.34.0 comparison, and the MLX path has always used CatmullRom (Keys a=-0.5, the bicubic family) regardless. Also records that 004's context was re-cut for this bump in fb18f5c, which is why it still applies: at 2b95b4a the context read (40, 280) and on main it reads (70, 1120). Documentation only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Three findings from the v0.34.1 fold session that belong beside this note — all on the same commit the fold pinned in. 1. The only gemma4 change between b10760 and b10864 is 2. The same commit is an open upstream regression, and the fork is immune to it by an accident of the launcher. ggml-org/llama.cpp#28954 (bisected to 3. What the split costs instead, on both payloads. With the fork's automatic generation batch of 1024 (ctx > 4096), a top-rung gemma4 image — 1117 to 1121 tokens in the runner logs — is decoded as two non-causal sub-batches, and the first 1024 image tokens never attend to the last ~95: one-way bidirectional attention at the top rung, for as long as the fork has served 1120 (ADR 0008), so not a fold regression but a latent inconsistency the gates have measured consistently (the post-deploy preflight did 16 such 2-batch decodes on production). The clean fork fix is a generation batch ≥ the 1120 ceiling for gemma4 vision runners (2048, at the compute-buffer cost of the larger ubatch); the measurement that decides it is the top-rung bbox cells at 🤖 Generated with Claude Code |
|
This lands right on top of something measured today on ROCm/gfx1151, and it sharpens the conclusion: 1120 is not just the rung we pass — it is the only rung where 0.34.x is broken. The point in this PR that matters most is "the fork passes The measurement
At 280 and 560 the builds are equivalent — 0.34.1 is marginally better. The failure is confined to the top rung. Scene
Why it matters for this PRThe conclusion here — keep 004, the ladder is a To be explicit about what that does not imply: capping at 560 is not a free mitigation. ADR 0003 raised the ceiling to 1120 for fine-text, and Scope and caveats
🤖 Generated with Claude Code |
|
Why the second commit here matters beyond tidiness: it eliminated a suspect in #310. While investigating a fine-text OCR regression between 0.33.2 and 0.34.0, a A resize-kernel change is an extremely plausible cause for a fine-text recall regression — small glyphs are exactly where interpolation quality shows. It would have been reasonable to stop there. It is not the cause, and the reason is the correction recorded in this PR: b10630 already sets The MLX path is unaffected either way — it has always used CatmullRom, the Keys a=-0.5 cubic, the same family as the reference's PIL BICUBIC. So a diff spanning two payload bumps invited treating both lines as new, and only one is. That is the trap this PR documents, and it cost nothing here only because the vendored b10630 tree was still on disk to check against. |
Answers a question raised while deploying the v0.34.1 fold, and records the answer where it will be found next time.
The question
b10864 changed llama.cpp's own gemma4 vision defaults from
set_limit_image_tokens(40, 280)to(70, 1120)— exactly the endpoints of the ladder004-llama-cpp-gemma4-budget-fill.patchenforces. That looks like upstream converging on our behaviour, and invites deleting the patch.The answer: no, and the numbers matching is a coincidence of endpoints
calc_size_preserved_ratiocalc_size_budget_fillcalc_size_preserved_ratiorounds each axis to align, then clamps down overmax_pixelsor scales up undermin_pixels. It neither snaps to the ladder nor fills to the budget — an under-budget image keeps its natural rounded grid. That is the behaviour 004 exists to replace, and b10864 does not change it.Two checks that settle it:
set_limit_image_tokens. That line appears in the patch only as context — which is also why the patch still applies cleanly to b10864.--image-min-tokens 70 --image-max-tokens 1120explicitly on every gemma4 launch, so the payload's own defaults never bound anything here regardless of what they are.The note says to re-open the question only if upstream adds ladder snapping or budget filling to the dyn_size preprocessor itself — not merely because the numbers match.
Second thing recorded
The MLX path does not use this patch at all: the
mlx-metalpreflight profiles carrypatchset = []. Apple Silicon gets the same geometry fromllm.BudgetFillSize, a Go mirror ofcalc_size_budget_fill. They must stay in lockstep — changing one and not the other silently splits GGUF and MLX onto different grids, and only the GGUF half is covered by this patch's tests. That is easy to miss when reading a llama.cpp patch and concluding it governs everything.Scope
Documentation only, no behaviour change.
go test ./llm/gemma4 budget tests pass. Worth remembering that the ladder is abox_2dgrounding correctness constraint (docs/maxusai/gemma4-bbox-investigation-findings.md), not a performance tuning, so it should not be relaxed for tidiness.🤖 Generated with Claude Code