fix: genai config gaps - decoder path, image_processor.json, spatial_merge_size - #204
Conversation
…merge_size 1. decoder.filename: "decoder/model.onnx" for multi-model packages 2. vision.config_filename: "image_processor.json" (matches ort-extensions) 3. vision.spatial_merge_size: 2 (from config) 4. Update reference config in examples/ Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Justin Chu <justinchu@microsoft.com>
Performance Comparison
|
The config_filename is read from genai_config.json at runtime — ORT GenAI/extensions don't hardcode the filename. Keep the existing processor_config.json convention for consistency across all VLMs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Justin Chu <justinchu@microsoft.com>
There was a problem hiding this comment.
Pull request overview
Fixes ORT GenAI export/config generation gaps for Gemma4 multimodal packages so generated artifacts match the known-good reference layout and filenames.
Changes:
- Update genai_config generation for multimodal packages (decoder filename path) and enrich Gemma4 vision config fields (
config_filename,spatial_merge_size). - Adjust auto-export processor config writing to emit
image_processor.jsonfor Gemma4 (ORT-extensions convention). - Update tests and example Gemma4 ORT-GenAI configs to match the new filenames/fields.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/mobius/integrations/ort_genai/genai_config.py | Adjusts decoder filename selection for multimodal genai_config generation. |
| src/mobius/integrations/ort_genai/auto_export.py | Writes Gemma4 image_processor.json and includes Gemma4 spatial_merge_size/config_filename in genai_config generation. |
| src/mobius/integrations/ort_genai/auto_export_test.py | Updates Gemma4-related assertions to validate spatial_merge_size and image_processor.json. |
| examples/gemma4/ort_genai/vlm/image_processor.json | Adds the renamed Gemma4 processor config example file. |
| examples/gemma4/ort_genai/vlm/genai_config.json | Updates example genai_config to reference decoder/model.onnx, image_processor.json, and spatial_merge_size. |
Comments suppressed due to low confidence (1)
src/mobius/integrations/ort_genai/auto_export.py:195
- This function’s docstring mentions writing
processor_config.json, but the implementation now writesimage_processor.jsonfor Gemma4 andprocessor_config.jsonotherwise. Please update the docstring to reflect that the output filename varies by model type (and, if callers rely on the name, consider returning the chosen filename explicitly).
else:
processor = {
"image_size": getattr(vision, "image_size", None) or 448,
| decoder_filename = "decoder/model.onnx" if is_multimodal else "model.onnx" | ||
| decoder: dict[str, Any] = { | ||
| "session_options": _make_session_options(self.ep), | ||
| "filename": "model.onnx", | ||
| "filename": decoder_filename, | ||
| "head_size": self.head_dim, |
There was a problem hiding this comment.
For multimodal packages this hardcodes the decoder path to decoder/model.onnx, but not all multi-model exports save the decoder under a decoder/ subfolder. For example, Phi4MMMultiModalTask stores the decoder under the component key model, so ModelPackage.save() writes it to model/model.onnx. This will generate a genai_config that points at a non-existent file for those packages.
Consider making the decoder filename configurable (e.g., a decoder_filename parameter on GenaiConfigGenerator), and set it in write_ort_genai_config() based on which decoder component key is present in the ModelPackage (e.g., prefer decoder/model.onnx if "decoder" in pkg else model/model.onnx if "model" in pkg). Adding a regression test for the phi4mm export path would prevent this from reoccurring.
| vision_kwargs["spatial_merge_size"] = None | ||
| vision_kwargs["config_filename"] = "vision_processor.json" | ||
| elif model_type in ("gemma4", "gemma4_text"): | ||
| vision_kwargs["spatial_merge_size"] = None | ||
| vision_kwargs["config_filename"] = "processor_config.json" | ||
| vision_cfg = getattr(config, "vision", None) | ||
| sms = getattr(vision_cfg, "spatial_merge_size", 2) | ||
| vision_kwargs["spatial_merge_size"] = sms | ||
|
|
||
| if vision_input_mapping is not None: | ||
| vision_kwargs["input_names"] = vision_input_mapping |
There was a problem hiding this comment.
genai_config.json can reference vision_processor.json when has_speech is true, but _write_processor_config() never writes that filename (it writes either processor_config.json or image_processor.json). This makes the generated artifacts inconsistent: ORT-GenAI will try to load a processor file that wasn't exported.
A concrete fix is to have processor-config writing share the same filename selection logic as genai-config generation (or write an additional copy under the referenced name when has_speech is set).
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Justin Chu <justinchu@microsoft.com>
Fix 4 gaps vs the reference working Gemma4 genai config:
"decoder/model.onnx"for multi-model VLM packages (was"model.onnx")"image_processor.json"for Gemma4 (matches ort-extensions convention)Also renames
examples/gemma4/ort_genai/vlm/processor_config.json→image_processor.json.