Merge pull request #175 from tetherto/tetherto/fix/qwen3vl-grid-select

QVAC-21396 fix(mtmd/qwen3vl): grid selection rewrite + --image-max-tiles override

Author: gianni-cor

common/arg.cpp

@@ -2270,6 +2270,17 @@ common_params_context common_params_parser_init(common_params & params, llama_ex
             else { throw std::invalid_argument("unknown --image-tile-mode: " + value + " (use batched, sequential, or disabled)"); }
         }
     ).set_examples(mmproj_examples).set_env("LLAMA_ARG_IMAGE_TILE_MODE"));
+    add_opt(common_arg(
+        {"--image-max-tiles"}, "N",
+        "maximum number of tiles for multi-tile vision models (e.g. Qwen3VL), overrides the\n"
+        "value from the GGUF; use when the GGUF lacks the key or the model default is wrong\n"
+        "for your model size (Qwen3VL defaults to 4, too low for 8B+ variants)",
+        [](common_params & params, int value) {
+            if (value < 1)   { throw std::invalid_argument("--image-max-tiles must be >= 1"); }
+            if (value > 256) { throw std::invalid_argument("--image-max-tiles must be <= 256"); }
+            params.image_max_tiles = value;
+        }
+    ).set_examples(mmproj_examples).set_env("LLAMA_ARG_IMAGE_MAX_TILES"));
     if (llama_supports_rpc()) {
         add_opt(common_arg(
             {"--rpc"}, "SERVERS",

common/common.h

@@ -595,6 +595,7 @@ struct common_params {
     int image_min_tokens = -1;
     int image_max_tokens = -1;
     common_image_tile_mode image_tile_mode = COMMON_IMAGE_TILE_MODE_SEQUENTIAL;
+    int image_max_tiles = -1;  // override preproc_max_tiles from GGUF; -1 = use model default
 
     // finetune
     struct lr_opt lr;

tools/cli/README.md

@@ -165,6 +165,8 @@
 | `--image, --audio FILE` | path to an image or audio file. use with multimodal models, use comma-separated values for multiple files |
 | `--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) |
 | `--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) |
+| `--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) |
+| `--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) |
 | `--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) |
 | `--jinja, --no-jinja` | whether to use jinja template engine for chat (default: enabled)<br/>(env: LLAMA_ARG_JINJA) |
 | `--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-impl.h

@@ -17,6 +17,11 @@
 
 #define MTMD_INTERNAL_HEADER
 
+// Upper bound on preproc_max_tiles, enforced at every site that sets it (GGUF read,
+// CLI/binding override). A larger value would flow into the O(max_tiles·log max_tiles)
+// grid-fitting reserve in mtmd-image.cpp and can request hundreds of GB -> std::bad_alloc.
+constexpr int CLIP_PREPROC_MAX_TILES_LIMIT = 256;
+
 #define KEY_FTYPE               "general.file_type"
 #define KEY_NAME                "general.name"
 #define KEY_DESCRIPTION         "general.description"

tools/mtmd/clip.cpp

@@ -178,7 +178,7 @@ struct clip_ctx {
     size_t fa_mem_capacity  = 0;   // device memory used to cap the cutoff (0 = unknown)
 
     bool debug_output_embeddings = false;
-    clip_image_tile_mode tile_mode = CLIP_IMAGE_TILE_MODE_BATCHED;
+    clip_image_tile_mode tile_mode = CLIP_IMAGE_TILE_MODE_SEQUENTIAL;
 
     // When the GPU backend lacks bf16 support but the GGUF has bf16 weights,
     // we declare the in-context tensors as f16 and convert on disk-load.
@@ -1529,8 +1529,34 @@ struct clip_model_loader {
                         hparams.image_resize_algo = RESIZE_ALGO_BILINEAR;
                         get_u32(KEY_SPATIAL_MERGE_SIZE, hparams.n_merge, false);
                         get_u32(KEY_WIN_ATTN_PATTERN, hparams.n_wa_pattern, model.proj_type == PROJECTOR_TYPE_QWEN25VL); // only 2.5 requires it
-                        // optional multi-tile cap; absent in GGUF → stays 0 and the qwen3vl preprocessor falls back to 4
+                        // SigLIP2-Large ViT (n_head=16, n_layer=24) is used for 2B/4B — max_tiles=4 confirmed.
+                        // Larger ViT variants (8B+) use SigLIP2-SO-400M with unknown max_tiles.
+                        // Read from GGUF if present; otherwise default to 4 and warn for unknown ViTs.
+                        hparams.preproc_max_tiles = 4;
+                        const bool has_max_tiles_key = gguf_find_key(ctx_gguf.get(), KEY_PREPROC_MAX_TILES) >= 0;
                         get_u32(KEY_PREPROC_MAX_TILES, hparams.preproc_max_tiles, false);
+                        if (has_max_tiles_key && (hparams.preproc_max_tiles < 1 || hparams.preproc_max_tiles > CLIP_PREPROC_MAX_TILES_LIMIT)) {
+                            LOG_WRN("%s: clip.vision.preproc_max_tiles=%d out of range [1,%d] in GGUF — defaulting to 4\n",
+                                    __func__, hparams.preproc_max_tiles, CLIP_PREPROC_MAX_TILES_LIMIT);
+                            hparams.preproc_max_tiles = 4;
+                        }
+                        if (!has_max_tiles_key && (hparams.n_head > 16 || hparams.n_layer > 24)) {
+                            const char * model_name =
+                                model.proj_type == PROJECTOR_TYPE_QWEN2VL  ? "Qwen2VL"  :
+                                model.proj_type == PROJECTOR_TYPE_QWEN25VL ? "Qwen2.5VL" : "Qwen3VL";
+                            // preproc_max_tiles is only read by the Qwen3VL preprocessor; Qwen2/2.5VL
+                            // use dyn_size, so --image-max-tiles is inert there and must not be suggested.
+                            if (model.proj_type == PROJECTOR_TYPE_QWEN3VL) {
+                                LOG_WRN("%s: %s large ViT detected (n_head=%d, n_layer=%d) but "
+                                        "clip.vision.preproc_max_tiles not in GGUF — defaulting to 4. "
+                                        "If using an 8B+ model, set the correct value with --image-max-tiles.\n",
+                                        __func__, model_name, hparams.n_head, hparams.n_layer);
+                            } else {
+                                LOG_WRN("%s: %s large ViT detected (n_head=%d, n_layer=%d) but "
+                                        "clip.vision.preproc_max_tiles not in GGUF — defaulting to 4.\n",
+                                        __func__, model_name, hparams.n_head, hparams.n_layer);
+                            }
+                        }
                         // ref: https://huggingface.co/Qwen/Qwen2.5-VL-7B-Instruct/blob/main/preprocessor_config.json
                         hparams.set_limit_image_tokens(8, 4096);
                         hparams.warmup_image_size = hparams.image_size; // warmup at actual tile size to match inference graph shape
@@ -2934,6 +2960,33 @@ struct clip_init_result clip_init(const char * fname, struct clip_context_params
         if (loader.has_vision) {
             ctx_vision = new clip_ctx(ctx_params);
             loader.load_hparams(ctx_vision->model, CLIP_MODALITY_VISION);
+            // apply CLI/binding overrides after load_hparams so GGUF defaults don't clobber them.
+            // Only the Qwen3VL preprocessor reads preproc_max_tiles live; Qwen2/2.5VL use dyn_size
+            // and InternVL uses a candidate list already frozen in load_hparams, so the override is
+            // inert for them — apply (and validate) it only where it takes effect.
+            //
+            // Treat only a POSITIVE value as an explicit override. -1 is the documented unset
+            // sentinel, and 0 is what a zero-initialized clip_context_params / mtmd_context_params
+            // passes (bindings / direct C API callers that never set the field). Both are treated
+            // as unset here so those callers keep the GGUF/model default instead of silently
+            // forcing single-tile and losing multi-tile preprocessing on large/high-res images.
+            if (ctx_params.image_max_tiles > 0) {
+                if (ctx_vision->model.proj_type != PROJECTOR_TYPE_QWEN3VL) {
+                    LOG_WRN("%s: --image-max-tiles is only supported for Qwen3VL; ignoring for this model\n", __func__);
+                } else {
+                    int max_tiles = ctx_params.image_max_tiles;
+                    // Re-validate at this library boundary: bindings populate clip_context_params
+                    // directly and bypass the CLI's [1,256] check. An unbounded value reaches the
+                    // grid-fitting reserve in mtmd-image.cpp and can OOM the process.
+                    if (max_tiles > CLIP_PREPROC_MAX_TILES_LIMIT) {
+                        LOG_WRN("%s: --image-max-tiles=%d exceeds [1,%d]; clamping\n",
+                                __func__, max_tiles, CLIP_PREPROC_MAX_TILES_LIMIT);
+                        max_tiles = CLIP_PREPROC_MAX_TILES_LIMIT;
+                    }
+                    ctx_vision->model.hparams.preproc_max_tiles = max_tiles;
+                    LOG_INF("%s: preproc_max_tiles: %d (custom value)\n", __func__, max_tiles);
+                }
+            }
             loader.load_tensors(*ctx_vision);
             if (ctx_params.warmup) {
                 loader.warmup(*ctx_vision);
@@ -3624,21 +3677,27 @@ bool clip_image_batch_encode(clip_ctx * ctx, const int n_threads, const clip_ima
                 GGML_ASSERT(pw % merge_ratio == 0 && ph % merge_ratio == 0 &&
                             "tile dimensions must be divisible by n_merge");
                 const int n_pos_tile   = pw * ph; // raw patches per tile == n_patches (graph sequence length)
-                // positions layout: tile-major [tile0: y,x,y,x | tile1: y,x,y,x
-                // | ...] each tile's block starts at b * n_pos_tile * 4
-                // (matches ggml_view_1d in graph)
-                std::vector<int32_t> positions((size_t)n_pos_tile * (size_t)batch_size * 4);
+                // positions layout: section-major over the FULL batched sequence. The mrope kernel
+                // reads section s of token i as positions[s * N + i], where N = n_pos_tile * batch_size
+                // is the rope tensor's ne[2]. The four sections are [y, x, y, x]. Every tile gets the
+                // same local-coordinate block (per-tile absolute offsets cancel under relative
+                // attention), so each section just repeats the tile coordinates batch_size times.
+                // For batch_size == 1 this is identical to the old tile-major layout; for
+                // batch_size > 1 (batched mode) it is the only layout the kernel reads correctly —
+                // a tile-major buffer would mis-index every section beyond the first tile.
+                const size_t N = (size_t)n_pos_tile * (size_t)batch_size;
+                std::vector<int32_t> positions(N * 4);
                 for (int b = 0; b < batch_size; b++) {
-                    const size_t base  = (size_t)b * (size_t)n_pos_tile * 4;
+                    const size_t tile_off = (size_t)b * (size_t)n_pos_tile;
                     int ptr = 0;
                     for (int y = 0; y < ph; y += merge_ratio) {
                         for (int x = 0; x < pw; x += merge_ratio) {
                             for (int dy = 0; dy < merge_ratio; dy++) {
                                 for (int dx = 0; dx < merge_ratio; dx++) {
-                                    positions[base + ptr]                          = y + dy;
-                                    positions[base + (size_t)n_pos_tile + ptr]     = x + dx;
-                                    positions[base + (size_t)2 * n_pos_tile + ptr] = y + dy;
-                                    positions[base + (size_t)3 * n_pos_tile + ptr] = x + dx;
+                                    positions[0 * N + tile_off + ptr] = y + dy;
+                                    positions[1 * N + tile_off + ptr] = x + dx;
+                                    positions[2 * N + tile_off + ptr] = y + dy;
+                                    positions[3 * N + tile_off + ptr] = x + dx;
                                     ptr++;
                                 }
                             }

tools/mtmd/clip.h

@@ -35,9 +35,16 @@ enum clip_flash_attn_type {
     CLIP_FLASH_ATTN_TYPE_ENABLED  = 1,
 };
 
+// WARNING: value 0 is BATCHED, which is NOT the default mode (sequential is).
+// A zero-initialized clip_context_params/mtmd_context_params (`{}`, memset, calloc)
+// therefore selects BATCHED — the one-forward-pass path whose ne[3] batching is not
+// yet verified on every backend (see the NOTE in models/qwen3vl.cpp). Do not rely on
+// zero-init for the default: initialize via mtmd_context_params_default() (sets
+// sequential) or set image_tile_mode explicitly. These values are part of the shipped
+// API (string "0"/"1"/"2" in consumers), so they are intentionally not renumbered.
 enum clip_image_tile_mode {
-    CLIP_IMAGE_TILE_MODE_BATCHED    = 0,
-    CLIP_IMAGE_TILE_MODE_SEQUENTIAL = 1,
+    CLIP_IMAGE_TILE_MODE_BATCHED    = 0, // NOT the default; zero-init lands here — see warning above
+    CLIP_IMAGE_TILE_MODE_SEQUENTIAL = 1, // the default (via mtmd_context_params_default)
     CLIP_IMAGE_TILE_MODE_DISABLED   = 2,
 };
 
@@ -51,7 +58,8 @@ struct clip_context_params {
     ggml_backend_sched_eval_callback cb_eval;
     void * cb_eval_user_data;
     const char * backend_device; // optional, if null will use env var or default GPU backend
-    int image_tile_mode; // 0=batched (default), 1=sequential, 2=disabled
+    int image_tile_mode;   // 0=batched, 1=sequential (default), 2=disabled. NOTE: 0 (batched) is the zero value but NOT the default — init via mtmd_context_params_default() or set explicitly.
+    int image_max_tiles;   // override preproc_max_tiles; -1 or 0 = use GGUF/model default (only a positive value overrides)
 };
 
 struct clip_init_result {

tools/mtmd/debug/mtmd-debug.cpp

@@ -88,6 +88,8 @@ int main(int argc, char ** argv) {
         mparams.warmup           = params.warmup;
         mparams.image_min_tokens = params.image_min_tokens;
         mparams.image_max_tokens = params.image_max_tokens;
+        mparams.image_tile_mode  = (int) params.image_tile_mode;
+        mparams.image_max_tiles  = params.image_max_tiles;
         {
             // always enable debug callback
             mparams.cb_eval_user_data = &cb_data;

tools/mtmd/models/qwen3vl.cpp

@@ -9,9 +9,9 @@ ggml_cgraph * clip_graph_qwen3vl::build() {
     // QKV/FFN run fully batched; attention is per-tile (each tile attends only to its own tokens).
     // M-RoPE offsets are mathematically inert in the vision encoder: relative attention cancels
     // any per-tile absolute offset. Tile arrangement reaches the LM via decoder positions in mtmd.cpp.
-    const int n_pos            = n_patches;         // patches per tile
-    const int n_pos_total      = n_pos * batch_size; // total sequence length (all tiles)
-    const int num_position_ids = n_pos_total * 4;   // M-RoPE: 4 coords per patch
+    const int     n_pos            = n_patches;                        // patches per tile
+    const int64_t n_pos_total      = (int64_t)n_pos * batch_size;      // total sequence length (all tiles)
+    const int64_t num_position_ids = n_pos_total * 4;                  // M-RoPE: 4 coords per patch
 
     norm_type norm_t = NORM_TYPE_NORMAL;
 
@@ -120,36 +120,37 @@ ggml_cgraph * clip_graph_qwen3vl::build() {
             cb(Kcur, "Kcur", il);
             cb(Vcur, "Vcur", il);
 
-            // Per-tile attention: each tile attends only to its own n_pos tokens.
-            // QKV/FFN linear layers above are fully batched and run in parallel.
-            ggml_tensor * attn_out = ggml_new_tensor_2d(ctx0, GGML_TYPE_F32, n_embd, n_pos * batch_size);
-            for (int b = 0; b < batch_size; b++) {
-                ggml_tensor * Q_b = ggml_view_3d(ctx0, Qcur, d_head, n_head, n_pos,
-                    Qcur->nb[1], Qcur->nb[2], (size_t)b * Qcur->nb[3]);
-                ggml_tensor * K_b = ggml_view_3d(ctx0, Kcur, d_head, n_head, n_pos,
-                    Kcur->nb[1], Kcur->nb[2], (size_t)b * Kcur->nb[3]);
-                ggml_tensor * V_b = ggml_view_3d(ctx0, Vcur, d_head, n_head, n_pos,
-                    Vcur->nb[1], Vcur->nb[2], (size_t)b * Vcur->nb[3]);
-
-                ggml_tensor * pos_b = ggml_view_1d(ctx0, positions, n_pos * 4,
-                    (size_t)b * n_pos * 4 * sizeof(int32_t));
-
-                Q_b = ggml_rope_multi(ctx0, Q_b, pos_b, nullptr,
-                    d_head/2, mrope_sections, GGML_ROPE_TYPE_VISION, 32768, 10000, 1, 0, 1, 32, 1);
-                K_b = ggml_rope_multi(ctx0, K_b, pos_b, nullptr,
-                    d_head/2, mrope_sections, GGML_ROPE_TYPE_VISION, 32768, 10000, 1, 0, 1, 32, 1);
-
-                cb(Q_b, "Qcur_rope", il);
-                cb(K_b, "Kcur_rope", il);
-
-                ggml_tensor * out_b = build_attn(layer.o_w, layer.o_b,
-                    Q_b, K_b, V_b, nullptr, kq_scale, il);
-
-                attn_out = ggml_set_2d(ctx0, attn_out,
-                    ggml_reshape_2d(ctx0, out_b, n_embd, n_pos),
-                    n_embd * sizeof(float),
-                    (size_t)b * n_embd * n_pos * sizeof(float));
-            }
+            // Batched block-diagonal attention: each tile attends only to its own n_pos tokens.
+            // nb[3] = nb[2] * n_pos on the 4D views, so tiles are contiguous in sequence space.
+            // Create zero-copy 3D non-contiguous views [d_head, n_head, n_pos*batch_size] with
+            // stride nb[2] between positions — same stride the old per-tile loop used per tile.
+            // rope assert: ne[2]*4 == positions.ne[0] → n_pos*batch_size*4 == num_position_ids ✅
+            // After rope, rebuild 4D via ggml_view_4d (no copy). ggml_mul_mat in build_attn
+            // treats ne[3] as batch dim, computing an independent n_pos×n_pos attention per tile.
+            // NOTE: verify on Metal, Vulkan (Android), and OpenCL that ggml_mul_mat correctly
+            // iterates ne[3] for both KQ and KQV products before merging.
+            ggml_tensor * Q3 = ggml_view_3d(ctx0, Qcur, d_head, n_head, n_pos * batch_size,
+                Qcur->nb[1], Qcur->nb[2], 0);
+            ggml_tensor * K3 = ggml_view_3d(ctx0, Kcur, d_head, n_head, n_pos * batch_size,
+                Kcur->nb[1], Kcur->nb[2], 0);
+
+            Q3 = ggml_rope_multi(ctx0, Q3, positions, nullptr,
+                d_head/2, mrope_sections, GGML_ROPE_TYPE_VISION, 32768, 10000, 1, 0, 1, 32, 1);
+            K3 = ggml_rope_multi(ctx0, K3, positions, nullptr,
+                d_head/2, mrope_sections, GGML_ROPE_TYPE_VISION, 32768, 10000, 1, 0, 1, 32, 1);
+
+            // Rebuild 4D view from rope output (same non-contiguous stride layout as Qcur/Kcur)
+            ggml_tensor * Q4 = ggml_view_4d(ctx0, Q3, d_head, n_head, n_pos, batch_size,
+                Q3->nb[1], Q3->nb[2], Q3->nb[2] * n_pos, 0);
+            ggml_tensor * K4 = ggml_view_4d(ctx0, K3, d_head, n_head, n_pos, batch_size,
+                K3->nb[1], K3->nb[2], K3->nb[2] * n_pos, 0);
+
+            cb(Q4, "Qcur_rope", il);
+            cb(K4, "Kcur_rope", il);
+
+            // build_attn output: [n_embd, n_pos * batch_size]
+            ggml_tensor * attn_out = build_attn(layer.o_w, layer.o_b,
+                Q4, K4, Vcur, nullptr, kq_scale, il);
             cb(attn_out, "attn_out", il);
 
             // [n_embd, n_pos * batch_size] → [n_embd, n_pos, batch_size]