mtmd: deepseek-ocr v1 multi-tile dynamic resolution + unified image-preprocessors for both versions (ds-ocr v1 and v2)#24647
Conversation
…reprocessors for both versions (ds-ocr v1 and v2)
ngxson
left a comment
There was a problem hiding this comment.
adding model-specific stitch_tile_grid in the pipeline is a no-go, move it to cgraph instead
| const int h = static_cast<int>(std::sqrt(static_cast<float>(n_patches))); | ||
| n_patches = h * (h + 1) + 1; |
There was a problem hiding this comment.
better to use img->nx() or img->ny() like other models:
img->ny() * (img->nx() + 1) * 1
| // Example, 2x2 grid of tiles A B / C D: | ||
| // raw = [ A B C D <overview> ] | ||
| // out = A.row0 B.row0 n, A.row1 B.row1 n, ..., C.row0 D.row0 n, ..., <overview> | ||
| static void stitch_tile_grid(clip_ctx * ctx, const clip_image_f32_batch & batch, |
There was a problem hiding this comment.
nope, this is too hacky, any assembling must be done on cgraph
| struct clip_image_f32 * clip_image_f32_get_img(const struct clip_image_f32_batch * batch, int idx); // equivalent to batch[idx]->data | ||
|
|
||
| // read the image-newline embedding from the backend; empty if the model has none | ||
| std::vector<float> clip_get_newline_embd(const struct clip_ctx * ctx); |
There was a problem hiding this comment.
this new API is hacky, remove it, move any assembling to cgraph as suggested
| ggml_tensor * imgnl = ggml_repeat_4d(ctx0, model.image_newline, n_dim, 1, h, 1); | ||
| cur = ggml_reshape_3d(ctx0, cur, n_dim, w, h); | ||
| cur = ggml_reshape_2d(ctx0, ggml_concat(ctx0, cur, imgnl, 1), n_dim, (w + 1) * h); | ||
| cur = ggml_concat(ctx0, cur, model.view_seperator, 1); // (n_dim, h*(w+1) + 1) |
There was a problem hiding this comment.
- view output as
[n_dim, w, h, batch] - add newline to
wdim - add viewsep
batchdim
just ~10 lines of code, no more hacky stitch_tile_grid, right?
There was a problem hiding this comment.
if img.add_viewsep is added conditionally (not all images in batch have it): I already explain in #24384 (comment) , please read it carefully
There was a problem hiding this comment.
@ngxson
Do you suggest that the tiles should be batched encoded?
Because what you suggest could only work if the tiles are encoded as a batch, which is essentially the closed PR #24300 (sf/dsocr-mul-tile-batched-encode).
That is why it is a "hack", and I said already without batch encoding the tiles, it will be a "hack".
There was a problem hiding this comment.
sorry I'm not in a good state to reply to your message now. there is a big misunderstanding from your side about how the batching API is designed for.
but in short: your other PR #24300 is designed with the assumption that batch is grid-aware:
that is not expected, and that's the main reason why your approach was rejected
That is why it is a "hack", and I said already without batch encoding the tiles, it will be a "hack".
I told you not to add batching because I guess you will hack the code again to make it fits
but tbh at this point, it's easier for me to just go ahead and fix this this. keep arguing like this is not a good use of my time
There was a problem hiding this comment.
Ok so I give an attempt today but it wasn't quite go as planned. I realized that I misunderstood the way image rows are being permuted before newline is being added. Please confirm if my assumption below is correct:
Let's say we have a row of 2 tiles A and B
- The raw ouput will be: A.row0 A.row1 B.row0 B.row1
- We firstly need to permute such that: A.row0 B.row0 A.row1 B.row1
- Then insert nl: A.row0 B.row0 nl A.row1 B.row1 nl
And one row is completely independent from the other.
If my assumption above is correct, then the problem can be viewed in a different way: the key idea of my impl to fuse a row of tiles into a single tile having w = tile_size and h = tile_size * n_col
So my impl is as follow:
- preprocessor fuses the tiles-in-a-row into a new f32 image: w = tile_size and h = tile_size * n_col
- inside cgraph, we detect if the input have h > w ; if yes, permute the fused image into n_batch dim
- process as if we have n_col square images in the batch
- permute the output rows and append nl
The advantage of this approach is that:
- The whole row is guranteed to be proceed in one go, no splitting is allowed
- Multi rows in one batch can be supported automatically without any changes to the existing batching API (assuming all input rows have the same number of tiles)
- No more manual stitching outside of cgraph
I was able to vibe-code an initial version but still something is not correct, so sharing this direction if you want to give a try. On my side, I will go back to this a different time.
There was a problem hiding this comment.
Let's say we have a row of 2 tiles A and B
- The raw ouput will be: A.row0 A.row1 B.row0 B.row1
- We firstly need to permute such that: A.row0 B.row0 A.row1 B.row1
- Then insert nl: A.row0 B.row0 nl A.row1 B.row1 nl
yes, your assumption is correct!
There was a problem hiding this comment.
Your suggested solution will work. But tbh, I consider it a "hack" — the only difference is that we'd be hiding the hack inside the preprocessor.
If you're really interested in having a clean API, why don't we discuss what I already suggested and demonstrated in #24300? And please don't dismiss it as a model-specific quirk/hack, because what looked like a hack at first sight can be generalized to all multi-tile models.
Why don't we consider using batch processing for all multi-tile models? This would be an extension to the API — an extension to the standard path, which currently processes tiles iteratively. DS-OCR (v1 + v2) would be the first model to use that path, and I'll help migrate the other multi-tile models.
Before dismissing my ideas, as you have unfortunately done several times, please contemplate them for a moment. These are my thoughts on the matter:
-
All of llama.cpp's contenders — MLX-VLM, sglang, vLLM (I assume), Transformers — encode the tiles as a single batch and don't go through the pain we do. They don't even care about the memory overhead.
-
I'm not saying we shouldn't care about memory overhead. The fact that we're so concerned with it is a big PLUS for the llama.cpp implementation, and limiting memory overhead should remain a priority. For DSOCR and many other multi-tile models, there's an upper bound (max-tiles) anyway. I've already been considering the issues around memory overhead and how to limit it properly.
Last but not least, I want to say that DSOCR — especially v1 — is worth the effort. Compare its output against all the others on the hard test (test-1.jpeg) we have in mtmd.
One more thing: I'm not paid for this, and I'm not doing it for my CV or to get hired by some big-name AI company. I just want to finish what I started, properly. DSOCR v1 and the llama.cpp community deserve that.
There was a problem hiding this comment.
Depending your point of view, if you compare mtmd and even libllama to another python-based framework, then the whole libmtmd is indeed bunch of hacks linked together to make it works, for example: other frameworks stores multimodal encoder and text model in the same model file, runs on the same context, share the same cgraph, etc.
My suggestion, despite sounds a bit hacky at first, is better because:
- Only DS-OCR v1 have the problem where you need to permute the embedding rows after encoding pass. The most important part is any quirks for DS-OCR v1 must be confined in model's cgraph and its preprocessor, adding a dedicated method is not expected
- It allows DS-OCR to reuse the llava-uhd batching infrastructure --> you seem to misunderstand this part, I'll explain below
Why don't we consider using batch processing for all multi-tile models? This would be an extension to the API
Yes we do, I don't yet enable it but the new batcing API is designed for this exact purpose.
Here is a concrete example:
- I have an input image that will be broken into one overview and a grid of 3x4 = 12 tiles --> in total: 1 (overview) + 12 (tiles) = 13 images input
- Given mtmd is configured to hold maximum 5 images at once, it will be broken down into 3 batches: 5 + 5 + 3
The logic above is already implemented, just not enabled yet, and it should work with ALL llava-uhd models, EXCEPT for DS-OCR which has these quirks:
- Overview image has different size than tiles --> batch API knows to exclude it from the batch, as we only accepts images with the same size in a batch
- For v1, it requires permute before adding newline --> this is why I refer to your impl as "hacky", it assumes that DS-OCR is the "standard" case and other models must be based on DS-OCR model, while the fact is that DS-OCR is a special cae of llava-uhd --> any extra processing must be confined to its preprocessor and cgraph
There was a problem hiding this comment.
- All of llama.cpp's contenders — MLX-VLM, sglang, vLLM (I assume), Transformers — encode the tiles as a single batch and don't go through the pain we do. They don't even care about the memory overhead.
I'm not sure what part you refers as "pain", but mtmd batch "scheduler" handles this automatically. If you want the whole batch to be process in one go, set the --mtmd-batch-max-tokens to a higher number. Otherwise, with less memory, the scheduler knows to only process at max N images in a batch.
AFAIK transformers encode not only process image per batch, but it processes ALL prompt images at once. This makes the case of DS-OCR easy, but is not a very comfortable choice for video input because the memory usage blows up quickly.
|
@ngxson |
Overview
deepseek-ocr v1 multi-tile dynamic resolution with host-side (not in-graph)
image_newlineweaving.Requirements