From e4ef7be677ee8c8d69b4822deaab4a176aa8162c Mon Sep 17 00:00:00 2001 From: Glenn Neuber Date: Sun, 11 Oct 2026 13:25:30 +1000 Subject: [PATCH 1/3] fix(vision): an explicit image_max_tokens of 1120 (or min of 70) is honoured; unset reaches the runner as 0 (ADR 0048) api.DefaultOptions fills the image budget with gemma4's 70/1120, so unset and an explicit 70/1120 reached the runners as the same values, and the resolvers for models with other bounds read 70/1120 as "unset" (ADR 0016 kept it as a documented limitation). On muse-glimmer, whose ceiling is 4096, image_max_tokens=1120 sized a 2048x2048 image at 4100 tokens, against 1091 at 1119 or 1121 (MLX, 30b-nvfp4 and 30b-mxfp8, 2026-10-11). - server: modelOptionsWithEmbeddingBatchDefault zeroes each bound neither the request nor the Modelfile set (hasOption, as for draft_num_predict), so unset reaches the runners as 0, "the model's own". - llm (nemotron_h_omni) and mlxrunner (ResolveImageBudget: muse-glimmer, qwen3.5, nemotron_h): only <= 0 is unset; any explicit value is honoured, 70 and 1120 included. - gemma4 is unchanged: its own bounds are 70/1120, and unset and explicit launch the same flags. - Tests that asserted the sentinel now assert the explicit value; TestModelOptionsImageBudgetUnsetIsZero and a nemotron reload row (an explicit 70/1120 is a different launch) are new. - ADR 0048 records the change; ADR 0016's and 0021's sentinel clauses and nemotron-dynres-patch.md point to it. Co-Authored-By: Claude Opus 5.5 --- .../0016-reload-on-resolved-vision-flags.md | 3 +- ...es-vision-through-upstreams-media-model.md | 2 + ...image-budget-reaches-the-runner-as-zero.md | 53 +++++++++++++++++++ docs/maxusai/nemotron-dynres-patch.md | 15 +++--- llm/llama_server.go | 24 ++++----- llm/llama_server_test.go | 48 +++++++++-------- mlxrunner/model/media.go | 29 ++++------ mlxrunner/model/media_test.go | 6 ++- .../model/nemotron_h/media_budget_test.go | 16 +++--- server/routes.go | 22 ++++++++ server/routes_options_test.go | 36 +++++++++++++ server/sched_test.go | 18 +++++-- 12 files changed, 193 insertions(+), 79 deletions(-) create mode 100644 docs/maxusai/adr/0048-an-unset-image-budget-reaches-the-runner-as-zero.md diff --git a/docs/maxusai/adr/0016-reload-on-resolved-vision-flags.md b/docs/maxusai/adr/0016-reload-on-resolved-vision-flags.md index 881056a77e6..42b94e601db 100644 --- a/docs/maxusai/adr/0016-reload-on-resolved-vision-flags.md +++ b/docs/maxusai/adr/0016-reload-on-resolved-vision-flags.md @@ -67,7 +67,8 @@ must not reload, a genuinely different ceiling must — and - The sentinel ambiguity itself remains: an explicit `70/1120` on nemotron is still read as unset and is not expressible. That is now a documented limitation rather than an accident — see the resolver's comment and - `nemotron-dynres-patch.md`. + `nemotron-dynres-patch.md`. **Superseded by [ADR 0048](0048-an-unset-image-budget-reaches-the-runner-as-zero.md): + the server sends an unset budget as 0 and an explicit 70/1120 is honoured.** - `ResolvedImageTokenBudget` is the single place that answers "what budget will this arch launch with", so an arch added to `visionServerArgs` must be added there too or its budget silently stops forcing reloads. The two diff --git a/docs/maxusai/adr/0021-gemma4-serves-vision-through-upstreams-media-model.md b/docs/maxusai/adr/0021-gemma4-serves-vision-through-upstreams-media-model.md index da6ee942df3..806cbf3ee89 100644 --- a/docs/maxusai/adr/0021-gemma4-serves-vision-through-upstreams-media-model.md +++ b/docs/maxusai/adr/0021-gemma4-serves-vision-through-upstreams-media-model.md @@ -54,6 +54,8 @@ Two constraints shaped the work, neither obvious from upstream's interface: `image_max_tokens` is never silently ignored (ADR 0009). Each keeps its own ceiling: a value equal to the shared api default counts as unset, the convention `llm/llama_server.go` already applies for nemotron and qwen-VL. + **Superseded by [ADR 0048](0048-an-unset-image-budget-reaches-the-runner-as-zero.md):** unset now + arrives as 0, and an explicit shared default is honoured. 4. **Bidirectional spans come from `b.Media`, and the opening chunk carries every one of them.** The `SeqOffsets[0] == 0` requirement is kept, because it is load-bearing rather than vestigial: the bidi path attends over the chunk's own diff --git a/docs/maxusai/adr/0048-an-unset-image-budget-reaches-the-runner-as-zero.md b/docs/maxusai/adr/0048-an-unset-image-budget-reaches-the-runner-as-zero.md new file mode 100644 index 00000000000..2f950745355 --- /dev/null +++ b/docs/maxusai/adr/0048-an-unset-image-budget-reaches-the-runner-as-zero.md @@ -0,0 +1,53 @@ +# ADR 0048: An unset image budget reaches the runner as zero + +Date: 2026-10-11 · Status: proposed. Supersedes the "sentinel" parts of +[ADR 0016](0016-reload-on-resolved-vision-flags.md) (its Consequences: "an explicit `70/1120` on nemotron is still +read as unset and is not expressible") and [ADR 0021](0021-gemma4-serves-vision-through-upstreams-media-model.md) +(decision 3: "a value equal to the shared api default counts as unset"), and the matching rows of +[nemotron-dynres-patch.md](../nemotron-dynres-patch.md). + +## Context + +`api.DefaultOptions` fills `image_min_tokens` / `image_max_tokens` with gemma4's ladder, 70/1120 +([ADR 0008](0008-gemma4-budget-fill-restores-1120.md)), so every request reached the runners carrying a budget whether or +not the caller set one. A model whose own bounds differ could not tell the two apart. Two resolvers therefore read +a value equal to 70/1120 as "unset" and substituted the model's own: +- `nemotronImageTokenBudget`, on the llama.cpp path; +- `ResolveImageBudget`, on MLX, which covers muse-glimmer, qwen3.5 and nemotron_h. + +ADR 0016 kept this knowingly: an explicit 70 or 1120 on nemotron "is not expressible — pick an adjacent value". The +resolver comment named the fix: the routes layer's `hasOption` check could tell them apart "if that ever matters". + +It matters now. On muse-glimmer, whose ceiling is 4096, 1120 is the value a client is most likely to send, because it +is the documented default. Measured on `muse-glimmer:30b-nvfp4` and `30b-mxfp8`, MLX, 2026-10-11 (#463), on a +2048×2048 image: + +| `image_max_tokens` | unset | 1024 | 1119 | **1120** | 1121 | 2048 | +|---|---|---|---|---|---|---| +| image tokens | 4100 | 1025 | 1091 | **4100** | 1091 | 2022 | + +A request for 1120 cost 3.7 times what it asked for, and nothing reported it. + +## Decision + +1. **The server sends an unset budget as 0.** `modelOptionsWithEmbeddingBatchDefault` records whether the request or + the Modelfile set each bound (`hasOption`, as it already does for `draft_num_predict`), and zeroes each one that + neither set. That holds only when a model is resolved; a nil model keeps the api defaults. +2. **The resolvers read only `<= 0` as unset.** `nemotronImageTokenBudget` and `ResolveImageBudget` lose their + equality test against the api defaults. Any explicit value is honoured, 70 and 1120 included. +3. **gemma4 is unchanged.** Its resolvers already map `<= 0` to 70/1120, its own bounds. An unset request and an + explicit 70/1120 launch the same flags and do not reload each other (`TestSchedNeedsReloadImageTokenBudget`). + +## Consequences + +- **An explicit 70 or 1120 is now a budget like any other** on nemotron_h (both paths), muse-glimmer and qwen3.5 + (MLX). On nemotron it launches different flags than an unset request, so the scheduler reloads for it, as for any + other change of budget (ADR 0016's B3). +- **Unset behaves exactly as before on every arch.** It still resolves to each model's own bounds. The preflight + rows send no budget except the pinned probe, whose values (560 on gemma4, 3328 on nemotron) were never the + sentinel, so no expectation moves. +- **Clients that sent exactly 70 or 1120 to nemotron or glimmer expecting the model's default now get what they + asked for.** No harness in this repo does: the vision suite sends a budget only when `IMAGE_*_TOKENS` is set. +- **One rule now stands for "unset" across the fork:** `draft_num_predict` on MLX (-1) and the image budget (0) are + both decided at the routes layer, where the request's raw options are still visible. A runner never infers it from a + value. diff --git a/docs/maxusai/nemotron-dynres-patch.md b/docs/maxusai/nemotron-dynres-patch.md index 3aead2289e9..adbb0a05bf0 100644 --- a/docs/maxusai/nemotron-dynres-patch.md +++ b/docs/maxusai/nemotron-dynres-patch.md @@ -102,12 +102,11 @@ Keeping the 128×128 grid through the compat layer and interpolating from it wou - Defaults **256 / 3328** (the model's native bounds). - The shared `ImageMinTokens`/`ImageMaxTokens` options arrive as the gemma4-shaped - DefaultOptions values when the caller left them alone; those exact values are treated - as unset for this arch. The sentinel is `api.DefaultImageMin/MaxTokens` themselves, so - it moves with the gemma4 default — **70 / 1120** today (ADR 0008; 40/1120 before - ADR 0007). Consequence: **explicitly requesting the current gemma4 defaults on - nemotron is not expressible** — pick an adjacent value (71, 1119, 1121…). Any other - value passes through; min clamps down to max. + DefaultOptions values when the caller left them alone. Since ADR 0048 the server sends a + bound neither the request nor the Modelfile set as 0, and only `<= 0` reads as unset + for this arch: an explicit **70 / 1120** (gemma4's defaults) is honoured like any other + value. Before ADR 0048 those exact values were a sentinel for "unset", and requesting + them on nemotron was not expressible. Min clamps down to max. - Values above 3328 are passed through, but exceed the model's training distribution — the reference never produces more than 13,312 patches. Don't raise the ceiling; only lower it (e.g. to trade detail for context on busy hosts, via a Modelfile @@ -129,14 +128,14 @@ For `modelArch == "nemotron_h_omni"` on a payload carrying the 002 patch: |---|---| | flags | `visionServerArgs()` always passes `--image-min-tokens` / `--image-max-tokens` | | defaults | **256 / 3328** (= the model's 1024/13312 pre-merge patch bounds ÷ 4) | -| option resolution | `ImageMinTokens`/`ImageMaxTokens` ≤ 0 **or exactly equal to the gemma4-shaped DefaultOptions values (`api.DefaultImageMin/MaxTokens`, 70/1120 today)** are treated as unset; min clamps down to max; both are **Runner** options — changing either reloads the runner, but only when it changes the *resolved* flags (`llm.ResolvedImageTokenBudget`), so naming 256/3328 explicitly no longer reloads against an unset request | +| option resolution | `ImageMinTokens`/`ImageMaxTokens` ≤ 0 are treated as unset (the server sends an unset bound as 0, ADR 0048; before it, a value exactly equal to the gemma4-shaped DefaultOptions values, 70/1120, was also read as unset); min clamps down to max; both are **Runner** options — changing either reloads the runner, but only when it changes the *resolved* flags (`llm.ResolvedImageTokenBudget`), so naming 256/3328 explicitly no longer reloads against an unset request | | per-image cost | `round(w/32) × round(h/32)` post-resize grid cells **+ 2** marker tokens (``/``), where the resize maps the image, aspect-preserved, into 262,144…3,407,872 px; small images are **upscaled** to the floor | | bounds | floor 256, ceiling 3,328 visual tokens; the 32px floor-alignment lands most shapes slightly under the ceiling (3000×2000 → 3,290), which is exact only when scaled dims hit multiples of 32 (2048×1664 → 3,328) | | ceiling caveat | the pixel budget is enforced before the per-dimension 32px minimum clamp, so **degenerate aspect ratios (≈100:1 and beyond) can exceed it** — e.g. a 4,000,000×1 input targets ~118 MPx and would exhaust memory. Inherited `dyn_size`-family behavior (qwen/kimivl/dots_ocr share it; nemotron was previously immune only because it squashed everything to 512²). Reject absurd-aspect images upstream of Ollama | | warmup | load-time warmup probes the ceiling (`warmup_image_size` ≈ 1846) | | unpatched payload | flags are parsed by llama-server but never consumed: exactly 256 tokens/image, no markers, letterboxed — byte-identical to today | | audio | unaffected (compat force-sets `clip.has_audio_encoder=false`) | -| not expressible | an explicit request of exactly 40 or 1120 (collides with the DefaultOptions sentinels; a `hasOption`-style check at the routes layer could lift this if it ever matters) | +| ~~not expressible~~ | lifted by ADR 0048: the routes layer's `hasOption` check tells an explicit 70/1120 from an unset budget, as this row anticipated | | ceiling clamp | `image_max_tokens` > 3328 clamps down to 3328 — the trained maximum, and a guard against the int32 pixel-math overflow at ≥ 2,097,152 in `set_limit_image_tokens` | Regression tests: `TestVisionServerArgs/nemotron_h_omni_*` in diff --git a/llm/llama_server.go b/llm/llama_server.go index 3cb00f761d1..5353fac1614 100644 --- a/llm/llama_server.go +++ b/llm/llama_server.go @@ -1237,25 +1237,19 @@ const ( ) // nemotronImageTokenBudget resolves the nemotron_h_omni image-token budget -// from opts. The shared ImageMinTokens/ImageMaxTokens options arrive carrying -// the gemma4-shaped DefaultOptions values whenever the caller left them -// alone; at this layer those are not distinguishable from an explicit request -// (server/routes.go's hasOption pattern could tell them apart if that ever -// matters), so treat them, like <= 0, as unset and substitute this arch's -// native bounds. The sentinel is the api.Default* constants themselves, not -// literals, so it tracks the gemma4 default as ADRs move it — currently -// 70/1120 (ADR 0008; it was 40/1120 before ADR 0007). Consequence: an -// explicit 70 or 1120 is not expressible for this arch — pick an adjacent -// value. A client that explicitly sends 256/3328 no longer forces a reload -// against an unset request: ResolvedImageTokenBudget normalizes both to the -// flags they launch before the scheduler compares them. Min is clamped down -// to max like gemma4's resolver. +// from opts. Unset arrives as 0 (server/routes.go zeroes a budget neither the +// request nor the Modelfile set), and <= 0 takes this arch's native bounds. +// Any explicit value is honoured -- gemma4's 70/1120 included, which before +// stood for "unset" here and could not be requested. A client that explicitly +// sends 256/3328 does not force a reload against an unset request: +// ResolvedImageTokenBudget normalizes both to the flags they launch before the +// scheduler compares them. Min is clamped down to max like gemma4's resolver. func nemotronImageTokenBudget(opts api.Options) (minTok, maxTok int) { minTok, maxTok = opts.ImageMinTokens, opts.ImageMaxTokens - if minTok <= 0 || minTok == api.DefaultImageMinTokens { + if minTok <= 0 { minTok = defaultNemotronImageMinTokens } - if maxTok <= 0 || maxTok == api.DefaultImageMaxTokens { + if maxTok <= 0 { maxTok = defaultNemotronImageMaxTokens } // 3328 is the model's trained ceiling (13312 pre-merge patches); raising diff --git a/llm/llama_server_test.go b/llm/llama_server_test.go index 84a274abe48..6648f213200 100644 --- a/llm/llama_server_test.go +++ b/llm/llama_server_test.go @@ -1632,14 +1632,13 @@ func TestVisionServerArgs(t *testing.T) { want: []string{"--image-min-tokens", "256", "--image-max-tokens", "3328"}, }, { - // The gemma4-shaped DefaultOptions values (70/1120 today) mean - // "untouched" and must map to the nemotron-native bounds, not be - // forwarded literally. Asserted through the constants, so the case - // keeps holding when an ADR moves the gemma4 default again. - name: "nemotron_h_omni DefaultOptions values treated as unset", + // Unset reaches the runner as 0 (server/routes.go), so gemma4's + // 70/1120 is an explicit request here like any other value and is + // forwarded; it no longer stands for "untouched". + name: "nemotron_h_omni explicit 70/1120 honoured", arch: "nemotron_h_omni", opts: api.Options{Runner: api.Runner{ImageMinTokens: api.DefaultImageMinTokens, ImageMaxTokens: api.DefaultImageMaxTokens}}, - want: []string{"--image-min-tokens", "256", "--image-max-tokens", "3328"}, + want: []string{"--image-min-tokens", "70", "--image-max-tokens", "1120"}, }, { name: "nemotron_h_omni custom budget", @@ -1655,17 +1654,17 @@ func TestVisionServerArgs(t *testing.T) { }, { // The production-realistic shape: a Modelfile sets only one param, so the - // other arrives at its DefaultOptions sentinel and must be substituted - // per-field, not only when both are sentinels. - name: "nemotron_h_omni max explicit, min at sentinel", + // other arrives unset (0) and must be substituted per-field, not only + // when both are unset. + name: "nemotron_h_omni max explicit, min unset", arch: "nemotron_h_omni", - opts: api.Options{Runner: api.Runner{ImageMinTokens: api.DefaultImageMinTokens, ImageMaxTokens: 2048}}, + opts: api.Options{Runner: api.Runner{ImageMinTokens: 0, ImageMaxTokens: 2048}}, want: []string{"--image-min-tokens", "256", "--image-max-tokens", "2048"}, }, { - name: "nemotron_h_omni min explicit, max at sentinel", + name: "nemotron_h_omni min explicit, max unset", arch: "nemotron_h_omni", - opts: api.Options{Runner: api.Runner{ImageMinTokens: 1024, ImageMaxTokens: api.DefaultImageMaxTokens}}, + opts: api.Options{Runner: api.Runner{ImageMinTokens: 1024, ImageMaxTokens: 0}}, want: []string{"--image-min-tokens", "1024", "--image-max-tokens", "3328"}, }, { @@ -1681,7 +1680,7 @@ func TestVisionServerArgs(t *testing.T) { // ceiling lands on 3328/3328. name: "nemotron_h_omni min above substituted ceiling", arch: "nemotron_h_omni", - opts: api.Options{Runner: api.Runner{ImageMinTokens: 3500, ImageMaxTokens: api.DefaultImageMaxTokens}}, + opts: api.Options{Runner: api.Runner{ImageMinTokens: 3500, ImageMaxTokens: 0}}, want: []string{"--image-min-tokens", "3328", "--image-max-tokens", "3328"}, }, { @@ -1843,11 +1842,11 @@ func TestMaxImageTokens(t *testing.T) { want: 3330, }, { - // The gemma4-shaped DefaultOptions sentinels mean "untouched". - name: "nemotron_h_omni DefaultOptions values treated as unset", + // An explicit 1120 is a ceiling like any other (unset is 0). + name: "nemotron_h_omni explicit 1120 honoured", arch: "nemotron_h_omni", opts: api.Options{Runner: api.Runner{ImageMinTokens: api.DefaultImageMinTokens, ImageMaxTokens: api.DefaultImageMaxTokens}}, - want: 3330, + want: 1122, }, { name: "nemotron_h_omni custom ceiling", @@ -4496,13 +4495,20 @@ func TestResolvedImageTokenBudget(t *testing.T) { wantMin: api.DefaultImageMinTokens, wantMax: api.DefaultImageMaxTokens, wantDerived: true, }, { - // The sentinel means "untouched" here, so it must resolve to the - // same bounds an explicit 256/3328 does — that equality is what - // keeps the scheduler from reloading for identical flags. - name: "nemotron sentinel resolves to native bounds", - arch: "nemotron_h_omni", opts: sentinel, + // Unset (0) must resolve to the same bounds an explicit 256/3328 + // does -- that equality is what keeps the scheduler from reloading + // for identical flags. + name: "nemotron unset resolves to native bounds", + arch: "nemotron_h_omni", opts: api.Options{}, wantMin: 256, wantMax: 3328, wantDerived: true, }, + { + // gemma4's 70/1120 is an explicit request on nemotron, and a + // different launch than unset, so the scheduler reloads for it. + name: "nemotron explicit 70/1120 is its own budget", + arch: "nemotron_h_omni", opts: sentinel, + wantMin: 70, wantMax: 1120, wantDerived: true, + }, { name: "nemotron explicit native bounds resolve identically", arch: "nemotron_h_omni", diff --git a/mlxrunner/model/media.go b/mlxrunner/model/media.go index 660ac9e29c8..0a739d10b29 100644 --- a/mlxrunner/model/media.go +++ b/mlxrunner/model/media.go @@ -9,7 +9,6 @@ import ( _ "golang.org/x/image/webp" - "github.com/ollama/ollama/api" "github.com/ollama/ollama/mlx" ) @@ -106,27 +105,21 @@ type MediaBudgetModel interface { PrepareMediaWithBudget(segments []Segment, imageMinTokens, imageMaxTokens int) (*PreparedRequest, error) } -// ResolveImageBudget folds a request's image-token budget into a model's own, -// using the convention llm/llama_server.go already applies for nemotron and -// qwen-VL: a non-positive value, or one equal to the shared api default, counts -// as unset and the model's own bound stands. -// -// The check against the default is the load-bearing half. api.DefaultOptions -// always populates these fields with gemma4's ladder (ADR 0008), so every -// request carries a budget whether or not the caller meant one. Without this, -// a model that deliberately keeps a different ceiling — glimmer's 4096, chosen -// because lowering it hurts OCR — would have gemma4's 1120 imposed on it by -// every default request, silently discarding detail it was built to keep. -// -// The cost is that a caller cannot explicitly request exactly the shared default -// on such a model; it resolves to the model's own. That is the right trade while -// the defaults are one architecture's ladder rather than a neutral value. +// ResolveImageBudget resolves a request's image-token budget against a model's +// own bounds. A request value <= 0 is unset and takes the model's; anything +// else is the caller's and wins. The server sends 0 for a budget neither the +// request nor the Modelfile set (server/routes.go), so gemma4's 70/1120 -- the +// api defaults -- no longer stand in for "unset": before, a model that keeps a +// different ceiling (muse-glimmer's 4096, qwen3.5's, nemotron_h's) could not be +// asked for exactly 1120, which resolved to its own 4096 (measured on +// muse-glimmer:30b-nvfp4, 2026-10-11). Unset still keeps such a model's own +// ceiling, which is what the sentinel protected. func ResolveImageBudget(reqMin, reqMax, modelMin, modelMax int) (minTok, maxTok int) { minTok, maxTok = reqMin, reqMax - if minTok <= 0 || minTok == api.DefaultImageMinTokens { + if minTok <= 0 { minTok = modelMin } - if maxTok <= 0 || maxTok == api.DefaultImageMaxTokens { + if maxTok <= 0 { maxTok = modelMax } return minTok, maxTok diff --git a/mlxrunner/model/media_test.go b/mlxrunner/model/media_test.go index b4a303872ae..32adf0419cd 100644 --- a/mlxrunner/model/media_test.go +++ b/mlxrunner/model/media_test.go @@ -23,9 +23,11 @@ func TestResolveImageBudgetKeepsTheModelsOwnCeiling(t *testing.T) { {"unset resolves to the model's own", 0, 0, modelMin, modelMax}, {"negative resolves to the model's own", -1, -1, modelMin, modelMax}, { - "shared api default counts as unset", + // Unset reaches the runner as 0, so the shared api default is an + // explicit request on a model with its own bounds, and is honoured. + "an explicit shared api default is honoured", + api.DefaultImageMinTokens, api.DefaultImageMaxTokens, api.DefaultImageMinTokens, api.DefaultImageMaxTokens, - modelMin, modelMax, }, {"an explicitly different value wins", 128, 2048, 128, 2048}, {"only the ceiling set", 0, 2048, modelMin, 2048}, diff --git a/mlxrunner/model/nemotron_h/media_budget_test.go b/mlxrunner/model/nemotron_h/media_budget_test.go index 2aefba830af..483415f5505 100644 --- a/mlxrunner/model/nemotron_h/media_budget_test.go +++ b/mlxrunner/model/nemotron_h/media_budget_test.go @@ -67,7 +67,7 @@ func TestImagePatchBounds(t *testing.T) { wantMin, wantMax int }{ {name: "unset keeps the model's own", minTok: 0, maxTok: 0, wantMin: 1024, wantMax: 13312}, - {name: "the shared api default counts as unset", minTok: api.DefaultImageMinTokens, maxTok: api.DefaultImageMaxTokens, wantMin: 1024, wantMax: 13312}, + {name: "an explicit shared api default is honoured", minTok: api.DefaultImageMinTokens, maxTok: api.DefaultImageMaxTokens, wantMin: 280, wantMax: 4480}, {name: "max maps tokens to patches x4", maxTok: 1024, wantMin: 1024, wantMax: 4096}, {name: "max below the model floor is honoured", maxTok: 128, wantMin: 512, wantMax: 512}, {name: "min raises the floor", minTok: 1024, wantMin: 4096, wantMax: 13312}, @@ -106,16 +106,14 @@ func TestPrepareMediaWithBudgetUnsetMatchesUpstream(t *testing.T) { gh, gw := nemotronImagePatchGrid(h, w, nemotronImagePatchBudget(cfg), cfg) want := []int{1, 3, gh * int(cfg.PatchSize), gw * int(cfg.PatchSize)} + // Unset is 0 (server/routes.go); gemma4's 70/1120 is an explicit request + // on this model and is covered by TestImagePatchBounds. unset, unsetTokens := prepareOne(t, m, data, 0, 0) - shared, sharedTokens := prepareOne(t, m, data, api.DefaultImageMinTokens, api.DefaultImageMaxTokens) - if !slices.Equal(unset.Dims, want) || !slices.Equal(shared.Dims, want) { - t.Fatalf("%dx%d: dims unset=%v shared-default=%v, want upstream's %v", w, h, unset.Dims, shared.Dims, want) + if !slices.Equal(unset.Dims, want) { + t.Fatalf("%dx%d: dims unset=%v, want upstream's %v", w, h, unset.Dims, want) } - if unsetTokens != sharedTokens || unsetTokens != m.visionTokenCount(want[2], want[3]) { - t.Fatalf("%dx%d: tokens unset=%d shared-default=%d, want %d", w, h, unsetTokens, sharedTokens, m.visionTokenCount(want[2], want[3])) - } - if !slices.Equal(unset.MediaData, shared.MediaData) { - t.Fatalf("%dx%d: pixels differ between an unset request and the shared default", w, h) + if unsetTokens != m.visionTokenCount(want[2], want[3]) { + t.Fatalf("%dx%d: tokens unset=%d, want %d", w, h, unsetTokens, m.visionTokenCount(want[2], want[3])) } viaMediaModel, err := m.PrepareMedia([]model.Segment{{Kind: "image", Data: data}}) diff --git a/server/routes.go b/server/routes.go index 808f1ba275d..e7837fe0591 100644 --- a/server/routes.go +++ b/server/routes.go @@ -137,8 +137,15 @@ func (s *Server) modelOptionsWithEmbeddingBatchDefault(model *Model, requestOpts // an unset draft_num_predict from the default. Track that while we still // have the raw model/request option maps. draftNumPredictSet := hasOption(requestOpts, "draft_num_predict") + // The same holds for the image-token budget: DefaultOptions carries + // gemma4's 70/1120 ladder, so an unset budget and an explicit 70 or 1120 + // would reach the runners as the same values. + imageMinSet := hasOption(requestOpts, "image_min_tokens") + imageMaxSet := hasOption(requestOpts, "image_max_tokens") if model != nil { draftNumPredictSet = draftNumPredictSet || hasOption(model.Options, "draft_num_predict") + imageMinSet = imageMinSet || hasOption(model.Options, "image_min_tokens") + imageMaxSet = imageMaxSet || hasOption(model.Options, "image_max_tokens") if err := opts.FromMap(model.GenerationDefaults); err != nil { return api.Options{}, err } @@ -160,6 +167,21 @@ func (s *Server) modelOptionsWithEmbeddingBatchDefault(model *Model, requestOpts opts.DraftNumPredict = 0 } + // An unset budget reaches the runner as 0, "the model's own": every + // resolver reads <= 0 that way (gemma4's own bounds are 70/1120, so + // nothing changes for it). An explicit value -- 70 and 1120 included -- + // is then honoured on the models whose own bounds differ (nemotron_h on + // both paths; muse-glimmer and qwen3.5 on MLX), which before could not + // be asked for exactly gemma4's values. + if model != nil { + if !imageMinSet { + opts.ImageMinTokens = 0 + } + if !imageMaxSet { + opts.ImageMaxTokens = 0 + } + } + return opts, nil } diff --git a/server/routes_options_test.go b/server/routes_options_test.go index 49e2a3d11a4..ec83fceec3a 100644 --- a/server/routes_options_test.go +++ b/server/routes_options_test.go @@ -3,6 +3,7 @@ package server import ( "testing" + "github.com/ollama/ollama/api" "github.com/ollama/ollama/llm" "github.com/ollama/ollama/types/model" ) @@ -319,3 +320,38 @@ func TestUsesAutomaticNumBatch(t *testing.T) { }) } } + +// TestModelOptionsImageBudgetUnsetIsZero: api.DefaultOptions carries gemma4's +// 70/1120 ladder, so before this an unset image budget and an explicit 70 or +// 1120 reached the runners as the same values, and the runners whose own +// bounds differ (nemotron_h on both paths, muse-glimmer and qwen3.5 on MLX) +// had to read 70/1120 as "unset". An explicit 1120 was therefore not +// expressible there: muse-glimmer:30b-nvfp4 sized a 2048x2048 image at 4096 +// tokens under image_max_tokens=1120, and at 1091 under 1119 or 1121 +// (2026-10-11). Unset now reaches the runners as 0, which every resolver +// already reads as "the model's own", and any explicit value is honoured. +func TestModelOptionsImageBudgetUnsetIsZero(t *testing.T) { + for _, tt := range []struct { + name string + model *Model + requestOpts map[string]any + wantMin, wantMax int + }{ + {name: "unset reaches the runner as the model's own", model: &Model{}, wantMin: 0, wantMax: 0}, + {name: "an explicit 1120 is kept", model: &Model{}, requestOpts: map[string]any{"image_max_tokens": float64(1120)}, wantMin: 0, wantMax: 1120}, + {name: "an explicit 70 is kept", model: &Model{}, requestOpts: map[string]any{"image_min_tokens": float64(70)}, wantMin: 70, wantMax: 0}, + {name: "a Modelfile value counts as set", model: &Model{Options: map[string]any{"image_max_tokens": float64(560)}}, wantMin: 0, wantMax: 560}, + {name: "the request overrides the Modelfile", model: &Model{Options: map[string]any{"image_max_tokens": float64(560)}}, requestOpts: map[string]any{"image_max_tokens": float64(1120)}, wantMin: 0, wantMax: 1120}, + {name: "no model: the api defaults stand", model: nil, wantMin: api.DefaultImageMinTokens, wantMax: api.DefaultImageMaxTokens}, + } { + t.Run(tt.name, func(t *testing.T) { + opts, err := (&Server{}).modelOptions(tt.model, tt.requestOpts) + if err != nil { + t.Fatal(err) + } + if opts.ImageMinTokens != tt.wantMin || opts.ImageMaxTokens != tt.wantMax { + t.Fatalf("image budget = %d/%d, want %d/%d", opts.ImageMinTokens, opts.ImageMaxTokens, tt.wantMin, tt.wantMax) + } + }) + } +} diff --git a/server/sched_test.go b/server/sched_test.go index a406683c310..56b7c72c5bb 100644 --- a/server/sched_test.go +++ b/server/sched_test.go @@ -2608,10 +2608,11 @@ func TestSchedNeedsReloadImageTokenBudget(t *testing.T) { ctx, done := context.WithTimeout(t.Context(), schedTestTimeout(100*time.Millisecond)) defer done() - // The image-token bounds are shared options carrying gemma4's defaults, so - // an arch that substitutes its own native bounds for the sentinel launches - // identical flags whether the caller left them alone or named them. Only a - // budget that changes the flags may reload the runner. + // The loaded runner left its budget unset, which the server sends as 0/0 + // (modelOptionsWithEmbeddingBatchDefault). An arch resolves 0 to its own + // native bounds, so a request that names those bounds launches identical + // flags and must not reload; only a budget that changes the flags may -- + // gemma4's 70/1120 included, which on nemotron is now a budget of its own. for _, tc := range []struct { name string family string @@ -2620,11 +2621,17 @@ func TestSchedNeedsReloadImageTokenBudget(t *testing.T) { wantReload bool }{ { - name: "nemotron explicit native bounds match the unset sentinel", + name: "nemotron explicit native bounds match an unset load", family: "nemotron_h_omni", minTok: 256, maxTok: 3328, wantReload: false, }, + { + name: "nemotron explicit 70/1120 is a different launch and reloads", + family: "nemotron_h_omni", + minTok: api.DefaultImageMinTokens, maxTok: api.DefaultImageMaxTokens, + wantReload: true, + }, { name: "nemotron genuinely different ceiling reloads", family: "nemotron_h_omni", @@ -2652,6 +2659,7 @@ func TestSchedNeedsReloadImageTokenBudget(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { do := api.DefaultOptions() + do.ImageMinTokens, do.ImageMaxTokens = 0, 0 // an unset load, as the server sends it runner := &runnerRef{ model: &Model{Config: model.ConfigV2{ModelFamily: tc.family}}, Options: &do, From 328a04812adf17b29d3eb71f141d2db6de1f68a5 Mon Sep 17 00:00:00 2001 From: Glenn Neuber Date: Tue, 13 Oct 2026 12:23:52 +1000 Subject: [PATCH 2/3] fix(vision): an image budget sent as JSON null counts as unset (ADR 0048) hasOption only checks that a key exists, while FromMap skips a nil value, so {"image_max_tokens": null} kept DefaultOptions' 1120 and read as an explicit request for it. On nemotron_h_omni (GGUF) that launched 70/1120 instead of 256/3328 and reloaded against unset requests. The image bounds now use hasNonNilOption, for the request and the Modelfile alike. draft_num_predict and num_batch keep hasOption. Co-Authored-By: Claude Opus 5.5 --- ...-image-budget-reaches-the-runner-as-zero.md | 5 +++-- server/routes.go | 18 +++++++++++++----- server/routes_options_test.go | 7 +++++++ 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/docs/maxusai/adr/0048-an-unset-image-budget-reaches-the-runner-as-zero.md b/docs/maxusai/adr/0048-an-unset-image-budget-reaches-the-runner-as-zero.md index 2f950745355..f15c8eb7903 100644 --- a/docs/maxusai/adr/0048-an-unset-image-budget-reaches-the-runner-as-zero.md +++ b/docs/maxusai/adr/0048-an-unset-image-budget-reaches-the-runner-as-zero.md @@ -31,8 +31,9 @@ A request for 1120 cost 3.7 times what it asked for, and nothing reported it. ## Decision 1. **The server sends an unset budget as 0.** `modelOptionsWithEmbeddingBatchDefault` records whether the request or - the Modelfile set each bound (`hasOption`, as it already does for `draft_num_predict`), and zeroes each one that - neither set. That holds only when a model is resolved; a nil model keeps the api defaults. + the Modelfile set each bound (`hasNonNilOption`, like `hasOption` for `draft_num_predict` but a key sent as JSON + `null` is unset, because `FromMap` skips it and leaves 70/1120 in place), and zeroes each one that neither set. + That holds only when a model is resolved; a nil model keeps the api defaults. 2. **The resolvers read only `<= 0` as unset.** `nemotronImageTokenBudget` and `ResolveImageBudget` lose their equality test against the api defaults. Any explicit value is honoured, 70 and 1120 included. 3. **gemma4 is unchanged.** Its resolvers already map `<= 0` to 70/1120, its own bounds. An unset request and an diff --git a/server/routes.go b/server/routes.go index e7837fe0591..cf94d25e57e 100644 --- a/server/routes.go +++ b/server/routes.go @@ -139,13 +139,14 @@ func (s *Server) modelOptionsWithEmbeddingBatchDefault(model *Model, requestOpts draftNumPredictSet := hasOption(requestOpts, "draft_num_predict") // The same holds for the image-token budget: DefaultOptions carries // gemma4's 70/1120 ladder, so an unset budget and an explicit 70 or 1120 - // would reach the runners as the same values. - imageMinSet := hasOption(requestOpts, "image_min_tokens") - imageMaxSet := hasOption(requestOpts, "image_max_tokens") + // would reach the runners as the same values. A key sent as JSON null is + // unset too: FromMap skips it, so it leaves 70/1120 in place. + imageMinSet := hasNonNilOption(requestOpts, "image_min_tokens") + imageMaxSet := hasNonNilOption(requestOpts, "image_max_tokens") if model != nil { draftNumPredictSet = draftNumPredictSet || hasOption(model.Options, "draft_num_predict") - imageMinSet = imageMinSet || hasOption(model.Options, "image_min_tokens") - imageMaxSet = imageMaxSet || hasOption(model.Options, "image_max_tokens") + imageMinSet = imageMinSet || hasNonNilOption(model.Options, "image_min_tokens") + imageMaxSet = imageMaxSet || hasNonNilOption(model.Options, "image_max_tokens") if err := opts.FromMap(model.GenerationDefaults); err != nil { return api.Options{}, err } @@ -200,6 +201,13 @@ func hasOption(opts map[string]any, name string) bool { return ok } +// hasNonNilOption reports whether opts sets name to a value FromMap applies. +// FromMap skips a nil value, so a key sent as JSON null leaves the default in +// place and must not count as an explicit request. +func hasNonNilOption(opts map[string]any, name string) bool { + return opts[name] != nil +} + // deprecatedOptions may still be set per request, and reach the runner, but // cannot be saved as model parameters. var deprecatedOptions = []string{"typical_p"} diff --git a/server/routes_options_test.go b/server/routes_options_test.go index ec83fceec3a..4949566ad5c 100644 --- a/server/routes_options_test.go +++ b/server/routes_options_test.go @@ -342,6 +342,13 @@ func TestModelOptionsImageBudgetUnsetIsZero(t *testing.T) { {name: "an explicit 70 is kept", model: &Model{}, requestOpts: map[string]any{"image_min_tokens": float64(70)}, wantMin: 70, wantMax: 0}, {name: "a Modelfile value counts as set", model: &Model{Options: map[string]any{"image_max_tokens": float64(560)}}, wantMin: 0, wantMax: 560}, {name: "the request overrides the Modelfile", model: &Model{Options: map[string]any{"image_max_tokens": float64(560)}}, requestOpts: map[string]any{"image_max_tokens": float64(1120)}, wantMin: 0, wantMax: 1120}, + // FromMap skips a nil value, so a key sent as JSON null leaves + // DefaultOptions' 70/1120 in place; it must not read as an explicit + // request for them. + {name: "a null image_max_tokens is unset", model: &Model{}, requestOpts: map[string]any{"image_max_tokens": nil}, wantMin: 0, wantMax: 0}, + {name: "a null image_min_tokens is unset", model: &Model{}, requestOpts: map[string]any{"image_min_tokens": nil}, wantMin: 0, wantMax: 0}, + {name: "a null Modelfile value is unset", model: &Model{Options: map[string]any{"image_min_tokens": nil, "image_max_tokens": nil}}, wantMin: 0, wantMax: 0}, + {name: "a null request keeps the Modelfile value", model: &Model{Options: map[string]any{"image_max_tokens": float64(560)}}, requestOpts: map[string]any{"image_max_tokens": nil}, wantMin: 0, wantMax: 560}, {name: "no model: the api defaults stand", model: nil, wantMin: api.DefaultImageMinTokens, wantMax: api.DefaultImageMaxTokens}, } { t.Run(tt.name, func(t *testing.T) { From 6a7bb3702e3c3c8c978c25681f35d2d9ca39c6df Mon Sep 17 00:00:00 2001 From: Glenn Neuber Date: Tue, 13 Oct 2026 12:23:52 +1000 Subject: [PATCH 3/3] docs(vision): the reload comments describe an unset budget as 0, not the 70/1120 sentinel Co-Authored-By: Claude Opus 5.5 --- llm/llama_server.go | 10 ++++++---- server/sched.go | 15 +++++++++------ 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/llm/llama_server.go b/llm/llama_server.go index 5353fac1614..9d6a59f1802 100644 --- a/llm/llama_server.go +++ b/llm/llama_server.go @@ -1647,10 +1647,12 @@ func Gemma4ImageBudget(opts api.Options) (minTok, maxTok int) { // will actually pass for modelArch under opts, and whether those flags derive // from opts at all. // -// The budget options are arch-agnostic but carry gemma4's defaults, so equal -// flags do not imply equal options: on nemotron the unset sentinel and an -// explicit 256/3328 launch the identical runner. Resolving before comparing -// is what lets the scheduler tell "same launch" from "same request". +// The budget options are arch-agnostic, and an unset one arrives as 0 +// (server/routes.go), which each arch resolves to its own bounds. So equal +// flags do not imply equal options: on nemotron an unset budget and an +// explicit 256/3328 launch the identical runner, as an unset budget and an +// explicit 70/1120 do on gemma4. Resolving before comparing is what lets the +// scheduler tell "same launch" from "same request". // derived is false for arches whose vision flags ignore the options (a fixed // floor, or none at all), where the bounds cannot affect the launch. func ResolvedImageTokenBudget(modelArch string, opts api.Options) (minTok, maxTok int, derived bool) { diff --git a/server/sched.go b/server/sched.go index 4a3fc38e988..64388e175e1 100644 --- a/server/sched.go +++ b/server/sched.go @@ -1551,12 +1551,15 @@ func (runner *runnerRef) needsReload(ctx context.Context, req *LlmRequest) bool optsExisting.NumGPU = -1 optsNew.NumGPU = -1 } - // The image-token bounds are arch-agnostic options carrying gemma4's - // defaults, so unequal values do not imply a different runner: on an arch - // that substitutes its own bounds for the sentinel, an unset request and - // one that names those bounds explicitly launch identical flags. Compare - // what the runner would be started with, not what the caller happened to - // send, or the second request reloads the model for nothing. + // The image-token bounds are arch-agnostic options, and an unset one + // arrives as 0 (server/routes.go), which each arch resolves to its own + // bounds. So unequal values do not imply a different runner: an unset + // request and one that names the arch's own bounds explicitly (256/3328 + // on nemotron, 70/1120 on gemma4) launch identical flags, and an arch + // whose flags ignore the budget launches the same runner whatever was + // sent. Compare what the runner would be started with, not what the + // caller happened to send, or the second request reloads the model for + // nothing. llm.NormalizeImageTokenBudget(req.model.Config.ModelFamily, &optsExisting) llm.NormalizeImageTokenBudget(req.model.Config.ModelFamily, &optsNew) // The same holds for the prompt-cache size: an invalid prompt_cache_ram is