feat(zmq): materialize precomputed multimodal inputs on msgpack ingest - #1081
feat(zmq): materialize precomputed multimodal inputs on msgpack ingest#1081slin1237 wants to merge 1 commit into
Conversation
The msgpack ZMQ path decodes multimodal payloads (the wire type is mm-complete and the decoder is tensor-aware) but bypasses the frontend InputProcessor, so three derived pieces the scheduler requires never get produced: per-item pad_values, M-RoPE positions, and the pad_input_tokens substitution that makes distinct images prefix-compare unequal. A precomputed-mm request over this wire would reach the scheduler unmaterialized. Extract the InputProcessor's precomputed-mm block into materialize_precomputed_inputs (runtime/multimodal/materialize.py) and run it from both admission paths: the in-process frontend as before, and now MsgpackRecvSocket on ingest, alongside the existing sampling validation. The engine owns the mm math on every path; an external frontend owes only expanded-placeholder input_ids plus items carrying feature/hash/offsets (and grid metadata). Ingest semantics: - a payload with input_ids_unpadded set is taken as fully materialized upstream and passed through untouched; - M-RoPE is computed only when ALL mrope fields are unset — a payload carrying any of them (including the documented scalar-only contract) owns its positions. The frontend path previously keyed only on mrope_positions, which would have recomputed over a scalar-only payload; both paths now honor the decode-side contract; - a malformed mm payload (or mm on a text-only model) is marked via validation_error like any invalid request, so the frontend gets a terminal abort instead of a dropped stream or a dead scheduler. model_config threads into the recv socket the same way vocab_size does. Tests (CPU-only, riding the scalar-only M-RoPE contract): ingest materializes pad values and padded ids while preserving the unpadded originals; text-only model rejection flows pre-marked; upstream- materialized payloads pass through untouched; text requests unaffected. Signed-off-by: Simo Lin <25425177+slin1237@users.noreply.github.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 406905cc92
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if self._model_config is None or not getattr( | ||
| self._model_config, "is_multimodal_active", False | ||
| ): | ||
| return "multimodal_inputs were provided for a text-only model" |
There was a problem hiding this comment.
In disaggregated decode with --language-model-only, ModelConfig.is_multimodal_active is false even for a multimodal checkpoint, and the frontend path explicitly treats precomputed multimodal inputs as absent so the decode worker can use the expanded input_ids length without running the encoder. With this new check, the msgpack path marks the same request invalid whenever SMG includes multimodal_inputs, so decode-side workers abort valid multimodal requests instead of receiving remote KV. Please mirror the InputProcessor decode/language-model-only exemption rather than treating inactive multimodal as text-only.
Useful? React with 👍 / 👎.
| multimodal_inputs.mrope_positions is None | ||
| and multimodal_inputs.mrope_position_delta is None | ||
| and multimodal_inputs.mrope_position_delta_scalar is None |
There was a problem hiding this comment.
When an upstream payload carries only mrope_position_delta or mrope_position_delta_scalar, this condition skips compute_mrope_positions even though mrope_positions is still absent. The prefill builder only consumes per-token mrope_positions for image/token sections; with positions missing it either applies a constant delta or falls back to linear positions, so Qwen-VL/Omni prefill loses the image-aware M-RoPE that the previous path computed whenever mrope_positions was None. Gate the skip on mrope_positions being present, or otherwise materialize the per-token positions.
Useful? React with 👍 / 👎.
| # copies of the Software, and to permit persons to whom the Software is | ||
| # furnished to do so, subject to the following conditions: | ||
| # | ||
| # THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR |
There was a problem hiding this comment.
The new file drops the MIT header paragraph that requires the copyright and permission notice to be included in copies; repository guidance requires the full MIT license header rather than a shortened copyright-only form, so this should match the existing file headers before merging.
AGENTS.md reference: AGENTS.md:L15-L16
Useful? React with 👍 / 👎.
1 participant
The msgpack ZMQ path decodes multimodal payloads (the wire type is
mm-complete and the decoder is tensor-aware) but bypasses the frontend
InputProcessor, so three derived pieces the scheduler requires never
get produced: per-item pad_values, M-RoPE positions, and the
pad_input_tokens substitution that makes distinct images
prefix-compare unequal. A precomputed-mm request over this wire would
reach the scheduler unmaterialized.
Extract the InputProcessor's precomputed-mm block into
materialize_precomputed_inputs (runtime/multimodal/materialize.py) and
run it from both admission paths: the in-process frontend as before,
and now MsgpackRecvSocket on ingest, alongside the existing sampling
validation. The engine owns the mm math on every path; an external
frontend owes only expanded-placeholder input_ids plus items carrying
feature/hash/offsets (and grid metadata).
Ingest semantics:
upstream and passed through untouched;
carrying any of them (including the documented scalar-only contract)
owns its positions. The frontend path previously keyed only on
mrope_positions, which would have recomputed over a scalar-only
payload; both paths now honor the decode-side contract;
validation_error like any invalid request, so the frontend gets a
terminal abort instead of a dropped stream or a dead scheduler.
model_config threads into the recv socket the same way vocab_size does.
Tests (CPU-only, riding the scalar-only M-RoPE contract): ingest
materializes pad values and padded ids while preserving the unpadded
originals; text-only model rejection flows pre-marked; upstream-
materialized payloads pass through untouched; text requests unaffected.