Skip to content

Commit 88ea48e

Browse files
author
Rita Law
committed
fix(mtmd/qwen3vl): grid selection rewrite + --image-max-tiles override
Rewrite Qwen3VL multi-tile grid selection: enumerate CxR grids that downscale the image in both dimensions (col*tile_px<=nx, row*tile_px<=ny), exclude 1x1 (equivalent to dyn_size), then pick within a ratio-error tolerance band by max tile count and min log-ratio error. Fall back to dyn_size when no grid fits. Add a global overview thumbnail at entries[0]. Add the --image-max-tiles CLI/env override (1..256), applied after load_hparams so GGUF defaults don't clobber it, and clamp preproc_min_tiles so a low override can't produce an empty candidate set on InternVL-family models. Guard the log-ratio division against zero-dimension images. Hardening: - Validate clip.vision.preproc_max_tiles read from the GGUF: clamp to [1,256] (the --image-max-tiles range) and fall back to 4 with a warning, matching the InternVL branch. A crafted/corrupt value could otherwise drive an oversized fitting.reserve() and OOM on load or first inference. - Size fitting.reserve() to max_tiles*ln(max_tiles) (the actual candidate count) instead of max_tiles*2, which undershot ~3x at the top of the range. - Fail a zero-dimension image gracefully (LOG_ERR + return false) instead of GGML_ASSERT, so one malformed upload fails its own request rather than aborting the whole server. Callers already treat false as a per-request error. - Doc/log consistency: reflow --image-max-tiles help to the sibling --image-* style, add the --image-tile-mode/--image-max-tiles rows to the cli/server READMEs, and log preproc_max_tiles as "(custom value)" when overridden. - Add tests/test-qwen3vl-grid.cpp (test-only hparams constructors bypass clip_ctx): max_tiles=1 dyn_size fallback, too-small image, max_tiles=256 ceiling, and the zero-dimension case.
1 parent e9ad5fc commit 88ea48e

14 files changed

Lines changed: 254 additions & 47 deletions

File tree

