Skip to content

fix: Fix Granite4 Vision regression based on n_tokens counting #24733

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
gabe-l-hart wants to merge 1 commit into from
Closed

fix: Fix Granite4 Vision regression based on n_tokens counting #24733

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions tools/mtmd/mtmd.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,10 @@ struct mtmd_bitmap {

// position indexing for decoder model
enum mtmd_pos_type {
MTMD_POS_TYPE_NORMAL, // number of positions equals to number of tokens
MTMD_POS_TYPE_MROPE, // qwen-vl mrope style, each image takes max(t,h,w) position indexes
MTMD_POS_TYPE_HUNYUANVL, // HunyuanVL mrope + BOI/EOI/newline layout with XD-RoPE dim-3
MTMD_POS_TYPE_NORMAL, // number of positions equals to number of tokens
MTMD_POS_TYPE_MROPE, // qwen-vl mrope style, each image takes max(t,h,w) position indexes
MTMD_POS_TYPE_HUNYUANVL, // HunyuanVL mrope + BOI/EOI/newline layout with XD-RoPE dim-3
MTMD_POS_TYPE_LLAVA_NEXT, // Llava-next: image grid acts as a batch w/out text delimiters
};

struct mtmd_image_tokens {
Expand All @@ -96,6 +97,10 @@ struct mtmd_image_tokens {
// [BOI] [row0 tokens + newline] ... [row(ny-1) tokens + newline] [EOI]
return (nx + 1) * ny + 2;
}
if (pos == MTMD_POS_TYPE_LLAVA_NEXT) {
// Batch dim is the tiles in the grid, so doesn't contribute to the final n_tokens
return nx * ny;
}
uint32_t nz = batch_f32.entries.size();
if (n_temporal_merge > 1) {
// [QWEN_VIDEO] this logic is quite ugly, it's mostly to make qwen-vl temporal merge work, can be improved in the future
Expand Down Expand Up @@ -1200,6 +1205,10 @@ struct mtmd_tokenizer {
image_tokens->image_idx = n_images_added;
GGML_ASSERT(n_tokens == (size_t)image_tokens->n_tokens());
}
// Llava-next style (Granite4Vision) uses a batch of tiles in a single mtmd_image_tokens
if (ctx->proj_type_v() == PROJECTOR_TYPE_GRANITE4_VISION) {
image_tokens->pos = MTMD_POS_TYPE_LLAVA_NEXT;
}
image_tokens->batch_f32 = std::move(batch_f32);
image_tokens->id = bitmaps[0]->id; // optional

Expand Down Expand Up @@ -1901,6 +1910,7 @@ mtmd_decoder_pos mtmd_image_tokens_get_decoder_pos(const mtmd_image_tokens * ima
pos.z = 0; // unused for now
} break;
case MTMD_POS_TYPE_NORMAL:
case MTMD_POS_TYPE_LLAVA_NEXT:
{
pos.t = pos_0 + i;
pos.x = pos_0 + i;
Expand Down Expand Up @@ -1954,6 +1964,7 @@ llama_pos mtmd_image_tokens_get_n_pos(const mtmd_image_tokens * image_tokens) {
case MTMD_POS_TYPE_MROPE:
return std::max(image_tokens->nx, image_tokens->ny);
case MTMD_POS_TYPE_NORMAL:
case MTMD_POS_TYPE_LLAVA_NEXT:
return image_tokens->n_tokens();
case MTMD_POS_TYPE_HUNYUANVL:
// HunyuanVL: the sequential (dim-0) position advances by the full token count
Expand Down
Loading