lablup / lablup/mlxcel

fix(youtu_vl): processor ignores preprocessor max_num_patches

Open
#1,611 0 comments 0 reactions 0 assignees View on GitHub
area:models modeltype:vlm priority:medium status:ready type:bug
Dominant language
Rust
Stars
467
Forks
54
Avg merge
4h 25m
Merged PRs (30d)
310

Description

## Problem / Background

`build_processor` in `src/loading/vlm_youtu_vl.rs` caps the Youtu-VL processor with `vision_config.num_patches`:

```rust
let mut processor = YoutuVLProcessor::new(vision_config.patch_size, vision_config.spatial_merge_size)
.with_max_patches_per_image(vision_config.num_patches);
```

For `tencent/Youtu-VL-4B-Instruct` that is 4096. It then reads `preprocessor_config.json` for `image_mean`, `image_std` and pixel hints, but never for `max_num_patches`, which that file sets to 256.

`num_patches` and `max_num_patches` are not the same quantity. In `modeling_siglip2.py`, `config.num_patches` sizes the learned position-embedding table (`nn.Embedding(self.num_patches, self.embed_dim)`, then bilinearly resized to each image's spatial shape), so it is a capacity bound. `max_num_patches` in `image_processing_siglip2_fast.py` is what `get_image_size_for_patches` actually scales each image down to, so it is the operating point. mlxcel uses the capacity as the operating point and so runs the tower at up to 16 times the patch count the checkpoint's own processor would ever produce.

## Evidence

With `patch_size` 16 and `spatial_merge_size` 2, `effective_max_pixels` becomes `min(16384 * 28 * 28, 4096 * 256)` = 1,048,576 pixels, so mlxcel resizes any large image to 1024x1024 for a 64x64 patch grid. The upstream processor scales the same image until `(h/16) * (w/16) <= 256`, that is to 256x256 for a 16x16 patch grid.

| image | mlxcel resized | mlxcel patch grid | upstream resized | upstream patch grid |
|---|---|---|---|---|
| 448x448 | 448x448 | 28x28 | 256x256 | 16x16 |
| 512x512 | 512x512 | 32x32 | 256x256 | 16x16 |
| 2048x2048 | 1024x1024 | 64x64 | 256x256 | 16x16 |

Two consequences worth separating.

**Parity.** No mlxcel Youtu-VL run can be compared against a transformers reference on the same image, because the two stages see different patch grids before any numerics are involved. That blocks the stage-by-stage reference diff this repository uses to validate VLM ports.

**Reachability of other defects.** A Youtu-VL attention window is 8x8 merged tokens. Under the upstream cap the merged grid never exceeds 8x8, so the window permutation is always the identity. The window-inverse defect fixed in #1600 is only reachable at all because of this cap divergence, and the same is true of any future defect that only appears above one window. That cuts both ways: honoring `max_num_patches` would mask #1600 rather than fix it, so this issue should not be treated as an alternative to that one.

## Proposed Solution

1. Read `max_num_patches` from `preprocessor_config.json` in `build_processor`, falling back to `vision_config.num_patches` when the key is absent, and pass it to `with_max_patches_per_image`. Keep `num_patches` as the hard allocation bound it is documented to be.
2. Add a loader unit test with a synthetic `preprocessor_config.json` asserting the processor takes 256 rather than 4096, in the shape of `tiling_validation_uses_processor_max_num_patches_not_vision_table_len` in `src/loading/vlm_lfm2_vl_tests.rs`, which is the same distinction already drawn for LFM2-VL.
3. Confirm on the real checkpoint that the resulting patch grid matches what the HuggingFace processor produces for the same image.

## Scope

**In scope:** `build_processor` in `src/loading/vlm_youtu_vl.rs` and its tests.

**Out of scope:** the patch emission order, filed separately. The window inverse (#1600).

## Acceptance Criteria

- [ ] `build_processor` honors `preprocessor_config.json`'s `max_num_patches` and falls back to `vision_config.num_patches` when it is missing.
- [ ] A loader unit test pins that precedence.
- [ ] The patch grid for a real image matches the HuggingFace processor's for the same image.
- [ ] `cargo test --workspace --profile test-fast --features metal,accelerate`, `cargo clippy --workspace --all-targets --features metal,accelerate -- -D warnings`, and `cargo fmt --all -- --check` are green.

## Technical Considerations

`src/loading/vlm_lfm2_vl.rs` already separates these two quantities for another SigLIP-derived tower and is the precedent to follow.

Contributor guide

Open the contributing guide

Research direction

Start with build_processor in src/loading/vlm_youtu_vl.rs and compare its configuration handling with src/loading/vlm_lfm2_vl.rs. Follow the test shape in src/loading/vlm_lfm2_vl_tests.rs, then run the loader tests and compare a real image's patch grid with the HuggingFace processor. Done means max_num_patches takes precedence, the fallback works, and the listed Rust checks pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
machine-learning
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
84/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.