Skip to content

mtmd: deepseek-ocr v1 multi-tile#24717

Merged
ngxson merged 13 commits into
masterfrom
xsn/mtmd_ds_ocr_tiles
Jul 10, 2026
Merged

mtmd: deepseek-ocr v1 multi-tile#24717
ngxson merged 13 commits into
masterfrom
xsn/mtmd_ds_ocr_tiles

Conversation

@ngxson

@ngxson ngxson commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator

Overview

Supersede #24647

Ref discussion: #24647 (comment)

Requirements

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sfallah I'm targeting to make this version works with only one row of tiles per batch (fused into one input image_f32: w = tile_size and h = tile_size * n_col)

then later on, we can simply flip support_batch() to true to allow multi-row batching

for now, shapes are matching but this output gibberish, appreciate if you can help debugging which part is wrong. I added extensive debugging logs, but they will be removed once we found the problem

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ngxson
I will start debugging it right away!

@github-actions github-actions Bot added examples python python script changes labels Jun 17, 2026
@sfallah

sfallah commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Disclaimer: the debugging was done by AI, and this comment is also AI-generated.

Two bugs:

1. Gibberishn_tokens() returns nx*ny*nz (nz = entries.size()), but DSOCR sets nx to the total over all entries, so it over-allocates (493→1479 for (1,2)) and feeds the uninitialized tail to the decoder. Single-view escapes it (nz=1). That nz multiply (#24656) is for MROPE where nx is per-frame — gate it:

if (pos != MTMD_POS_TYPE_MROPE) {
    return nx * ny;  // nx already holds the total
}

2. v2 multi-column crash — the shared row-fusion makes a tall image that only the v1 graph re-splits; v2 feeds it straight to SAM and trips the get_rel_pos assert. v2 has no in-graph weave, so keep it on separate tiles:

const bool fuse_row = clip_get_projector_type(ctx) == PROJECTOR_TYPE_DEEPSEEKOCR;

Verified bf16 / flash-off — v1 & v2 both pass single / (1,2) / (2,3); tests.sh big 42/42. Pushed both fixes + a (2,3) regression case + debug-log removal to sfallah:sf/pr24717-fixescompare.

@sfallah

sfallah commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

hi @ngxson
do you want me to finish this PR and make it ready?
Or you will continue with it yourself.

@ngxson

ngxson commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks for the investigation. The problem with n_tokens() counting is fixed with a different approach, see: #24732

I updated the preprocessor code as you suggested and it's working correctly.

Can you confirm if this passes the tests now?

Test results on my side:

[vision] OK:   ggml-org/SmolVLM-500M-Instruct-GGUF:Q8_0
[vision] OK:   ggml-org/SmolVLM2-2.2B-Instruct-GGUF:Q4_K_M
[vision] OK:   ggml-org/SmolVLM2-500M-Video-Instruct-GGUF:Q8_0
[vision] OK:   ggml-org/gemma-3-4b-it-GGUF:Q4_K_M
[vision] OK:   THUDM/glm-edge-v-5b-gguf:Q4_K_M
[vision] OK:   second-state/Llava-v1.5-7B-GGUF:Q2_K
[vision] OK:   cjpais/llava-1.6-mistral-7b-gguf:Q3_K_M
[vision] OK:   ibm-research/granite-vision-3.2-2b-GGUF:Q4_K_M
[vision] OK:   second-state/MiniCPM-Llama3-V-2_5-GGUF:Q2_K
[vision] OK:   openbmb/MiniCPM-V-2_6-gguf:Q2_K
[vision] OK:   openbmb/MiniCPM-o-2_6-gguf:Q4_0
[vision] OK:   bartowski/Qwen2-VL-2B-Instruct-GGUF:Q4_K_M
[vision] OK:   ggml-org/Qwen2.5-VL-3B-Instruct-GGUF:Q4_K_M
[vision] OK:   ggml-org/InternVL2_5-1B-GGUF:Q8_0
[vision] OK:   ggml-org/InternVL3-1B-Instruct-GGUF:Q8_0
[vision] OK:   ggml-org/Qwen2.5-Omni-3B-GGUF:Q4_K_M
[vision] OK:   ggml-org/LFM2-VL-450M-GGUF:Q8_0
[vision] OK:   ggml-org/granite-docling-258M-GGUF:Q8_0
[vision] OK:   ggml-org/LightOnOCR-1B-1025-GGUF:Q8_0
[vision] OK:   ggml-org/DeepSeek-OCR-GGUF:Q8_0
[vision] OK:   ggml-org/dots.ocr-GGUF:Q8_0
[vision] OK:   ggml-org/HunyuanOCR-GGUF:Q8_0
[vision] OK:   ggml-org/gemma-4-E2B-it-GGUF:Q8_0
[audio]  OK:   ggml-org/ultravox-v0_5-llama-3_2-1b-GGUF:Q8_0
[audio]  OK:   ggml-org/Qwen2.5-Omni-3B-GGUF:Q4_K_M
[audio]  OK:   ggml-org/Voxtral-Mini-3B-2507-GGUF:Q4_K_M
[audio]  OK:   ggml-org/LFM2-Audio-1.5B-GGUF:Q8_0
[audio]  OK:   ggml-org/gemma-4-E2B-it-GGUF:Q8_0
[audio]  OK:   ggml-org/Qwen3-ASR-0.6B-GGUF:Q8_0

@ngxson ngxson marked this pull request as ready for review June 18, 2026 14:36
@ngxson ngxson requested a review from a team as a code owner June 18, 2026 14:36
@sfallah

sfallah commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

@ngxson

no, sadly something is wrong.
There is a strong regression on single-view test.
FYI: I actually was on this point too after merging master in my fix branch.

============================================================
Free OCR evaluation:
============================================================
CER 0.3594 (HF 0.3030, <= 0.3230 -> FAIL)
chrF (0-100) 60.56 (HF 67.52, >= 65.52 -> FAIL)
Expected chars 1188
Aligned chars 994 (of 1194 OCR chars)

Result: FAIL

The CER score on master 0.2256.
I will debug it.

@ngxson

ngxson commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator Author

hmm ok I spot some problems regarding handling overview image for all llava-uhd models

I'm pushing a refactoring on that part, will tag you for for testing

@ngxson

ngxson commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator Author

@sfallah Ok so I narrowed down the potentially regressions, it seems like there are two potential problems:

  1. The norm eps may not be correct
  2. For v1, the 1280x1280 resolution is gone

I verified that the small input image (tools/mtmd/test-1.jpeg) is preprocessed to a single overview-only, no tiles, so at this point I'm pretty confident that none of my additional logic cause the regression.

Can you isolate your changes and try to find the root cause?

@sfallah

sfallah commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

@ngxson

I verified that the small input image (tools/mtmd/test-1.jpeg) is preprocessed to a single overview-only, no tiles, so at this point I'm pretty confident that none of my additional logic cause the regression.

I will take a look at it.
But on my fix branch an all the other branches (for the closed PRs) that included this logic, single-view CER scores were fix 0.2256.
Screenshot 2026-06-18 at 20 13 49

@sfallah

sfallah commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

@ngxson
despite my AI finding above.
I see also regression on CUDA now, too.
This needs more analysis, but it is difficult for me to see how the mtmd changes, from me and other changes by you, are causing this.

@sfallah

sfallah commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

@ngxson
please ignore the AI finding, above so I removed the comment.
Although reverting metal rope_back PR (#24725) works, it is not the right path.

Comment thread tools/mtmd/tests/test-deepseek-ocr.py Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"-p", "Free OCR.",

With this small change it will also pass the regression test for ds-ocr v1.

@ngxson
I had also a closer look at the output it is fine.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like it's removed on master, so should be good after my last rebase commit - please give it a try

@sfallah

sfallah commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Opened #25481 with a few fixes for this branch.

* ds-ocr img-preproc fuse_row tile-drop fix for multi rows and columns images

* mtmd drop the duplicate redundant img_end

* deepseekocr graph simplify CLS broadcast cleanup
@github-actions github-actions Bot added the mtmd Related to multimodal functionality (video/image/audio) label Jul 9, 2026
@sfallah

sfallah commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Opened #25486 with test fixes for this branch.

@ngxson ngxson merged commit 3e706dd into master Jul 10, 2026
29 of 30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

examples mtmd Related to multimodal functionality (video/image/audio) python python script changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants