Skip to content

No-op: onnxruntime-genai#2188 is C++ runtime only, nothing to propagate to the Python builder - #383

Closed
xadupre with Copilot wants to merge 1 commit into
mainfrom
copilot/xadupre-373-propagate-https
Closed

No-op: onnxruntime-genai#2188 is C++ runtime only, nothing to propagate to the Python builder#383
xadupre with Copilot wants to merge 1 commit into
mainfrom
copilot/xadupre-373-propagate-https

Conversation

Copilot AI commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

The task was to propagate onnxruntime-genai#2188 ("Fix Gemma4 multimodal processing") into mbext. After analyzing the upstream diff, no changes are required: the PR modifies only the C++ runtime, which mbext does not mirror.

Findings

  • PR #2188 touches 7 C++ files onlysrc/config.{cpp,h}, src/models/embeddings.{cpp,h}, src/models/multi_modal.{cpp,h}, src/models/gemma4_multimodal_processor.cpp. No builder.py / Python changes.
  • mbext mirrors only the Python model builder (src/python/py/models/modelbuilder/), per the README. The three upstream fixes are all runtime-level:
    • pixel_values float32-stride trim (NaN fix) — C++ memcpy.
    • pixel_position_ids extraction — C++ ToBeAssigned() temporary-lifetime bug.
    • per_layer_inputs embedding→decoder forwarding — C++ pipeline wiring + genai_config schema reads.
  • per_layer_inputs exists nowhere in the upstream Python builder (GitHub code search returns only the C++ files). The runtime support landed ahead of any builder, so there is no Python layout that emits it to propagate.
  • Gemma4 PLE is already implemented in modelbuilder/builders/gemma.py (_make_ple_pre_computation / _make_ple_layer_block), computed internally in a monolithic decoder and covered by existing tests/fast/test_random_gemma4.py PLE tests. It does not depend on the new runtime per_layer_inputs boundary.

Recommendation

Close the tracking issue as not applicable to the Python builder.

A separate, larger effort could add decoder-side per_layer_inputs as a model input under --exclude_embeds (with decoder.inputs.per_layer_inputs / embedding.outputs.per_layer_inputs in genai_config), but that would put mbext ahead of upstream rather than propagating #2188, and there is no Gemma4 embedding-model producer or test harness here to validate it. Happy to scope that as a follow-up if desired.

Copilot AI changed the title [WIP] Propagate changes from pull request #2188 No-op: onnxruntime-genai#2188 is C++ runtime only, nothing to propagate to the Python builder Jun 17, 2026
Copilot AI requested a review from xadupre June 17, 2026 09:32
@xadupre xadupre closed this Jun 17, 2026
@xadupre
xadupre deleted the copilot/xadupre-373-propagate-https branch June 17, 2026 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants