Repository navigation
[Bugfix][MiniCPM-o] Size the duplex HD slice reservation from the frame - #7654
Conversation
_duplex_vision_tokens reserves three 66-token blocks for the base frame of every stacked pair, but MiniCPM-o's HD slicing depends on the frame's size. At the official max_slice_nums=2 the model's grid search has only two outcomes -- get_sliced_grid takes min(ceil(w * h / scale_resolution**2), max_slice_nums) and returns no grid at all below 2 -- so the base frame is either one tile or three blocks. Anything at or below one tile reserved three blocks for one. The number is not advisory. build_duplex_data_plane_prompt turns it into [scheduler_token_id] * budget, and a unit whose embeddings outnumber its slots has its tail dropped with a warning rather than failing. Reserving one block for a 960x540 pair on a live server produces exactly that: "276 embeddings but the scheduler reserved only 226 prompt slots", while the session still answers with a full audio stream. Under-reserving is silent audio loss, so every uncertainty here resolves upwards. scale_resolution is the checkpoint's -- MiniCPMVImageProcessor is built with scale_resolution=config.image_size and Stage0 loads the checkpoint's own processor -- so the tile is resolved from the model config when the session opens and carried in the private runtime config. Without it the reservation stays at the sliced count. Verified against the released MiniCPM-o-4.5 processor: ten frame sizes in JPEG and PNG match the block count process_image produces for max_slice_nums=[2, 1] in all twenty cases, and a sweep of 66861 sizes at each of three scale_resolutions finds no size where the tile comparison disagrees with the model's grid search. On a live duplex server a 448x448 pair now reserves 132 tokens instead of 264 and a 960x540 pair still reserves 264; neither truncates. These functions had no test coverage; the new file also pins the audio side of the budget. Part of vllm-project#7636 (issue 15). Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Tianyao Wu <rayroy31@gmail.com>
|
This PR appears to belong to: docs/design/module/model_integration.md, docs/design/module/ar_runtime.md. Module owners: @tzhouam @fake0fan @Gaohan123 Routing: @tzhouam via module of the changed files, CODEOWNERS; @fake0fan via module of the changed files; @Gaohan123 via module of the changed files @twu3202, please review your own changes and leave a short self-review comment describing what you checked. PRs without author self-review may not be assigned a reviewer. Please take a look when you have a chance. If you would like an automated review, mention @vllm-omni-review-bot in a comment. |
…ppend Self-review follow-ups on this branch. `_model_vision_tile_pixels` and `_duplex_vision_tile_pixels` had no unit coverage. Every case drove the budget with a literal tile, so a checkpoint whose `scale_resolution` could not be read would still pass the whole file while production quietly fell back to the sliced reservation. The new cases cover the four places the tile is read from, the round trip through the runtime config, and the key being server-owned. `build_duplex_data_plane_prompt` computed the vision tokens unconditionally and `duplex_scheduler_token_budget` computed them again, so an append carrying frames parsed a frame header twice where `main` parses none. Moved back into the branch that uses it. Also normalization, not normalisation, to match the rest of the file. Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Tianyao Wu <rayroy31@gmail.com>
|
Self-review done. Two things worth mentioning, both fixed in 24a61bd:
I also fixed a number in the description: the per-file count read 87, which is the whole What else I checked: the reservation only moves up when the tile or the header cannot be read, |
linyueqian
left a comment
There was a problem hiding this comment.
Reviewed at 24a61bd9 (static read against main and against the processor code; nothing executed). The reservation now matches what Stage 0 emits: Stage 0 hands the decoded frame to process_image([frame], max_slice_nums=2) without resizing, get_sliced_grid at max_slice_nums=2 is binary (ceil(w*h/scale^2) <= 1 gives one tile, anything else gives the 2-cell grid plus source, so 1 or 3 blocks), and the plugin's width * height <= tile_pixels is the same predicate on the same raw size for positive integers, including the exact-tile and one-pixel-over boundaries. EXIF orientation and alpha cannot change the decision because neither side transposes before sizing and the area is rotation invariant. Every failure to read the header falls back to the sliced count, which over-reserves, and Stage 0 raises on the same frame, so there is no reachable under-reserve from decoding. Frames past the first are counted, not decoded. test_a_stacked_pair_reserves_what_the_model_slices_it_into pins the counts against the processor's own grid walk over JPEG and PNG, three tile sizes and the boundary sizes. Two suggestions inline (one about where the tile size is read from, one duplicate decode on first appends); neither blocks. Approving once the general lane reports on this head.
| configuration. ``None`` when it cannot be read, which keeps the reservation | ||
| at the sliced count. | ||
| """ | ||
| hf_config = getattr(model_config, "hf_config", None) |
There was a problem hiding this comment.
[suggestion] The tile area is read from the model config (slice_config.scale_resolution, then image_size) while Stage 0 slices with the checkpoint's processor, which reads preprocessor_config.json. For the released checkpoint all three are 448 so they agree, but a deployment that overrides one and not the other turns this into an under-reservation (a 500x500 base frame with a 560 tile here and 448 in the processor reserves 132 instead of 264 vision tokens, and the worker then drops the unit tail with a warning). Reading the value from the same processor Stage 0 loads, or falling back to the sliced count whenever the two sources disagree, keeps the reservation tied to the code that does the slicing. The tests give both sides the same scale, so they cannot see this.
| first_units = duplex_first_append_unit_count(payload) | ||
| if first_units is not None: | ||
| token_budget = context_reserve + first_units * 12 - 1 + _duplex_vision_tokens(payload) | ||
| vision_tokens = _duplex_vision_tokens(payload, tile_pixels=tile_pixels) |
There was a problem hiding this comment.
[suggestion] For a first append with a stacked pair and a known tile size this is the second base64 decode and header parse of the same frame in one call: duplex_scheduler_token_budget at the top of the function already computed _duplex_vision_tokens for the same payload. Computing the vision tokens once and reusing them in both branches keeps the append path to one decode.
…me (vllm-project#7654) Signed-off-by: Tianyao Wu <rayroy31@gmail.com> Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Matthieu Laneuville <matthieu.laneuville@surf.nl>
…me (vllm-project#7654) Signed-off-by: Tianyao Wu <rayroy31@gmail.com> Co-authored-by: Claude <noreply@anthropic.com>
Purpose
Issue 15 of #7636.
_duplex_vision_tokensgives the base frame of every stacked pair three 66-tokenblocks (
vllm_omni/model_executor/models/minicpmo_4_5/duplex/plugin.py:110), buthow many blocks MiniCPM-o actually makes depends on the frame's size. At the
official
max_slice_nums=2there are only two answers:MiniCPMVImageProcessor.get_sliced_gridtakesmin(ceil(w * h / scale_resolution**2), max_slice_nums)and returns no grid atall below 2, so the frame is either one tile or three blocks. Anything that fits
in one tile was getting three.
The number is not only a scheduling hint.
build_duplex_data_plane_promptturnsit into
[scheduler_token_id] * budget, andpreprocess(
minicpmo_4_5_omni.py:392) fits the unit's embeddings into exactly that manyprompt positions:
front of the unit, and the model reads them. On
maina 448x448 pair puts 132of them before every unit.
minicpmo_4_5_omni.py:411only fires while the whole prompt is still shorterthan one unit. After that it is the start of the unit that goes, which is where
the unit marker and the frames sit, and nothing is logged.
Cutting is the worse of the two, so the size is read from the frame's own header,
in the bare-base64 JPEG/PNG Stage0 decodes, and anything that cannot be read keeps
the old count.
The tile size belongs to the checkpoint, not to the code:
MiniCPMVImageProcessoris built withscale_resolution=config.image_size(
minicpmo_4_5_omni_llm.py:3124) and Stage0 loads the checkpoint's own processor(
duplex/stage0.py:686). It is resolved once when the session opens and carriedin the private runtime config next to
duplex_scheduler_token_id.Test Plan
vLLM Version: 0.29.0
vLLM-Omni Commit: fa639b8
tests/assets/minicpmo_4_5/response_required_16k.wavwith a stackedcamera pair on every unit into a three-stage server built from
fa639b889and from this branch, and record the prompt slots and embeddings of each unit.
what
MiniCPMOProcessor.process_imageon the released checkpoint produces formax_slice_nums=[2, 1].w * h <= tileagainst the model's grid search over a wide size rangeat three
scale_resolutionvalues.pytest -q tests/model_executor -m 'core_model and cpu', plustests/engineand
tests/entrypointsat the same marker, andtest_duplex_single_session_video_inputon both commits.Test Result
1. Live server
Real weights. To fit a 48 GB card, Stage 0 ran with
max_num_batched_tokens: 4096andmax_model_len: 32768; the rest ofminicpmo_4_5.yamlis as shipped. The clip goes in as 200 ms appends, and eachunit carries the frame plus a 2W x H composite. A logging-only wrapper around
preprocessrecorded how many positions each unit was scheduled into and how manyembeddings Stage0 built for it.
fa639b889The last unit of the clip gets 12 more slots in every row, with or without this
change.
The forced row is a deliberate under-reservation, to check the measurement can
see one. All five units with frames were short, and the server logged the
truncation warning once, for the first of them.
One thing showed up that this change does not cause. With 448x448 frames, this
branch stops mid-answer on this clip in 6 of 6 runs: four audio deltas and no
response.done.fa639b889finishes all 6. With the pads gone the model's outputchanges, and an empty TTS segment now ends just before the answer starts, while
the client's last units are still arriving. That hits a race in the silence
continuation: the continuation armed by that segment waits one chunk period, finds
new units and an open response, gives up, and nothing schedules another unit.
Replaying the same order of outputs through the session runner on
fa639b889stops the same way. Reported in #7729.
2. What the model actually slices
66 tokens per block, stacked pair (current frame + composite):
Same in JPEG and PNG, 20 cases, no mismatches. The plugin column reads the tile
from the checkpoint's config through the same reader a session uses. The released checkpoint
uses
scale_resolution=448, so 448x448 is the largest frame that still fits onetile.
Worth being clear about the size of this: only frames that fit in one tile
change. 640x480 and everything above still reserves 264, so an ordinary webcam
capture is unaffected.
3. Boundary sweep
66 861 sizes, including 4000x40, 40x4000, 1x200000 and everything near the
boundary, at three
scale_resolutionvalues:scale_resolutionmax_slice_nums=2w * h <= tilegets it wrongNone,(1, 2),(2, 1)None,(1, 2),(2, 1)None,(1, 2),(2, 1)4. CI scope
tests/model_executortests/enginetests/entrypointstests/model_executor/models/minicpmo_4_5/duplex/test_scheduler_token_budget.pyalone: 70 passed. It is a new file using the new signature, so it does not run
against
main; what it pins is thebeforecolumn in §2.test_duplex_single_session_video_input, the video case of the merge-levelOmni · MiniCPM-o 4.5 Duplex Test, passes onfa639b889and on this branch withreal weights and the Stage 0 settings from §1. Its 448x448 frames get 132 fewer
slots per unit here, and no unit is short.
Not run: the rest of the CUDA jobs the MiniCPM-o group gates, including the
duplex perf job.
Why the tile is read from the config
The obvious alternative is to call
get_sliced_griddirectly.plugin.pyrunsengine-side and nothing it imports reaches
minicpmo_4_5_omni_llm, so calling itper append would drag the whole model module into the engine core. The comparison
is spelled out in a comment instead, against a value that comes from the
checkpoint. The test parametrizes
scale_resolutionover 336/448/560 to keepthat honest — a hardcoded 448 fails six of those cases.
Note
Neither
_duplex_vision_tokensnorduplex_scheduler_token_budgethad anytests. The new file also covers the audio side of the budget, which this does not
change, and which frame of a pair gets sliced, which nothing covered before.