Skip to content

Commit fcce509

Browse files
author
Rita Law
committed
fix(mtmd/qwen3vl): collapse batched attention loop + sequential default
Replace the per-tile attention loop (a ggml_new_tensor_2d accumulator with ggml_set_2d writes and per-tile ggml_view_1d position slices, one rope pair per tile per layer) with zero-copy 3D views over the batched QKV buffer: a single rope pair runs on the collapsed n_pos*batch_size sequence, then 4D views feed one block-diagonal build_attn per layer. Fewer graph nodes and tensor descriptors, scaling with n_layer instead of n_layer*batch_size. Fix the M-RoPE position layout for batch_size>1: the mrope kernel reads section s of token i at positions[s*N + i] (N = n_pos*batch_size), so the buffer must be section-major across the full batched sequence, not tile- major. The old layout mis-indexed every section beyond the first tile, encoding (y,y) instead of (y,x). batch_size==1 (the sequential path) is unchanged. Default --image-tile-mode to sequential.
1 parent fe37515 commit fcce509

7 files changed

Lines changed: 60 additions & 47 deletions

File tree

tools/mtmd/clip.cpp

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ struct clip_ctx {
166166
bool is_allocated = false;
167167

168168
bool debug_output_embeddings = false;
169-
clip_image_tile_mode tile_mode = CLIP_IMAGE_TILE_MODE_BATCHED;
169+
clip_image_tile_mode tile_mode = CLIP_IMAGE_TILE_MODE_SEQUENTIAL;
170170

171171
// When the GPU backend lacks bf16 support but the GGUF has bf16 weights,
172172
// we declare the in-context tensors as f16 and convert on disk-load.
@@ -3417,21 +3417,27 @@ bool clip_image_batch_encode(clip_ctx * ctx, const int n_threads, const clip_ima
34173417
GGML_ASSERT(pw % merge_ratio == 0 && ph % merge_ratio == 0 &&
34183418
"tile dimensions must be divisible by n_merge");
34193419
const int n_pos_tile = pw * ph; // raw patches per tile == n_patches (graph sequence length)
3420-
// positions layout: tile-major [tile0: y,x,y,x | tile1: y,x,y,x
3421-
// | ...] each tile's block starts at b * n_pos_tile * 4
3422-
// (matches ggml_view_1d in graph)
3423-
std::vector<int32_t> positions((size_t)n_pos_tile * (size_t)batch_size * 4);
3420+
// positions layout: section-major over the FULL batched sequence. The mrope kernel
3421+
// reads section s of token i as positions[s * N + i], where N = n_pos_tile * batch_size
3422+
// is the rope tensor's ne[2]. The four sections are [y, x, y, x]. Every tile gets the
3423+
// same local-coordinate block (per-tile absolute offsets cancel under relative
3424+
// attention), so each section just repeats the tile coordinates batch_size times.
3425+
// For batch_size == 1 this is identical to the old tile-major layout; for
3426+
// batch_size > 1 (batched mode) it is the only layout the kernel reads correctly —
3427+
// a tile-major buffer would mis-index every section beyond the first tile.
3428+
const size_t N = (size_t)n_pos_tile * (size_t)batch_size;
3429+
std::vector<int32_t> positions(N * 4);
34243430
for (int b = 0; b < batch_size; b++) {
3425-
const size_t base = (size_t)b * (size_t)n_pos_tile * 4;
3431+
const size_t tile_off = (size_t)b * (size_t)n_pos_tile;
34263432
int ptr = 0;
34273433
for (int y = 0; y < ph; y += merge_ratio) {
34283434
for (int x = 0; x < pw; x += merge_ratio) {
34293435
for (int dy = 0; dy < merge_ratio; dy++) {
34303436
for (int dx = 0; dx < merge_ratio; dx++) {
3431-
positions[base + ptr] = y + dy;
3432-
positions[base + (size_t)n_pos_tile + ptr] = x + dx;
3433-
positions[base + (size_t)2 * n_pos_tile + ptr] = y + dy;
3434-
positions[base + (size_t)3 * n_pos_tile + ptr] = x + dx;
3437+
positions[0 * N + tile_off + ptr] = y + dy;
3438+
positions[1 * N + tile_off + ptr] = x + dx;
3439+
positions[2 * N + tile_off + ptr] = y + dy;
3440+
positions[3 * N + tile_off + ptr] = x + dx;
34353441
ptr++;
34363442
}
34373443
}

tools/mtmd/clip.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ 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, 1=sequential (default), 2=disabled
5555
int image_max_tiles; // override preproc_max_tiles; -1 = use GGUF/model default
5656
};
5757

tools/mtmd/debug/mtmd-debug.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,8 @@ int main(int argc, char ** argv) {
8888
mparams.warmup = params.warmup;
8989
mparams.image_min_tokens = params.image_min_tokens;
9090
mparams.image_max_tokens = params.image_max_tokens;
91+
mparams.image_tile_mode = (int) params.image_tile_mode;
92+
mparams.image_max_tiles = params.image_max_tiles;
9193
{
9294
// always enable debug callback
9395
mparams.cb_eval_user_data = &cb_data;

tools/mtmd/models/qwen3vl.cpp

Lines changed: 34 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,9 @@ ggml_cgraph * clip_graph_qwen3vl::build() {
99
// QKV/FFN run fully batched; attention is per-tile (each tile attends only to its own tokens).
1010
// M-RoPE offsets are mathematically inert in the vision encoder: relative attention cancels
1111
// any per-tile absolute offset. Tile arrangement reaches the LM via decoder positions in mtmd.cpp.
12-
const int n_pos = n_patches; // patches per tile
13-
const int n_pos_total = n_pos * batch_size; // total sequence length (all tiles)
14-
const int num_position_ids = n_pos_total * 4; // M-RoPE: 4 coords per patch
12+
const int n_pos = n_patches; // patches per tile
13+
const int64_t n_pos_total = (int64_t)n_pos * batch_size; // total sequence length (all tiles)
14+
const int64_t num_position_ids = n_pos_total * 4; // M-RoPE: 4 coords per patch
1515

1616
norm_type norm_t = NORM_TYPE_NORMAL;
1717

@@ -120,36 +120,37 @@ ggml_cgraph * clip_graph_qwen3vl::build() {
120120
cb(Kcur, "Kcur", il);
121121
cb(Vcur, "Vcur", il);
122122

123-
// Per-tile attention: each tile attends only to its own n_pos tokens.
124-
// QKV/FFN linear layers above are fully batched and run in parallel.
125-
ggml_tensor * attn_out = ggml_new_tensor_2d(ctx0, GGML_TYPE_F32, n_embd, n_pos * batch_size);
126-
for (int b = 0; b < batch_size; b++) {
127-
ggml_tensor * Q_b = ggml_view_3d(ctx0, Qcur, d_head, n_head, n_pos,
128-
Qcur->nb[1], Qcur->nb[2], (size_t)b * Qcur->nb[3]);
129-
ggml_tensor * K_b = ggml_view_3d(ctx0, Kcur, d_head, n_head, n_pos,
130-
Kcur->nb[1], Kcur->nb[2], (size_t)b * Kcur->nb[3]);
131-
ggml_tensor * V_b = ggml_view_3d(ctx0, Vcur, d_head, n_head, n_pos,
132-
Vcur->nb[1], Vcur->nb[2], (size_t)b * Vcur->nb[3]);
133-
134-
ggml_tensor * pos_b = ggml_view_1d(ctx0, positions, n_pos * 4,
135-
(size_t)b * n_pos * 4 * sizeof(int32_t));
136-
137-
Q_b = ggml_rope_multi(ctx0, Q_b, pos_b, nullptr,
138-
d_head/2, mrope_sections, GGML_ROPE_TYPE_VISION, 32768, 10000, 1, 0, 1, 32, 1);
139-
K_b = ggml_rope_multi(ctx0, K_b, pos_b, nullptr,
140-
d_head/2, mrope_sections, GGML_ROPE_TYPE_VISION, 32768, 10000, 1, 0, 1, 32, 1);
141-
142-
cb(Q_b, "Qcur_rope", il);
143-
cb(K_b, "Kcur_rope", il);
144-
145-
ggml_tensor * out_b = build_attn(layer.o_w, layer.o_b,
146-
Q_b, K_b, V_b, nullptr, kq_scale, il);
147-
148-
attn_out = ggml_set_2d(ctx0, attn_out,
149-
ggml_reshape_2d(ctx0, out_b, n_embd, n_pos),
150-
n_embd * sizeof(float),
151-
(size_t)b * n_embd * n_pos * sizeof(float));
152-
}
123+
// Batched block-diagonal attention: each tile attends only to its own n_pos tokens.
124+
// nb[3] = nb[2] * n_pos on the 4D views, so tiles are contiguous in sequence space.
125+
// Create zero-copy 3D non-contiguous views [d_head, n_head, n_pos*batch_size] with
126+
// stride nb[2] between positions — same stride the old per-tile loop used per tile.
127+
// rope assert: ne[2]*4 == positions.ne[0] → n_pos*batch_size*4 == num_position_ids ✅
128+
// After rope, rebuild 4D via ggml_view_4d (no copy). ggml_mul_mat in build_attn
129+
// treats ne[3] as batch dim, computing an independent n_pos×n_pos attention per tile.
130+
// NOTE: verify on Metal, Vulkan (Android), and OpenCL that ggml_mul_mat correctly
131+
// iterates ne[3] for both KQ and KQV products before merging.
132+
ggml_tensor * Q3 = ggml_view_3d(ctx0, Qcur, d_head, n_head, n_pos * batch_size,
133+
Qcur->nb[1], Qcur->nb[2], 0);
134+
ggml_tensor * K3 = ggml_view_3d(ctx0, Kcur, d_head, n_head, n_pos * batch_size,
135+
Kcur->nb[1], Kcur->nb[2], 0);
136+
137+
Q3 = ggml_rope_multi(ctx0, Q3, positions, nullptr,
138+
d_head/2, mrope_sections, GGML_ROPE_TYPE_VISION, 32768, 10000, 1, 0, 1, 32, 1);
139+
K3 = ggml_rope_multi(ctx0, K3, positions, nullptr,
140+
d_head/2, mrope_sections, GGML_ROPE_TYPE_VISION, 32768, 10000, 1, 0, 1, 32, 1);
141+
142+
// Rebuild 4D view from rope output (same non-contiguous stride layout as Qcur/Kcur)
143+
ggml_tensor * Q4 = ggml_view_4d(ctx0, Q3, d_head, n_head, n_pos, batch_size,
144+
Q3->nb[1], Q3->nb[2], Q3->nb[2] * n_pos, 0);
145+
ggml_tensor * K4 = ggml_view_4d(ctx0, K3, d_head, n_head, n_pos, batch_size,
146+
K3->nb[1], K3->nb[2], K3->nb[2] * n_pos, 0);
147+
148+
cb(Q4, "Qcur_rope", il);
149+
cb(K4, "Kcur_rope", il);
150+
151+
// build_attn output: [n_embd, n_pos * batch_size]
152+
ggml_tensor * attn_out = build_attn(layer.o_w, layer.o_b,
153+
Q4, K4, Vcur, nullptr, kq_scale, il);
153154
cb(attn_out, "attn_out", il);
154155

155156
// [n_embd, n_pos * batch_size] → [n_embd, n_pos, batch_size]

tools/mtmd/mtmd-cli.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,8 @@ struct mtmd_cli_context {
141141
mparams.n_threads = params.cpuparams.n_threads;
142142
mparams.flash_attn_type = params.flash_attn_type;
143143
mparams.warmup = params.warmup;
144-
mparams.image_min_tokens = params.image_min_tokens;
145-
mparams.image_max_tokens = params.image_max_tokens;
144+
mparams.image_min_tokens = params.image_min_tokens;
145+
mparams.image_max_tokens = params.image_max_tokens;
146146
mparams.image_tile_mode = (int)params.image_tile_mode;
147147
mparams.image_max_tiles = params.image_max_tiles;
148148
if (std::getenv("MTMD_DEBUG_GRAPH") != nullptr) {

tools/mtmd/mtmd.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -615,6 +615,8 @@ struct mtmd_caps mtmd_get_cap_from_file(const char * fname) {
615615
cp.cb_eval = nullptr;
616616
cp.cb_eval_user_data= nullptr;
617617
cp.backend_device = nullptr;
618+
cp.image_tile_mode = CLIP_IMAGE_TILE_MODE_SEQUENTIAL;
619+
cp.image_max_tiles = -1; // -1 = use model default
618620
clip_init_result init = clip_init(fname, cp);
619621
if (init.ctx_v != nullptr) {
620622
cap.inp_vision = clip_has_vision_encoder(init.ctx_v);
@@ -649,6 +651,8 @@ std::map<ggml_backend_dev_t, size_t> mtmd_get_memory_usage(
649651
cp.cb_eval = nullptr;
650652
cp.cb_eval_user_data= nullptr;
651653
cp.backend_device = nullptr;
654+
cp.image_tile_mode = ctx_params.image_tile_mode;
655+
cp.image_max_tiles = ctx_params.image_max_tiles;
652656
clip_init_result init = clip_init(mmproj_fname, cp);
653657
auto merge = [&](struct clip_ctx * c) {
654658
if (c == nullptr) return;

tools/mtmd/mtmd.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ struct mtmd_context_params {
100100

101101
const char * backend_device; // optional GPU backend name (e.g. "CUDA", "Metal", "Vulkan"), if null will use env var or default
102102

103-
// tile encoding mode for multi-tile vision models (Qwen3VL): 0=batched (default), 1=sequential, 2=disabled
103+
// tile encoding mode for multi-tile vision models (Qwen3VL): 0=batched, 1=sequential (default), 2=disabled
104104
int image_tile_mode;
105105

106106
// override preproc_max_tiles from GGUF; -1 = use model default (4 for Qwen3VL 2B/4B)

0 commit comments

Comments
 (0)