feat(cpu): add Qwen3.5 0.8B single-image multimodal support - #695
Conversation
📝 WalkthroughWalkthroughQwen3.5 0.8B now supports single-image inference. The change adds vision configuration, image preprocessing, a vision transformer, multimodal token and position handling, runtime integration, CLI options, checkpoint validation, conversion recipes, documentation, and tests. ChangesQwen3.5 multimodal support
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Qwen3_5CLI
participant Qwen3_5Tokenizer
participant Qwen3_5Model
participant Qwen3_5VisionModel
Qwen3_5CLI->>Qwen3_5Tokenizer: convert prompt and image path
Qwen3_5Tokenizer->>Qwen3_5Tokenizer: preprocess image and expand placeholders
Qwen3_5Tokenizer->>Qwen3_5Model: tokens, pixel values, grid, and token types
Qwen3_5Model->>Qwen3_5VisionModel: encode image
Qwen3_5VisionModel->>Qwen3_5Model: image embeddings
Qwen3_5Model->>Qwen3_5Model: replace image tokens and forward embeddings
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (5)
tests/cpu/Qwen35MultimodalTest.cpp (1)
210-213: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInitialize the tensors used in the rejection test.
Tensor::empty(...).alloc()leaves the buffers uninitialized. Both cases currently throw before the model reads any element, so the test passes. If the validation order inQwen3_5ForCausalLM::forwardchanges, the test reads indeterminate token IDs and becomes non-deterministic. Fillsequence,image_grid, andtoken_typeswith valid values.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/cpu/Qwen35MultimodalTest.cpp` around lines 210 - 213, The rejection test must initialize its input tensors before invoking the model. In the test setup around sequence, image_grid, and token_types, fill each allocated buffer with valid deterministic values while preserving the existing tensor shapes and types; pixel_values does not require changes unless the test reads it.mllm/models/qwen3_5/modeling_qwen3_5_vision.hpp (2)
269-270: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove the vision LayerNorm epsilon into the config.
1.0e-6Fis hard-coded inQwen3_5VisionBlockandQwen3_5VisionPatchMerger. The text tower readscfg.rms_norm_eps. Add avision_layer_norm_epsfield toQwen3_5Configand read it here. This keeps the epsilon aligned with the checkpointvision_configand removes the magic value.Also applies to: 295-295
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mllm/models/qwen3_5/modeling_qwen3_5_vision.hpp` around lines 269 - 270, Add a vision_layer_norm_eps field to Qwen3_5Config, populated from the checkpoint vision_config, and replace the hard-coded 1.0e-6F epsilon in Qwen3_5VisionBlock and Qwen3_5VisionPatchMerger LayerNorm construction with cfg.vision_layer_norm_eps.
67-77: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winPrecompute the inverse frequencies outside the sequence loop.
std::powruns once per (sequence, axis, dim) element, but the value depends only ond. Compute theinv_freq_dimvalues once and reuse them. This removessequence * 2redundantstd::powcalls per element.♻️ Proposed refactor
+ std::vector<float> inv_freqs(static_cast<size_t>(inv_freq_dim)); + for (int32_t d = 0; d < inv_freq_dim; ++d) { + inv_freqs[d] = 1.0F / std::pow(theta, static_cast<float>(2 * d) / axis_dim); + } for (int32_t s = 0; s < sequence; ++s) { for (int32_t axis = 0; axis < 2; ++axis) { for (int32_t d = 0; d < inv_freq_dim; ++d) { - const float inv_freq = 1.0F / std::pow(theta, static_cast<float>(2 * d) / axis_dim); - const float value = static_cast<float>(positions[s * 2 + axis]) * inv_freq; + const float value = static_cast<float>(positions[s * 2 + axis]) * inv_freqs[d];Add
#include <vector>to the header includes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mllm/models/qwen3_5/modeling_qwen3_5_vision.hpp` around lines 67 - 77, In the vision rotary-frequency computation, precompute the `inv_freq_dim` inverse-frequency values once before the sequence loop, storing them in a vector, and reuse them for both axes and every sequence position instead of calling `std::pow` inside the nested loops. Add the required vector include and preserve the existing `sin_ptr`/`cos_ptr` indexing and outputs.mllm/models/qwen3_5/multimodal_qwen3_5.hpp (1)
17-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPublic API documentation is missing across the three new Qwen3.5 headers. Each header defines public entities with strict, undocumented tensor contracts. Callers must currently read the validation code to learn the required shapes, dtypes, layouts, and error conditions.
mllm/models/qwen3_5/multimodal_qwen3_5.hpp#L17-L19: documentexpandQwen3_5SingleImagePlaceholders,makeQwen3_5InterleavedRotaryEmbedding,makeQwen3_5SingleImagePositionIds, andadvanceQwen3_5PositionIds, including the[3,1,S]position layout and thestd::invalid_argumentconditions.mllm/models/qwen3_5/modeling_qwen3_5_vision.hpp#L20-L24: documentmakeQwen3_5VisionPositionIds,makeQwen3_5VisionRotaryEmbedding,makeQwen3_5VisionBilinearPositionEmbedding, andqwen3_5ExactGelu, including the block-major output order.mllm/models/qwen3_5/image_preprocessor_qwen3_5.hpp#L18-L30: documentQwen3_5ImagePreprocessor, its five geometry parameters, and the returned patch and grid tensors.As per coding guidelines: "Ensure public APIs, classes, and functions have clear docstrings or comments explaining purpose, parameters, returns, and errors."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mllm/models/qwen3_5/multimodal_qwen3_5.hpp` around lines 17 - 19, Add clear API documentation to all three affected headers: in mllm/models/qwen3_5/multimodal_qwen3_5.hpp lines 17-19, document expandQwen3_5SingleImagePlaceholders, makeQwen3_5InterleavedRotaryEmbedding, makeQwen3_5SingleImagePositionIds, and advanceQwen3_5PositionIds, including tensor contracts, the [3,1,S] position layout, and std::invalid_argument conditions; in mllm/models/qwen3_5/modeling_qwen3_5_vision.hpp lines 20-24, document makeQwen3_5VisionPositionIds, makeQwen3_5VisionRotaryEmbedding, makeQwen3_5VisionBilinearPositionEmbedding, and qwen3_5ExactGelu, including block-major output order; in mllm/models/qwen3_5/image_preprocessor_qwen3_5.hpp lines 18-30, document Qwen3_5ImagePreprocessor, its five geometry parameters, and the returned patch and grid tensors.Source: Coding guidelines
mllm/models/qwen3_5/tokenization_qwen3_5.hpp (1)
385-388: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winDerive the image token ID from one source.
The tokenizer resolves the image token through
bpe_._lookup_vocab(L"<|image_pad|>").Qwen3_5ForCausalLMcompares againstcfg.image_token_id. Two independent sources define the same value. A mismatch surfaces late as "Qwen3.5 image token IDs and modality token types disagree", which does not name the real cause.Pass the configured
image_token_idinto the tokenizer, or validate the looked-up ID against the config once at construction and throw a message that names both values.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mllm/models/qwen3_5/tokenization_qwen3_5.hpp` around lines 385 - 388, Unify the image token ID used by tokenization and Qwen3_5ForCausalLM by passing the configured cfg.image_token_id into the tokenizer, or validating it against bpe_._lookup_vocab(L"<|image_pad|>") during construction. If validating, throw an error that explicitly includes both IDs; update expandQwen3_5SingleImagePlaceholders usage to rely on the single validated/configured value.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@mllm/models/qwen3_5/image_preprocessor_qwen3_5.hpp`:
- Around line 73-83: Update flattenNormalizedPatches to reject non-contiguous
image_hwc tensors alongside its existing dtype, device, and shape validation.
Use image_hwc.isContiguous(), matching the validation pattern in
qwen3_5ExactGelu, and preserve the existing invalid_argument behavior for
invalid inputs.
In `@mllm/models/qwen3_5/modeling_qwen3_5.hpp`:
- Around line 664-675: Update the validation loop in the Qwen3.5 single-image
path to require all image tokens marked by types[s] == 1 to form one contiguous
span starting at image_begin; reject any later image token after a non-image
token. Perform this check regardless of whether position_ids is supplied, before
the slice written at the subsequent image-feature assignment.
- Around line 515-530: The forwardEmbeddings entry path currently bypasses
module-level dispatch and tracing. Update Qwen3_5’s forwardEmbeddings method to
invoke the module’s __main dispatch, passing the embedding inputs and kv_cache
through it, while preserving forwardEmbeddingsImpl as the underlying
implementation used by the dispatched call.
In `@mllm/models/qwen3_5/multimodal_qwen3_5.hpp`:
- Around line 181-188: Update the next-position calculation in the cached
multimodal position flow to compute the maximum of the three final axis values,
add one, and assign that single value to every axis in the returned tensor. Keep
the existing shape validation and tensor allocation unchanged.
In `@mllm/models/qwen3_5/tokenization_qwen3_5.hpp`:
- Around line 359-366: Extend the kReservedMarkers list in the prompt validation
loop to include both "<|im_start|>" and "<|im_end|>", preserving the existing
invalid_argument behavior for any prompt containing reserved multimodal or
chat-control markers.
In `@tests/cpu/Qwen35MultimodalTest.cpp`:
- Around line 6-9: Update the include list in Qwen35MultimodalTest to explicitly
add the standard headers <algorithm> for std::fill, std::copy, and std::equal,
and <utility> for std::pair, without relying on transitive includes.
- Around line 156-159: Widen the EXPECT_NEAR tolerances in the exact-GELU
assertions within qwen3_5ExactGelu to 1.0e-6F for the nonzero expected outputs,
while leaving the EXPECT_FLOAT_EQ zero assertion unchanged.
---
Nitpick comments:
In `@mllm/models/qwen3_5/modeling_qwen3_5_vision.hpp`:
- Around line 269-270: Add a vision_layer_norm_eps field to Qwen3_5Config,
populated from the checkpoint vision_config, and replace the hard-coded 1.0e-6F
epsilon in Qwen3_5VisionBlock and Qwen3_5VisionPatchMerger LayerNorm
construction with cfg.vision_layer_norm_eps.
- Around line 67-77: In the vision rotary-frequency computation, precompute the
`inv_freq_dim` inverse-frequency values once before the sequence loop, storing
them in a vector, and reuse them for both axes and every sequence position
instead of calling `std::pow` inside the nested loops. Add the required vector
include and preserve the existing `sin_ptr`/`cos_ptr` indexing and outputs.
In `@mllm/models/qwen3_5/multimodal_qwen3_5.hpp`:
- Around line 17-19: Add clear API documentation to all three affected headers:
in mllm/models/qwen3_5/multimodal_qwen3_5.hpp lines 17-19, document
expandQwen3_5SingleImagePlaceholders, makeQwen3_5InterleavedRotaryEmbedding,
makeQwen3_5SingleImagePositionIds, and advanceQwen3_5PositionIds, including
tensor contracts, the [3,1,S] position layout, and std::invalid_argument
conditions; in mllm/models/qwen3_5/modeling_qwen3_5_vision.hpp lines 20-24,
document makeQwen3_5VisionPositionIds, makeQwen3_5VisionRotaryEmbedding,
makeQwen3_5VisionBilinearPositionEmbedding, and qwen3_5ExactGelu, including
block-major output order; in mllm/models/qwen3_5/image_preprocessor_qwen3_5.hpp
lines 18-30, document Qwen3_5ImagePreprocessor, its five geometry parameters,
and the returned patch and grid tensors.
In `@mllm/models/qwen3_5/tokenization_qwen3_5.hpp`:
- Around line 385-388: Unify the image token ID used by tokenization and
Qwen3_5ForCausalLM by passing the configured cfg.image_token_id into the
tokenizer, or validating it against bpe_._lookup_vocab(L"<|image_pad|>") during
construction. If validating, throw an error that explicitly includes both IDs;
update expandQwen3_5SingleImagePlaceholders usage to rely on the single
validated/configured value.
In `@tests/cpu/Qwen35MultimodalTest.cpp`:
- Around line 210-213: The rejection test must initialize its input tensors
before invoking the model. In the test setup around sequence, image_grid, and
token_types, fill each allocated buffer with valid deterministic values while
preserving the existing tensor shapes and types; pixel_values does not require
changes unless the test reads it.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a9a4b884-85c1-443a-9b7c-b9595cfcbcaf
📒 Files selected for processing (16)
examples/qwen3_5/README.mdexamples/qwen3_5/config_0.8B_multimodal_w4a32_kai.jsonexamples/qwen3_5/main.cppexamples/qwen3_5/quant_cfg_0.8B_multimodal_w4a32_kai.jsonexamples/qwen3_5/test_validators.pyexamples/qwen3_5/validate_checkpoint.pyexamples/qwen3_5/validate_converted_model.pymllm/models/qwen3_5/configuration_qwen3_5.hppmllm/models/qwen3_5/image_preprocessor_qwen3_5.hppmllm/models/qwen3_5/modeling_qwen3_5.hppmllm/models/qwen3_5/modeling_qwen3_5_vision.hppmllm/models/qwen3_5/multimodal_qwen3_5.hppmllm/models/qwen3_5/tokenization_qwen3_5.hpptests/cpu/CMakeLists.txttests/cpu/Qwen35ConfigTest.cpptests/cpu/Qwen35MultimodalTest.cpp
| [[nodiscard]] std::pair<Tensor, Tensor> flattenNormalizedPatches(const Tensor& image_hwc) const { | ||
| const auto& shape = image_hwc.shape(); | ||
| if (image_hwc.dtype() != kFloat32 || image_hwc.device() != kCPU || shape.size() != 3 || shape[2] != 3) { | ||
| throw std::invalid_argument("Qwen3.5 preprocessor expects a float32 CPU RGB tensor in HWC layout"); | ||
| } | ||
| const int32_t height = shape[0]; | ||
| const int32_t width = shape[1]; | ||
| const int32_t factor = patch_size_ * merge_size_; | ||
| if (height <= 0 || width <= 0 || height % factor != 0 || width % factor != 0) { | ||
| throw std::invalid_argument("Qwen3.5 resized image dimensions must be divisible by patch_size * merge_size"); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add a contiguity check on image_hwc.
The function reads image_hwc.ptr<float>() with computed HWC offsets. A non-contiguous input produces silently wrong patch data. qwen3_5ExactGelu in mllm/models/qwen3_5/modeling_qwen3_5_vision.hpp already validates isContiguous(). Apply the same check here, because this method is public and documented as test-facing.
🛡️ Proposed fix
- if (image_hwc.dtype() != kFloat32 || image_hwc.device() != kCPU || shape.size() != 3 || shape[2] != 3) {
+ if (image_hwc.dtype() != kFloat32 || image_hwc.device() != kCPU || !image_hwc.isContiguous() || shape.size() != 3
+ || shape[2] != 3) {
throw std::invalid_argument("Qwen3.5 preprocessor expects a float32 CPU RGB tensor in HWC layout");
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| [[nodiscard]] std::pair<Tensor, Tensor> flattenNormalizedPatches(const Tensor& image_hwc) const { | |
| const auto& shape = image_hwc.shape(); | |
| if (image_hwc.dtype() != kFloat32 || image_hwc.device() != kCPU || shape.size() != 3 || shape[2] != 3) { | |
| throw std::invalid_argument("Qwen3.5 preprocessor expects a float32 CPU RGB tensor in HWC layout"); | |
| } | |
| const int32_t height = shape[0]; | |
| const int32_t width = shape[1]; | |
| const int32_t factor = patch_size_ * merge_size_; | |
| if (height <= 0 || width <= 0 || height % factor != 0 || width % factor != 0) { | |
| throw std::invalid_argument("Qwen3.5 resized image dimensions must be divisible by patch_size * merge_size"); | |
| } | |
| [[nodiscard]] std::pair<Tensor, Tensor> flattenNormalizedPatches(const Tensor& image_hwc) const { | |
| const auto& shape = image_hwc.shape(); | |
| if (image_hwc.dtype() != kFloat32 || image_hwc.device() != kCPU || !image_hwc.isContiguous() || shape.size() != 3 | |
| || shape[2] != 3) { | |
| throw std::invalid_argument("Qwen3.5 preprocessor expects a float32 CPU RGB tensor in HWC layout"); | |
| } | |
| const int32_t height = shape[0]; | |
| const int32_t width = shape[1]; | |
| const int32_t factor = patch_size_ * merge_size_; | |
| if (height <= 0 || width <= 0 || height % factor != 0 || width % factor != 0) { | |
| throw std::invalid_argument("Qwen3.5 resized image dimensions must be divisible by patch_size * merge_size"); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mllm/models/qwen3_5/image_preprocessor_qwen3_5.hpp` around lines 73 - 83,
Update flattenNormalizedPatches to reject non-contiguous image_hwc tensors
alongside its existing dtype, device, and shape validation. Use
image_hwc.isContiguous(), matching the validation pattern in qwen3_5ExactGelu,
and preserve the existing invalid_argument behavior for invalid inputs.
| Tensor embed(Tensor input_ids) { return embedding_(input_ids); } | ||
|
|
||
| std::vector<Tensor> forwardEmbeddings(Tensor x, Tensor llm_embedding_sin, Tensor llm_embedding_cos, | ||
| const AnyValue& kv_cache) { | ||
| std::vector<AnyValue> args = {kv_cache}; | ||
| return forwardEmbeddingsImpl(x, llm_embedding_sin, llm_embedding_cos, args); | ||
| } | ||
|
|
||
| std::vector<Tensor> forward(const std::vector<Tensor>& inputs, const std::vector<AnyValue>& args) override { | ||
| auto x = embedding_(inputs[0]); | ||
| auto llm_embedding_sin = inputs[1]; | ||
| auto llm_embedding_cos = inputs[2]; | ||
| return forwardEmbeddingsImpl(x, inputs[1], inputs[2], args); | ||
| } | ||
|
|
||
| private: | ||
| std::vector<Tensor> forwardEmbeddingsImpl(Tensor x, Tensor llm_embedding_sin, Tensor llm_embedding_cos, | ||
| const std::vector<AnyValue>& args) { |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect what nn::Module::__main does around forward().
ast-grep run --pattern '__main($$$) { $$$ }' --lang cpp mllm/nn/Module.hpp
rg -nP -C15 '__main' --glob '**/nn/Module.*'Repository: UbiquitousLearning/mllm
Length of output: 161
🏁 Script executed:
#!/bin/bash
set -e
printf 'Tracked nn/ files:\n'
git ls-files 'mllm/nn/*' | sed -n '1,120p'
printf '\nModule-like files:\n'
fd -a 'Module|module' . | sed -n '1,120p'
printf '\nSearch __main occurrences:\n'
rg -n -C5 '__main' --glob '*.hpp' --glob '*.h' --glob '*.cpp' --glob '*.cc' . || true
printf '\nSearch operator() and forward implementations:\n'
rg -n -C6 'struct Module|class Module|operator\(|virtual Tensor forward|Tensor forward\(' --glob '*.hpp' --glob '*.h' --glob '*.cpp' --glob '*.cc' . | sed -n '1,240p'Repository: UbiquitousLearning/mllm
Length of output: 28113
🏁 Script executed:
#!/bin/bash
set -e
printf 'module.cpp __main:\n'
sed -n '100,220p' mllm/nn/Module.cpp
printf '\nmodule.hpp operator/__main declarations:\n'
sed -n '90,230p' mllm/nn/Module.hpp
printf '\nqwen3_5 model section:\n'
sed -n '490,550p' mllm/models/qwen3_5/modeling_qwen3_5.hpp
printf '\ncontext profiling/trace definitions:\n'
rg -n -C7 'trace_mode|thread_tracer|profile|planner|Plan|SubmitTask|TaskTypes::kExecuteModule|send_graph' --glob '*.hpp' --glob '*.cpp' --glob '*.h' mllm | sed -n '1,260p'Repository: UbiquitousLearning/mllm
Length of output: 30221
Route forwardEmbeddings through Module::__main.
llm(...) uses operator(), which calls __main, submitting kGraphBegin/kGraphEnd tasks and either tracing or submitting an execution task. forwardEmbeddings calls forwardEmbeddingsImpl directly, so the image prefill path bypasses that module-level dispatch/tracing bookkeeping. Use __main(...) for this entry path as well.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mllm/models/qwen3_5/modeling_qwen3_5.hpp` around lines 515 - 530, The
forwardEmbeddings entry path currently bypasses module-level dispatch and
tracing. Update Qwen3_5’s forwardEmbeddings method to invoke the module’s __main
dispatch, passing the embedding inputs and kv_cache through it, while preserving
forwardEmbeddingsImpl as the underlying implementation used by the dispatched
call.
| for (int32_t s = 0; s < seq_len; ++s) { | ||
| if (input_ids[s] == video_token_id_ || types[s] == 2) { | ||
| throw std::invalid_argument("Qwen3.5 CPU single-image support does not accept video tokens"); | ||
| } | ||
| if ((input_ids[s] == image_token_id_) != (types[s] == 1)) { | ||
| throw std::invalid_argument("Qwen3.5 image token IDs and modality token types disagree"); | ||
| } | ||
| if (types[s] == 1) { | ||
| if (image_begin < 0) image_begin = s; | ||
| ++image_count; | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reject a non-contiguous image token span.
The loop records image_begin and image_count, but does not verify that the image tokens are contiguous. Line 684 then writes into the slice {image_begin, image_begin + image_count}. For token types such as {1, 0, 1}, that slice covers a text position, and the text embedding is replaced by an image feature.
makeQwen3_5SingleImagePositionIds rejects a split span, but it runs only when position_ids is absent. A caller that supplies both position_ids and pixel_values reaches this code with no contiguity check.
🛡️ Proposed fix
if (types[s] == 1) {
if (image_begin < 0) image_begin = s;
+ if (s != image_begin + image_count) {
+ throw std::invalid_argument("Qwen3.5 supports exactly one contiguous image token span");
+ }
++image_count;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for (int32_t s = 0; s < seq_len; ++s) { | |
| if (input_ids[s] == video_token_id_ || types[s] == 2) { | |
| throw std::invalid_argument("Qwen3.5 CPU single-image support does not accept video tokens"); | |
| } | |
| if ((input_ids[s] == image_token_id_) != (types[s] == 1)) { | |
| throw std::invalid_argument("Qwen3.5 image token IDs and modality token types disagree"); | |
| } | |
| if (types[s] == 1) { | |
| if (image_begin < 0) image_begin = s; | |
| ++image_count; | |
| } | |
| } | |
| for (int32_t s = 0; s < seq_len; ++s) { | |
| if (input_ids[s] == video_token_id_ || types[s] == 2) { | |
| throw std::invalid_argument("Qwen3.5 CPU single-image support does not accept video tokens"); | |
| } | |
| if ((input_ids[s] == image_token_id_) != (types[s] == 1)) { | |
| throw std::invalid_argument("Qwen3.5 image token IDs and modality token types disagree"); | |
| } | |
| if (types[s] == 1) { | |
| if (image_begin < 0) image_begin = s; | |
| if (s != image_begin + image_count) { | |
| throw std::invalid_argument("Qwen3.5 supports exactly one contiguous image token span"); | |
| } | |
| +image_count; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mllm/models/qwen3_5/modeling_qwen3_5.hpp` around lines 664 - 675, Update the
validation loop in the Qwen3.5 single-image path to require all image tokens
marked by types[s] == 1 to form one contiguous span starting at image_begin;
reject any later image token after a non-image token. Perform this check
regardless of whether position_ids is supplied, before the slice written at the
subsequent image-feature assignment.
| if (shape[0] != 3 || shape[1] != 1 || shape[2] <= 0) { | ||
| throw std::invalid_argument("Qwen3.5 cached multimodal positions must have shape [3,1,S]"); | ||
| } | ||
| auto next = Tensor::empty({3, 1, 1}, kInt64, kCPU).alloc(); | ||
| const auto* previous = previous_position_ids.ptr<int64_t>(); | ||
| auto* output = next.ptr<int64_t>(); | ||
| for (int32_t axis = 0; axis < 3; ++axis) { output[axis] = previous[axis * shape[2] + shape[2] - 1] + 1; } | ||
| return next; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Advance all three MRoPE axes from the maximum last position.
The reference MRoPE decode step computes one next position as max(last_t, last_h, last_w) + 1 and assigns it to all three axes. This code advances each axis from its own last value. The two forms agree only when the last prefill token is a text token, because the text tail writes the same value to all axes. If an image span ever ends the sequence, the axes diverge from the reference. The fixed image template currently prevents that case, so this is defensive hardening rather than a live defect.
🛡️ Proposed fix
auto next = Tensor::empty({3, 1, 1}, kInt64, kCPU).alloc();
const auto* previous = previous_position_ids.ptr<int64_t>();
auto* output = next.ptr<int64_t>();
- for (int32_t axis = 0; axis < 3; ++axis) { output[axis] = previous[axis * shape[2] + shape[2] - 1] + 1; }
+ int64_t last = previous[shape[2] - 1];
+ for (int32_t axis = 1; axis < 3; ++axis) { last = std::max(last, previous[axis * shape[2] + shape[2] - 1]); }
+ for (int32_t axis = 0; axis < 3; ++axis) { output[axis] = last + 1; }
return next;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (shape[0] != 3 || shape[1] != 1 || shape[2] <= 0) { | |
| throw std::invalid_argument("Qwen3.5 cached multimodal positions must have shape [3,1,S]"); | |
| } | |
| auto next = Tensor::empty({3, 1, 1}, kInt64, kCPU).alloc(); | |
| const auto* previous = previous_position_ids.ptr<int64_t>(); | |
| auto* output = next.ptr<int64_t>(); | |
| for (int32_t axis = 0; axis < 3; ++axis) { output[axis] = previous[axis * shape[2] + shape[2] - 1] + 1; } | |
| return next; | |
| if (shape[0] != 3 || shape[1] != 1 || shape[2] <= 0) { | |
| throw std::invalid_argument("Qwen3.5 cached multimodal positions must have shape [3,1,S]"); | |
| } | |
| auto next = Tensor::empty({3, 1, 1}, kInt64, kCPU).alloc(); | |
| const auto* previous = previous_position_ids.ptr<int64_t>(); | |
| auto* output = next.ptr<int64_t>(); | |
| int64_t last = previous[shape[2] - 1]; | |
| for (int32_t axis = 1; axis < 3; ++axis) { last = std::max(last, previous[axis * shape[2] + shape[2] - 1]); } | |
| for (int32_t axis = 0; axis < 3; ++axis) { output[axis] = last + 1; } | |
| return next; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mllm/models/qwen3_5/multimodal_qwen3_5.hpp` around lines 181 - 188, Update
the next-position calculation in the cached multimodal position flow to compute
the maximum of the three final axis values, add one, and assign that single
value to every axis in the returned tensor. Keep the existing shape validation
and tensor allocation unchanged.
| static constexpr std::string_view kReservedMarkers[] = { | ||
| "<|vision_start|>", "<|vision_end|>", "<|vision_pad|>", "<|image_pad|>", "<|video_pad|>", | ||
| }; | ||
| for (const auto marker : kReservedMarkers) { | ||
| if (message.prompt.find(marker) != std::string::npos) { | ||
| throw std::invalid_argument("Qwen3.5 prompt must not inject reserved multimodal markers"); | ||
| } | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Also reject the chat-control markers.
kReservedMarkers blocks the vision markers but allows <|im_start|> and <|im_end|>. A prompt that contains <|im_end|> closes the user turn early and can inject a forged assistant or system turn into the template. Add both markers to the list.
🛡️ Proposed fix
static constexpr std::string_view kReservedMarkers[] = {
- "<|vision_start|>", "<|vision_end|>", "<|vision_pad|>", "<|image_pad|>", "<|video_pad|>",
+ "<|im_start|>", "<|im_end|>", "<|vision_start|>", "<|vision_end|>",
+ "<|vision_pad|>", "<|image_pad|>", "<|video_pad|>",
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| static constexpr std::string_view kReservedMarkers[] = { | |
| "<|vision_start|>", "<|vision_end|>", "<|vision_pad|>", "<|image_pad|>", "<|video_pad|>", | |
| }; | |
| for (const auto marker : kReservedMarkers) { | |
| if (message.prompt.find(marker) != std::string::npos) { | |
| throw std::invalid_argument("Qwen3.5 prompt must not inject reserved multimodal markers"); | |
| } | |
| } | |
| static constexpr std::string_view kReservedMarkers[] = { | |
| "<|im_start|>", "<|im_end|>", "<|vision_start|>", "<|vision_end|>", | |
| "<|vision_pad|>", "<|image_pad|>", "<|video_pad|>", | |
| }; | |
| for (const auto marker : kReservedMarkers) { | |
| if (message.prompt.find(marker) != std::string::npos) { | |
| throw std::invalid_argument("Qwen3.5 prompt must not inject reserved multimodal markers"); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mllm/models/qwen3_5/tokenization_qwen3_5.hpp` around lines 359 - 366, Extend
the kReservedMarkers list in the prompt validation loop to include both
"<|im_start|>" and "<|im_end|>", preserving the existing invalid_argument
behavior for any prompt containing reserved multimodal or chat-control markers.
| #include <array> | ||
| #include <cmath> | ||
| #include <stdexcept> | ||
| #include <vector> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Include <algorithm> and <utility> explicitly.
The tests call std::fill, std::copy, and std::equal, which come from <algorithm>, and construct std::pair from <utility>. Neither header is included. The build currently works through transitive includes, which differ between standard libraries and can break on Windows or a different toolchain.
As per coding guidelines: "Ensure code is portable across supported platforms (e.g., Linux, Windows) unless explicitly platform-specific."
🛠️ Proposed fix
+#include <algorithm>
`#include` <array>
`#include` <cmath>
`#include` <stdexcept>
+#include <utility>
`#include` <vector>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #include <array> | |
| #include <cmath> | |
| #include <stdexcept> | |
| #include <vector> | |
| `#include` <algorithm> | |
| `#include` <array> | |
| `#include` <cmath> | |
| `#include` <stdexcept> | |
| `#include` <utility> | |
| `#include` <vector> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/cpu/Qwen35MultimodalTest.cpp` around lines 6 - 9, Update the include
list in Qwen35MultimodalTest to explicitly add the standard headers <algorithm>
for std::fill, std::copy, and std::equal, and <utility> for std::pair, without
relying on transitive includes.
Source: Coding guidelines
| const auto output = qwen3_5ExactGelu(input); | ||
| EXPECT_NEAR(output.ptr<float>()[0], -0.15865526F, 1.0e-7F); | ||
| EXPECT_FLOAT_EQ(output.ptr<float>()[1], 0.0F); | ||
| EXPECT_NEAR(output.ptr<float>()[2], 0.84134477F, 1.0e-7F); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Widen the exact-GELU tolerance.
EXPECT_NEAR(..., 1.0e-7F) is about two float32 ULPs at magnitude 0.84. std::erf(float) results differ by one or two ULPs between libm implementations, for example glibc and Android bionic. The test can fail on a supported target. Use 1.0e-6F, which still detects a wrong activation formula.
💚 Proposed fix
- EXPECT_NEAR(output.ptr<float>()[0], -0.15865526F, 1.0e-7F);
+ EXPECT_NEAR(output.ptr<float>()[0], -0.15865526F, 1.0e-6F);
EXPECT_FLOAT_EQ(output.ptr<float>()[1], 0.0F);
- EXPECT_NEAR(output.ptr<float>()[2], 0.84134477F, 1.0e-7F);
+ EXPECT_NEAR(output.ptr<float>()[2], 0.84134477F, 1.0e-6F);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const auto output = qwen3_5ExactGelu(input); | |
| EXPECT_NEAR(output.ptr<float>()[0], -0.15865526F, 1.0e-7F); | |
| EXPECT_FLOAT_EQ(output.ptr<float>()[1], 0.0F); | |
| EXPECT_NEAR(output.ptr<float>()[2], 0.84134477F, 1.0e-7F); | |
| const auto output = qwen3_5ExactGelu(input); | |
| EXPECT_NEAR(output.ptr<float>()[0], -0.15865526F, 1.0e-6F); | |
| EXPECT_FLOAT_EQ(output.ptr<float>()[1], 0.0F); | |
| EXPECT_NEAR(output.ptr<float>()[2], 0.84134477F, 1.0e-6F); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/cpu/Qwen35MultimodalTest.cpp` around lines 156 - 159, Widen the
EXPECT_NEAR tolerances in the exact-GELU assertions within qwen3_5ExactGelu to
1.0e-6F for the nonzero expected outputs, while leaving the EXPECT_FLOAT_EQ zero
assertion unchanged.
2 participants
What this PR does
Adds an end-to-end Qwen3.5-0.8B single-image multimodal path to the existing
mllm mobile CPU backend:
This PR deliberately targets one production contract: one still image, batch
size 1, Qwen3.5-0.8B. Video, multiple images, MTP, 4B multimodal, and
performance optimization are excluded.
End-to-end demo
What animals are shown in this image? Answer in one short sentence.Qwen3.5-0.8B Multimodal
Two cats are sleeping on a pink bed.
The exact candidate generated 9 tokens on the primary OnePlus 13T after
reverifying the model/artifact identities and both loaded runtime libraries.
Review guide
configuration_qwen3_5.hpp,config_0.8B_multimodal_w4a32_kai.jsonimage_preprocessor_qwen3_5.hppmodeling_qwen3_5_vision.hppmultimodal_qwen3_5.hpp,modeling_qwen3_5.hpp,tokenization_qwen3_5.hppvalidate_*.py,main.cpp,Qwen35MultimodalTest.cppSuggested review order: model contract → preprocessing → vision tower →
multimodal integration → conversion/tests.
Supported production contract
11/11/10)The runner rejects images with a text-only config or benchmark mode and checks
the runtime/model contract before model construction.
Current-head validation
Candidate HEAD:
f10ea711e27c53a6c4505686951897c71f4376e7Merged-main base:
9a0a21ded8567076c37edb17f91f639a031500a3arm64-v8aRelease build plus ELF, Build ID, dependency, symbol, and deployment-manifest audits passedFull validation matrix
TOKEN_ID:3833,One), preserving the #690 pathTOKEN_ID:332)--help, and negative image/config/benchmark casesOnePlus/PKX110, UID 10388, Android 16/API 36; manifest-bound deployment; 39 RUN / 38 PASS / 1 expected macOS-only skip / 0 failTOKEN_ID:332; 9-token demo returnedTwo cats are sleeping on a pink bed.TOKEN_ID:332smokeThe semantic oracle is pinned to Transformers commit
dfff6dc70d3fffadf539353743a9e176af8109e9. It proves the geometry and layoutcontracts, not bitwise equality with PIL bicubic interpolation. Device smoke
and the demo are end-to-end integration evidence, not formal image-quality,
numerical-parity, or performance claims.
Implementation and quantization notes
dense vision attention, VisionRoPE, and the exact-GELU merger.
replaces only that span with projected visual embeddings.
three axes consistently during decode.
offsets, and the absence of unsupported MTP tensors.
every prompt.
The user-facing quantization contract is W4A8: KAI Linear dynamically
quantizes activations to INT8 and uses packed INT4 weights with FP32 operator
inputs/outputs. Existing
w4a32_kainames remain unchanged for toolingcompatibility. Vision Conv3D, learned positions, LayerNorm, embeddings,
recurrent parameters, and convolution weights remain FP32.
How to use it
examples/qwen3_5/README.mddocuments checkpoint validation, multimodalconversion, desktop/Android builds, and runner invocation.
Known limits
Tracks #644.
Extends the Qwen3.5-0.8B and 4B mobile CPU support merged in #690 and #691,
respectively, and is based on the merged GDN/benchmark work in #693.
Summary by CodeRabbit
New Features
--image_path.Documentation
Tests