[OpenVINO] fix: Respect trust_remote_code in export and improve safety for multimodal configs FIXES #1668#1673
Conversation
MissLostCodes
changed the title
trust_remote_code in OpenVINO export and improve safety for multimodal configstrust_remote_code in OpenVINO export and improve safety for multimodal configs
MissLostCodes
changed the title
trust_remote_code in OpenVINO export and improve safety for multimodal configstrust_remote_code in export and improve safety for multimodal configs
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
|
@echarlaix @popovaan @rkazants From the failing CI logs, it appears that This fix introduces:
This fix ensures:
After these changes, the tests that were previously failing due to incorrect propagation of |
MissLostCodes
changed the title
trust_remote_code in export and improve safety for multimodal configstrust_remote_code in export and improve safety for multimodal configs FIXES ISSUE #1668
|
@echarlaix @rkazants @popovaan |
MissLostCodes
changed the title
trust_remote_code in export and improve safety for multimodal configs FIXES ISSUE #1668trust_remote_code in export and improve safety for multimodal configs FIXES #1668
|
@echarlaix @popovaan @rkazants
|
|
@rkazants @popovaan @echarlaix
Here's the detailed test result notebook |
There was a problem hiding this comment.
Pull request overview
This PR updates the OpenVINO export flow to consistently honor trust_remote_code (to prevent unintended remote code execution) and adds defensive handling for certain multimodal configs during export.
Changes:
- Propagate
trust_remote_codethrough key OpenVINO export entry points and multimodal export config construction. - Make multimodal config handling more robust (e.g., guard access to
mm_vision_tower). - Add an early warning in task inference when
trust_remote_code=Falsefor Transformers models.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
optimum/exporters/openvino/__main__.py |
Adds a warning when remote code trust is disabled and ensures trust_remote_code is passed to AutoConfig.from_pretrained. |
optimum/exporters/openvino/convert.py |
Threads trust_remote_code into submodel/export-config selection, and conditionally passes it into relevant multimodal config constructors. |
optimum/exporters/openvino/model_configs.py |
Stops hardcoding trust_remote_code=True for multimodal vision-tower config loads; adds defensive hasattr checks. |
tests/openvino/test_genai.py |
Updates the set of architectures treated as requiring remote code in GenAI tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if export_model_type in {"llava-qwen2", "phi3_v"}: | ||
| config_kwargs["trust_remote_code"] = trust_remote_code | ||
|
|
There was a problem hiding this comment.
Why is trust_remote_code flag propagated here only for "llava-qwen2" and "phi3_v"? Maybe we need to do it unconditionally?
There was a problem hiding this comment.
other models do require trust_remote_code at the Transformers level but their corresponding OpenVINO config classes do not accept this parameter yet . So the condition is required
propagating trust_remote_code unconditionally, leads to :
TypeError: Qwen2VLOpenVINOConfig.__init__() got an unexpected keyword argument 'trust_remote_code'
TypeError: InternVLChatOpenVINOConfig.__init__() got an unexpected keyword argument 'trust_remote_code'
TypeError: MiniCPMVOpenVINOConfig.__init__() got an unexpected keyword argument 'trust_remote_code'
| "xglm", | ||
| "granitemoe", | ||
| "granite", | ||
| "llama4", | ||
| "zamba2", |
There was a problem hiding this comment.
why these changes are here? Looks not correct merge conflicts resolution
Please add tests to show that without 'trust_rmote_code' export of models do not work for certain models.
This test should check that error is raised for such models exporting wihtout this option. Quite useful test
Summary
This PR ensures that the trust_remote_code flag is consistently respected across the OpenVINO export pipeline and improves robustness when handling multimodal model configurations.
Motivation
Some multimodal models (e.g. LLaVA, Phi-3 Vision, Qwen-VL) rely on custom code from their Hugging Face repositories.
Previously:
trust_remote_codewas not consistently propagatedA warning is added in
infer_task()whentrust_remote_code=False: This informs users that the model may require remote code appears before Transformers raises an exceptionDefensive handling of multimodal configs
Added check:
hasattr(config, "mm_vision_tower")Reason :
Behavior
Without --trust-remote-code
Output :
This model may require executing custom code from its repository. For security reasons, this is disabled by default. Please review the source and rerun with
--trust-remote-codeif needed.With --trust-remote-code
Output:
testing_colab_notebook
All tests relevant tests were passed
Docs already include related info for flag... no need to change
Fixes #1668