Repository navigation
[EPD] Metadata-only image and video inputs for GLM-5.3-Flash - #64071
Conversation
|
Documentation preview: https://vllm--64071.org.readthedocs.build/en/64071/ |
Isotr0py
left a comment
There was a problem hiding this comment.
Also cc @gty111 @JaredforReal
| ### Vision inputs | ||
|
|
||
| GLM-5.3-Flash accepts metadata-only image and video inputs through the Python | ||
| EPD proxy. The encoder publishes `image_grid_thw` for each image, and | ||
| `video_grid_thw` plus one timestamp per temporal patch for each video. The | ||
| consumer rebuilds the placeholders from those fields and receives the | ||
| embeddings through its EC connector, without repeating vision preprocessing. | ||
|
|
There was a problem hiding this comment.
I think this too model-specific in feature doc.
There was a problem hiding this comment.
Removed the GLM-5.3-Flash section from docs/features/disagg_encoder.md. The feature doc no longer calls out one model.
| def _published_video_timestamps(self, video_item) -> list[int]: | ||
| """Timestamps the encoder publishes so a consumer can skip sampling.""" | ||
| video, metadata = video_item | ||
| return self._get_video_second_idx_glm46v(metadata, len(video)) | ||
|
|
||
| def _placeholder_from_timestamps( | ||
| self, | ||
| timestamps: Sequence[float] | torch.Tensor, | ||
| grid_thw: torch.Tensor, | ||
| ) -> list[int]: | ||
| """Video placeholder from a grid and the timestamps that size it.""" | ||
| hf_processor = self.get_hf_processor() | ||
| tokenizer = self.get_tokenizer() | ||
| hf_config = self.get_hf_config() | ||
| merge_length = hf_processor.image_processor.merge_size**2 | ||
| if isinstance(timestamps, torch.Tensor): | ||
| timestamp_values = timestamps.flatten().tolist() | ||
| else: | ||
| timestamp_values = list(timestamps) | ||
|
|
||
| frames_idx_token = [ | ||
| tokenizer.encode(f"{float(ts):.1f} seconds", add_special_tokens=False) | ||
| for ts in timestamp_values | ||
| ] | ||
| _, height, width = grid_thw | ||
| num_tokens_per_frame = int(height * width) // merge_length | ||
| frame_embed_token_id = hf_processor.image_token_id | ||
| placeholder = [hf_config.video_start_token_id] | ||
| for frame_idx in frames_idx_token: | ||
| placeholder.append(hf_config.image_start_token_id) | ||
| placeholder.extend([frame_embed_token_id] * num_tokens_per_frame) | ||
| placeholder.append(hf_config.image_end_token_id) | ||
| placeholder.extend(frame_idx) | ||
| placeholder.append(hf_config.video_end_token_id) | ||
| return placeholder |
There was a problem hiding this comment.
I think this is duplicate with _construct_video_placeholder?
There was a problem hiding this comment.
Done in 287204d
_construct_video_placeholder now only selects the timestamps and calls _placeholder_from_timestamps. The token assembly is not duplicated. The metadata-only path uses that same helper.
| def _apply_hf_processor_main( | ||
| self, | ||
| mm_items: MultiModalDataItems, | ||
| hf_kwargs: Mapping[str, object], | ||
| ): | ||
| processed = super()._apply_hf_processor_main(mm_items, hf_kwargs) | ||
| video_items = mm_items.get("video") | ||
| if ( | ||
| video_items is not None | ||
| and video_items.get_count() > 0 | ||
| and not isinstance(video_items, DictEmbeddingItems) | ||
| and "timestamps" not in processed | ||
| ): | ||
| # Publish the same timestamps the prompt replacement encodes, so an | ||
| # EC consumer can rebuild the placeholder without the frames. | ||
| processed["timestamps"] = [ | ||
| self._glm_info()._published_video_timestamps(video_items.get(i)) | ||
| for i in range(video_items.get_count()) | ||
| ] | ||
| return processed |
There was a problem hiding this comment.
I think we should update these methods at vllm/model_executor/models/glm4_1v.py instead? GLM5.3-Flash's mm processing is quite similar to GLM4.6V.
There was a problem hiding this comment.
Done in 287204d
Moved the metadata-only parser, timestamp publishing, and prompt update onto Glm4vProcessingInfo / Glm4vMultiModalProcessor. GLM-5.3-Flash inherits them. Its frame sampler still overrides _get_video_second_idx_glm46v, which this path calls.
gty111
left a comment
There was a problem hiding this comment.
Two suggestions on video timestamp handling:
| num_tokens_per_frame = int(H * W) // merge_length | ||
| placeholder = [] | ||
| placeholder.append(bov_token_id) | ||
| _, height, width = grid_thw |
There was a problem hiding this comment.
Could we validate len(timestamp_values) == grid_thw[0] here and add a negative test? Otherwise, inconsistent metadata produces a placeholder count that does not match the encoder output.
There was a problem hiding this comment.
Done in 002d127
_placeholder_from_timestamps now checks len(timestamps) == video_grid_thw[0] before it builds the placeholder. A mismatch raises ValueError instead of emitting a placeholder whose token count cannot match the encoder rows.
The negative case is test_placeholder_rejects_timestamp_count_mismatch in tests/models/multimodal/processing/test_glm5next.py. I reran that check on the current patch with the local zai-org/GLM-5.3-Flash processor: two timestamps against grid_t=4 raised the error.
| and "timestamps" not in processed | ||
| ): | ||
| processed["timestamps"] = [ | ||
| self.info._video_timestamps(*video_items.get(i)) |
There was a problem hiding this comment.
Could we derive timestamps from the actual frame indices used by the HF processor, where available, rather than recomputing sampling from the original metadata? This would avoid drift if sampling options or padding behavior change.
There was a problem hiding this comment.
Done in 002d127
When frames_indices is already on the video metadata, the timestamp helpers use those indices instead of sampling again from fps and duration, including when do_sample_frames is still true. After call_hf_processor, indices recorded on the VideoMetadata are copied back onto the item, so the timestamps published for EPD follow the frames the processor kept. If the processor did not record indices, the previous sampler is unchanged.
test_video_timestamps_follow_supplied_frame_indices covers this. On the current patch with the local GLM-5.3-Flash processor, indices [0, 8, 80, 88] produced timestamps [0, 10]; resampling that clip without those indices produced 32 timestamps.
Would you mind sharing the encoder launch command? My understanding is that a dedicated ec_producer automatically enables encoder-only mode. In our TP=1 test on latest main, the vision weights were ~1.05 GiB and startup GPU memory was ~3.97 GiB without eager mode. Could you clarify why encoder TP=4 was needed here? |
|
@gty111 No separate Your reading of current main is right. A dedicated I reran the encoder alone on the current patch with |
gty111
left a comment
There was a problem hiding this comment.
Two issues found in the updated patch:
| # The loader already picked the frames and the processor keeps them | ||
| # as they are, so the frame count is what was handed in and | ||
| # ``frames_indices`` only supplies the timestamp values. | ||
| if "frames_indices" in metadata or not do_sample_frames: |
There was a problem hiding this comment.
Input frames_indices may describe frames before HF sampling. With 32 frames at 8 fps, frames_indices=list(range(32)), and do_sample_frames=True, I reproduced 16 timestamps for grid_t=4 on the raw-video path. The sampled-index writeback only covers the old Glm4vProcessor branch, not GLM-5.3's direct path. Could we synchronize the actual sampled indices there too and add this roundtrip case?
There was a problem hiding this comment.
Done in fb25f7e.
Input frames_indices with do_sample_frames=True is the pre-sample range. The HF processor returns a new VideoMetadata whose frames_indices are the frames it kept; the object we passed in is left unchanged. GLM-5.3's direct path now copies those returned indices back onto the item before timestamps are built. The older Glm4vProcessor branch reads the returned metadata the same way.
test_glm5_raw_video_timestamps_follow_sampled_frames is the 32-frame, 8 fps, frames_indices=range(32) case. On this patch it produces grid_t=4 and timestamps [0, 1, 2, 3], and the metadata-only prompt matches the raw one.
| def _parse_image_data(self, data): | ||
| if isinstance(data, dict): | ||
| required, optional = self.embedding_field_sets("image") | ||
| return DictEmbeddingItems( |
There was a problem hiding this comment.
Could we validate supplied embeddings in both dictionary branches: rank, hidden size, and row count against the grid? With expected_hidden_size=16 and grid [1,4,4] (merge size 2), this parser accepts shapes [4,1], [4], and [3,16]. The ordinary tensor path rejects the wrong hidden size, while this path defers failures to model execution. Metadata-only inputs should remain valid.
There was a problem hiding this comment.
Done in fb25f7e.
Both dictionary branches now check a supplied embedding before building DictEmbeddingItems: rank 2, hidden size against expected_hidden_size when that is set, and row count against prod(grid_thw) / merge_size**2. With expected_hidden_size=16 and grid [1, 4, 4] (merge size 2), shapes [4, 1], [4], and [3, 16] are rejected for image and video. Omitting the embedding tensor is still valid when allow_missing_mm_embeddings is set.
Covered by test_glm_dict_embeds_must_match_grid_and_hidden_size.
gty111
left a comment
There was a problem hiding this comment.
One remaining issue with multi-image embedding validation:
| f"grid_thw must have shape (N, 3), got {tuple(grid_thw.shape)}" | ||
| ) | ||
| merge = self._spatial_merge_size**2 | ||
| return int(grid.prod().item()) // merge |
There was a problem hiding this comment.
Could we sum the per-item row counts instead? grid.prod() multiplies across images, so two grids [1,4,4] with merge size 2 require 64 rows here instead of 8, rejecting valid embeddings. Suggested fix: int((grid.prod(dim=-1) // merge).sum().item()). A multi-image test would cover this.
There was a problem hiding this comment.
Done in 55c590b.
_embed_row_count now sums each item's prod(t, h, w) / merge_size**2 instead of multiplying the whole grid tensor. Two [1, 4, 4] grids with merge size 2 accept 8 rows and reject 64. Image and video dict embeddings share this count. The multi-image case is in test_glm_dict_embeds_must_match_grid_and_hidden_size.
gty111
left a comment
There was a problem hiding this comment.
LGTM. Thanks for the work.
55c590b to
097536d
Compare
|
/ci run |
|
✅ Triggered Buildkite CI #98208 for commit ℹ️ This PR is 180 commits behind upstream |
Head branch was pushed to by a user without write access
19c5c90 to
c569fbe
Compare
|
/ci run |
|
✅ Triggered Buildkite CI #98242 for commit |
|
@Isotr0py Thanks for the approval and kicking off CI. Buildkite #98208 failed on
Root cause: this PR started preferring metadata Fix in
Re-triggered CI: https://buildkite.com/vllm/ci/builds/98242 |
c569fbe to
55d3260
Compare
Let an EC consumer size image and video placeholders from the encoder's grid and timestamps instead of running vision preprocessing again. Signed-off-by: grYe99 <guorongye99@gmail.com>
Move the placeholder metadata parser onto GLM-4V so GLM-5.3-Flash inherits it, and build video placeholders from the existing constructor. Signed-off-by: grYe99 <guorongye99@gmail.com>
InputProcessingContext is frozen, so the consumer-path check has to replace call_hf_processor on the class. Signed-off-by: grYe99 <guorongye99@gmail.com>
…context" This reverts commit b4594d7. Signed-off-by: grYe99 <guorongye99@gmail.com>
Compare raw and metadata-only prompt tokens without patching the frozen processor context. Signed-off-by: grYe99 <guorongye99@gmail.com>
Placeholder construction now requires one timestamp per temporal patch. When the processor or loader already selected frames, those indices are used instead of sampling again. Signed-off-by: grYe99 <guorongye99@gmail.com>
The direct path was publishing timestamps from the pre-sample frame range. Dict embeddings now have to match the grid and hidden size; metadata-only inputs stay valid. Signed-off-by: grYe99 <guorongye99@gmail.com>
grid.prod() across a batch of grids rejects a valid multi-image embedding. Each item's token count is computed first, then added. Signed-off-by: grYe99 <guorongye99@gmail.com>
Preferring frames_indices whenever present made GLM-4.1V reuse the pre-sample range and emit far more timestamps than video_grid_thw. Restore the do_sample_frames gate, and clear that flag after HF write-back so GLM-5.3 still keeps the sampled indices. Signed-off-by: grYe99 <guorongye99@gmail.com>
In-place write-back of do_sample_frames=False poisoned the caller's mm_data dict. A later re-parse (cache path in processing_correctness) then ran HF without sampling on the original odd-length video, so grid_t exceeded the timestamp count from the written-back indices. Copy the metadata dict into mm_items instead. Signed-off-by: grYe99 <guorongye99@gmail.com>
Feed HF-sampled frame metadata into the timestamp helper via overrides instead of mutating mm_items. test_processing_correctness reuses the same items for baseline then cache; persisting do_sample_frames=False against a full-length video made pass-2 HF emit mismatched timestamps for odd-length videos. Signed-off-by: grYe99 <guorongye99@gmail.com>
55d3260 to
e33a639
Compare
|
@Isotr0py Follow-up after #98242. That run failed on GLM-5.3 Post-approval bugfixes now on the branch (force-pushed while rebasing from
Risk vs your approved tip ( |
|
/ci run |
|
✅ Triggered Buildkite CI #98301 for commit ℹ️ This PR is 7 commits behind upstream |
|
pre-commit fail |
Narrow MultiModalKwargsItem and NestedTensors types in test_glm5_raw_video_timestamps_follow_sampled_frames, which landed in vllm-project#64071 concurrently with vllm-project#64342 enabling stricter mypy for tests/models/multimodal. Signed-off-by: Micah Williamson <micah.williamson@amd.com> Co-authored-by: Cursor <cursoragent@cursor.com>
…oject#64071) Signed-off-by: grYe99 <guorongye99@gmail.com> Co-authored-by: grYe99 <guorongye99@gmail.com> Co-authored-by: vllm-agent <claw@inferact.ai> Signed-off-by: Dao Le <Dao007forever@gmail.com>
Overview
GLM-5.3-Flash can now take metadata-only image and video inputs in Python EPD, the same out-of-band placeholder path as Qwen-VL (#50390) and audio models (#57887). The encoder publishes
image_grid_thw, and for videovideo_grid_thwplus one timestamp per temporal patch. The consumer rebuilds placeholders from those fields and receives embeddings through the EC connector.No open PR covers this. #61418 is routing-metadata adapters, not EPD media metadata.
Claims
Validation
Not a duplicate of an open PR (search above).
AI assistance was used. The submitting human should review the diff.
Parser contract on the same H200 pod, against the installed tree with this patch (
pytest9.1.1,torchvision0.28.0):4 passed in 15.54s.
End-to-end on 8× H200,
zai-orgcheckpoint/models/GLM-5.3-Flash(bf16/fp8), vLLM 0.32.0 plus this patch. The PD worker loads the full checkpoint, 76.5 GiB per TP=4 rank, so the PD side is TP=4 on GPU 4–7. The encoder is a dedicatedec_producerand runs encoder-only. In the throughput runs it was also started at TP=4 on GPU 0–3, and each encoder worker loaded 1.17 GiB; a later startup of this patch with--tensor-parallel-size 1loaded 1.93 GiB and used 4579 MiB on one GPU. Encoder TP=4 was not required for the vision tower.ECExampleConnectorover local disk was used becauseECMooncakeConnectorproducers requiretensor_parallel_size=1, and the producer was originally launched with the full-model command. That check does not rule out Mooncake for an encoder-only producer. The tables below were measured with encoder TP=4 andECExampleConnector. Max model len 8192, prefix caching left on for the PD instance, CUDA graphs on, concurrency 64, 64 distinct 768×768 images, 64 output tokens, greedy. Each arm was a fresh process start plus one warmup request, then two measured passes. Zero failed requests. The metadata proxy logged 129 rewrites; the raw proxy (--no-rewrite) logged 0.Logs: the run was on a private pod; the table is the
Serving Benchmark Resultblock fromvllm bench serve.Video throughput was rerun with
vllm bench serveafter the first 16-request burst proved to be one unschedulable wave (raw warm slower than raw cold, metadata cold dominated by a few multi-second prefills). This run is a different pod, same image and patch. Each arm is a fresh 1E+1PD process and a fresh EC cache. 64 distinct 2-second 320×240 clips at 8 fps, ShareGPT dataset, 64 output tokens, greedy, concurrency 8. One probe measured 248 prompt tokens, and 8192 / 248 caps concurrency at 8. Pass 1 is preceded by 8 warmups of the first clip so Triton JIT is outside the measurement. Pass 2 and pass 3 are repeats with no extra warmup. Zero failed requests. Mean TTFT stays next to the median, and P99 TTFT stays within about 1.5× the mean. The metadata proxy logged 201 rewrites; the raw proxy logged 0. On each paired pass, metadata-only is slightly ahead (about 1.07–1.11× output tok/s and 0.86–0.92× mean TTFT). Repeating a warm pass on the same arm moves output tok/s by 9–12%, which is the same size as that gap, so this table is not a video speedup claim.Correctness used the same 1E+1PD stack and checkpoint, greedy,
max_tokens=32, one request at a time. Raw (--no-rewrite) first, then metadata-only.Image: 32 distinct 768×768 images. The metadata proxy logged 32 rewrites.
Video: 16 distinct 2-second 320×240 clips (8 fps). The metadata proxy logged 16 rewrites. The throughput table above is a separate 64-clip
vllm bench serverun, not this greedy check.Image and video placeholder reconstruction, without a serving stack, is
test_glm5_metadata_only_roundtripintests/models/multimodal/processing/test_glm5next.py. On the H200 pod, with this patch and the local/models/GLM-5.3-Flashprocessor:2 passed in 24.56s (
imageandvideo). Each case builds the prompt from raw media, publishes the EC metadata, and checks that metadata-only inputs reproduce the same prompt token ids and placeholders.Details
Glm4vMultiModalDataParser.embedding_fieldsmarks the grid and video timestamps as placeholder metadata and the embeds as values, soallow_missing_mm_embeddingsdrops only the embeds on an EC consumer. Timestamps are computed with the existing GLM-5.3 frame sampler and stored on the processed video item, which is what the encoder publishes and what the prompt replacement reads. Image replacement was already grid-sized; it now works when the request carries onlyimage_grid_thw.Video still needs those timestamps because a GLM placeholder includes one timestamp string per temporal patch, not only
grid.prod(). Requests that omit the metadata stay on the raw-media path.Pull Request Checklist
I used vLLM's
/pr-checklistskill. (Mandatory for agents, optional for humans).AI assistance was used during the creation of this PR.
Design Fit: Minimizes impact on core components, reuses existing functionality, and justifies added complexity.
Testing and Validation: Validates the change and ensures any added tests are meaningful and reliable, with CI coverage or documented CI resource constraints and validation performed outside CI.
Code Quality and Style: Keeps code and comments clear and concise, and updates relevant documentation and examples.
Pull Request Contents: Includes a brief summary and relevant links, supports claims with evidence, explains root causes and implementation trade-offs, and follows the contributing guide.