common/arg.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2270,6 +2270,17 @@ common_params_context common_params_parser_init(common_params & params, llama_ex
22702270
else { throw std::invalid_argument("unknown --image-tile-mode: " + value + " (use batched, sequential, or disabled)"); }
22712271
}
22722272
).set_examples(mmproj_examples).set_env("LLAMA_ARG_IMAGE_TILE_MODE"));
2273+
add_opt(common_arg(
2274+
{"--image-max-tiles"}, "N",
2275+
"maximum number of tiles for multi-tile vision models (e.g. Qwen3VL), overrides the\n"
2276+
"value from the GGUF; use when the GGUF lacks the key or the model default is wrong\n"
2277+
"for your model size (Qwen3VL defaults to 4, too low for 8B+ variants)",
2278+
[](common_params & params, int value) {
2279+
if (value < 1) { throw std::invalid_argument("--image-max-tiles must be >= 1"); }
2280+
if (value > 256) { throw std::invalid_argument("--image-max-tiles must be <= 256"); }
2281+
params.image_max_tiles = value;
2282+
}
2283+
).set_examples(mmproj_examples).set_env("LLAMA_ARG_IMAGE_MAX_TILES"));
22732284
if (llama_supports_rpc()) {
22742285
add_opt(common_arg(
22752286
{"--rpc"}, "SERVERS",

common/common.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -595,6 +595,7 @@ struct common_params {
595595
int image_min_tokens = -1;
596596
int image_max_tokens = -1;
597597
common_image_tile_mode image_tile_mode = COMMON_IMAGE_TILE_MODE_SEQUENTIAL;
598+
int image_max_tiles = -1; // override preproc_max_tiles from GGUF; -1 = use model default
598599

599600
// finetune
600601
struct lr_opt lr;

tests/CMakeLists.txt

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,12 @@ if (LLAMA_MTMD)
297297
llama_build_and_test(test-mtmd-c-api.c)
298298
target_link_libraries(${LLAMA_TEST_NAME} PRIVATE mtmd)
299299
unset(LLAMA_TEST_NAME)
300+
301+
set(LLAMA_TEST_NAME test-qwen3vl-grid)
302+
llama_build_and_test(test-qwen3vl-grid.cpp)
303+
target_link_libraries(${LLAMA_TEST_NAME} PRIVATE mtmd)
304+
target_include_directories(${LLAMA_TEST_NAME} PRIVATE ${PROJECT_SOURCE_DIR}/tools/mtmd)
305+
unset(LLAMA_TEST_NAME)
300306
endif()
301307

302308
# GGUF model data fetcher library for tests that need real model metadata

tests/test-qwen3vl-grid.cpp

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
// Unit tests for Qwen3VL multi-tile grid selection (tools/mtmd/mtmd-image.cpp).
2+
// Exercises mtmd_image_preprocessor_qwen3vl::preprocess() directly via the
3+
// test-only hparams constructor, without loading a real GGUF model.
4+
5+
#include "clip-model.h"
6+
#include "mtmd-image.h"
7+
8+
#include <cstdio>
9+
#include <cstdlib>
10+
11+
// plain assert() is compiled out under -DNDEBUG (this project's default Release
12+
// build), which would make every check below a silent no-op; use an explicit
13+
// check that always runs instead.
14+
#define CHECK(cond) \
15+
do { \
16+
if (!(cond)) { \
17+
fprintf(stderr, "%s:%d: CHECK failed: %s\n", __FILE__, __LINE__, #cond); \
18+
abort(); \
19+
} \
20+
} while (0)
21+
22+
static clip_hparams make_hparams(int image_size, int patch_size) {
23+
clip_hparams hp;
24+
hp.image_size = image_size;
25+
hp.patch_size = patch_size;
26+
hp.n_merge = 2; // matches PROJECTOR_TYPE_QWEN3VL default in clip.cpp
27+
// dyn_size fallback needs these set; mirrors clip.cpp's set_limit_image_tokens(8, 4096) for Qwen-VL
28+
hp.set_limit_image_tokens(8, 4096);
29+
return hp;
30+
}
31+
32+
static clip_image_u8 make_image(int nx, int ny) {
33+
clip_image_u8 img;
34+
img.nx = nx;
35+
img.ny = ny;
36+
img.buf.resize((size_t) nx * ny * 3);
37+
return img;
38+
}
39+
40+
// max_tiles == 1 excludes every grid but 1x1, which is itself excluded as
41+
// "equivalent to dyn_size" -> the candidate set is always empty -> dyn_size fallback.
42+
static void test_max_tiles_one_falls_back_to_dyn_size() {
43+
clip_hparams hp = make_hparams(/*image_size*/ 768, /*patch_size*/ 16);
44+
mtmd_image_preprocessor_qwen3vl pre(hp, /*max_tiles*/ 1);
45+
46+
clip_image_u8 img = make_image(2000, 1000);
47+
clip_image_f32_batch out;
48+
bool ok = pre.preprocess(img, out);
49+
50+
CHECK(ok);
51+
CHECK(out.grid_x == 0 && out.grid_y == 0);
52+
CHECK(out.entries.size() == 1);
53+
printf("test_max_tiles_one_falls_back_to_dyn_size: OK\n");
54+
}
55+
56+
// An image smaller than one tile in both dimensions has no downscaling grid ->
57+
// empty candidate set -> dyn_size fallback (same outcome as max_tiles=1, different cause).
58+
static void test_empty_candidate_set_falls_back_to_dyn_size() {
59+
clip_hparams hp = make_hparams(/*image_size*/ 768, /*patch_size*/ 16);
60+
mtmd_image_preprocessor_qwen3vl pre(hp, /*max_tiles*/ 256);
61+
62+
clip_image_u8 img = make_image(100, 100);
63+
clip_image_f32_batch out;
64+
bool ok = pre.preprocess(img, out);
65+
66+
CHECK(ok);
67+
CHECK(out.grid_x == 0 && out.grid_y == 0);
68+
CHECK(out.entries.size() == 1);
69+
printf("test_empty_candidate_set_falls_back_to_dyn_size: OK\n");
70+
}
71+
72+
// A large, well-fitting image at the max_tiles=256 ceiling should tile, and the
73+
// selected grid must respect the tile budget: grid_x*grid_y <= max_tiles.
74+
static void test_max_tiles_ceiling_selects_valid_grid() {
75+
clip_hparams hp = make_hparams(/*image_size*/ 768, /*patch_size*/ 16);
76+
mtmd_image_preprocessor_qwen3vl pre(hp, /*max_tiles*/ 256);
77+
78+
clip_image_u8 img = make_image(768 * 16, 768 * 16); // square, downscales cleanly at many grids
79+
clip_image_f32_batch out;
80+
bool ok = pre.preprocess(img, out);
81+
82+
CHECK(ok);
83+
CHECK(out.grid_x > 0 && out.grid_y > 0);
84+
CHECK(out.grid_x * out.grid_y <= 256);
85+
CHECK(out.entries.size() == (size_t)(out.grid_x * out.grid_y + 1)); // +1 overview thumbnail
86+
printf("test_max_tiles_ceiling_selects_valid_grid: OK (grid=%dx%d)\n", out.grid_x, out.grid_y);
87+
}
88+
89+
// A zero-dimension image must fail this one preprocess() call gracefully
90+
// (return false) rather than aborting the whole process via GGML_ASSERT.
91+
static void test_zero_dimension_image_fails_gracefully() {
92+
clip_hparams hp = make_hparams(/*image_size*/ 768, /*patch_size*/ 16);
93+
mtmd_image_preprocessor_qwen3vl pre(hp, /*max_tiles*/ 4);
94+
95+
clip_image_u8 img = make_image(0, 100);
96+
clip_image_f32_batch out;
97+
bool ok = pre.preprocess(img, out);
98+
99+
CHECK(!ok);
100+
printf("test_zero_dimension_image_fails_gracefully: OK\n");
101+
}
102+
103+
int main(void) {
104+
test_max_tiles_one_falls_back_to_dyn_size();
105+
test_empty_candidate_set_falls_back_to_dyn_size();
106+
test_max_tiles_ceiling_selects_valid_grid();
107+
test_zero_dimension_image_fails_gracefully();
108+
printf("\nAll qwen3vl grid selection tests passed.\n");
109+
return 0;
110+
}

tools/cli/README.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,8 @@
165165
| `--image, --audio FILE` | path to an image or audio file. use with multimodal models, use comma-separated values for multiple files |
166166
| `--image-min-tokens N` | minimum number of tokens each image can take, only used by vision models with dynamic resolution (default: read from model)<br/>(env: LLAMA_ARG_IMAGE_MIN_TOKENS) |
167167
| `--image-max-tokens N` | maximum number of tokens each image can take, only used by vision models with dynamic resolution (default: read from model)<br/>(env: LLAMA_ARG_IMAGE_MAX_TOKENS) |
168+
| `--image-tile-mode MODE` | tile encoding mode for multi-tile vision models (e.g. Qwen3VL):<br/> batched - all tiles in one forward pass<br/> sequential - tiles encoded one-by-one (default)<br/> disabled - tiling disabled, single tile only<br/>(env: LLAMA_ARG_IMAGE_TILE_MODE) |
169+
| `--image-max-tiles N` | maximum number of tiles for multi-tile vision models (e.g. Qwen3VL), overrides the<br/>value from the GGUF; use when the GGUF lacks the key or the model default is wrong<br/>for your model size (Qwen3VL defaults to 4, too low for 8B+ variants)<br/>(env: LLAMA_ARG_IMAGE_MAX_TILES) |
168170
| `--chat-template-kwargs STRING` | sets additional params for the json template parser, must be a valid json object string, e.g. '{"key1":"value1","key2":"value2"}'<br/>(env: LLAMA_CHAT_TEMPLATE_KWARGS) |
169171
| `--jinja, --no-jinja` | whether to use jinja template engine for chat (default: enabled)<br/>(env: LLAMA_ARG_JINJA) |
170172
| `--reasoning-format FORMAT` | controls whether thought tags are allowed and/or extracted from the response, and in which format they're returned; one of:<br/>- none: leaves thoughts unparsed in `message.content`<br/>- deepseek: puts thoughts in `message.reasoning_content`<br/>- deepseek-legacy: keeps `<think>` tags in `message.content` while also populating `message.reasoning_content`<br/>(default: auto)<br/>(env: LLAMA_ARG_THINK) |

tools/mtmd/clip.cpp

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1416,8 +1416,26 @@ struct clip_model_loader {
14161416
hparams.image_resize_algo = RESIZE_ALGO_BILINEAR;
14171417
get_u32(KEY_SPATIAL_MERGE_SIZE, hparams.n_merge, false);
14181418
get_u32(KEY_WIN_ATTN_PATTERN, hparams.n_wa_pattern, model.proj_type == PROJECTOR_TYPE_QWEN25VL); // only 2.5 requires it
1419-
// optional multi-tile cap; absent in GGUF → stays 0 and the qwen3vl preprocessor falls back to 4
1419+
// SigLIP2-Large ViT (n_head=16, n_layer=24) is used for 2B/4B — max_tiles=4 confirmed.
1420+
// Larger ViT variants (8B+) use SigLIP2-SO-400M with unknown max_tiles.
1421+
// Read from GGUF if present; otherwise default to 4 and warn for unknown ViTs.
1422+
hparams.preproc_max_tiles = 4;
1423+
const bool has_max_tiles_key = gguf_find_key(ctx_gguf.get(), KEY_PREPROC_MAX_TILES) >= 0;
14201424
get_u32(KEY_PREPROC_MAX_TILES, hparams.preproc_max_tiles, false);
1425+
if (has_max_tiles_key && (hparams.preproc_max_tiles < 1 || hparams.preproc_max_tiles > 256)) {
1426+
LOG_WRN("%s: clip.vision.preproc_max_tiles=%d out of range [1,256] in GGUF — defaulting to 4\n",
1427+
__func__, hparams.preproc_max_tiles);
1428+
hparams.preproc_max_tiles = 4;
1429+
}
1430+
if (!has_max_tiles_key && (hparams.n_head > 16 || hparams.n_layer > 24)) {
1431+
const char * model_name =
1432+
model.proj_type == PROJECTOR_TYPE_QWEN2VL ? "Qwen2VL" :
1433+
model.proj_type == PROJECTOR_TYPE_QWEN25VL ? "Qwen2.5VL" : "Qwen3VL";
1434+
LOG_WRN("%s: %s large ViT detected (n_head=%d, n_layer=%d) but "
1435+
"clip.vision.preproc_max_tiles not in GGUF — defaulting to 4. "
1436+
"If using an 8B+ model, set the correct value with --image-max-tiles.\n",
1437+
__func__, model_name, hparams.n_head, hparams.n_layer);
1438+
}
14211439
// ref: https://huggingface.co/Qwen/Qwen2.5-VL-7B-Instruct/blob/main/preprocessor_config.json
14221440
hparams.set_limit_image_tokens(8, 4096);
14231441
hparams.warmup_image_size = hparams.image_size; // warmup at actual tile size to match inference graph shape
@@ -2744,6 +2762,18 @@ struct clip_init_result clip_init(const char * fname, struct clip_context_params
27442762
if (loader.has_vision) {
27452763
ctx_vision = new clip_ctx(ctx_params);
27462764
loader.load_hparams(ctx_vision->model, CLIP_MODALITY_VISION);
2765+
// apply CLI overrides after load_hparams so GGUF defaults don't clobber them
2766+
if (ctx_params.image_max_tiles != -1) {
2767+
ctx_vision->model.hparams.preproc_max_tiles = ctx_params.image_max_tiles;
2768+
LOG_INF("%s: preproc_max_tiles: %d (custom value)\n", __func__, ctx_vision->model.hparams.preproc_max_tiles);
2769+
// models with a min-tiles floor (e.g. InternVL) would produce an empty candidate
2770+
// set if the override drops max below min; clamp min so the invariant holds.
2771+
if (ctx_vision->model.hparams.preproc_min_tiles > ctx_vision->model.hparams.preproc_max_tiles) {
2772+
LOG_WRN("%s: --image-max-tiles=%d is below the model's preproc_min_tiles=%d; clamping min\n",
2773+
__func__, ctx_params.image_max_tiles, ctx_vision->model.hparams.preproc_min_tiles);
2774+
ctx_vision->model.hparams.preproc_min_tiles = ctx_vision->model.hparams.preproc_max_tiles;
2775+
}
2776+
}
27472777
loader.load_tensors(*ctx_vision);
27482778
if (ctx_params.warmup) {
27492779
loader.warmup(*ctx_vision);

tools/mtmd/clip.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,8 @@ struct clip_context_params {
5151
ggml_backend_sched_eval_callback cb_eval;
5252
void * cb_eval_user_data;
5353
const char * backend_device; // optional, if null will use env var or default GPU backend
54-
int image_tile_mode; // 0=batched (default), 1=sequential, 2=disabled
54+
int image_tile_mode; // 0=batched (default), 1=sequential, 2=disabled
55+
int image_max_tiles; // override preproc_max_tiles; -1 = use GGUF/model default
5556
};
5657

5758
struct clip_init_result {

tools/mtmd/mtmd-cli.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,8 @@ struct mtmd_cli_context {
143143
mparams.warmup = params.warmup;
144144
mparams.image_min_tokens = params.image_min_tokens;
145145
mparams.image_max_tokens = params.image_max_tokens;
146-
mparams.image_tile_mode = (int)params.image_tile_mode;
146+
mparams.image_tile_mode = (int)params.image_tile_mode;
147+
mparams.image_max_tiles = params.image_max_tiles;
147148
if (std::getenv("MTMD_DEBUG_GRAPH") != nullptr) {
148149
mparams.cb_eval_user_data = &cb_data;
149150
mparams.cb_eval = common_debug_cb_eval;

0 commit comments

Comments
 (0)