Repository navigation
Feature/openvino model package - #2717
Preetha Veeramalai (preetha-intel) wants to merge 4 commits into
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved path-safety, registration-lifecycle, and package-completeness issues can cause unsafe or unusable output.
Review effort: Balanced
Findings: 2
Open (8)
Protect shared registrations from premature unregistration · New Prevent path traversal in external-data staging · New Use the correct weightless option for supported ORT versions · New Reject missing requested device IDs unless partial packaging is enabled · New Reject duplicate generated variant names · New Handle weights when shared-asset staging is disabled · New Copy EPContext binaries using recorded relative paths · New Traverse all subgraph and attribute tensors for external data · New
What changed in this PR
Adds OpenVINO model packaging for ONNX Runtime’s OrtModelPackageApi, with per-device variants and shared weights.
Changes:
- Supports compilation and packaging of precompiled models.
- Extends EPContext generation with OpenVINO plugin support.
- Adds pass registration, configuration examples, documentation, and tests.
| File | Description |
|---|---|
test/passes/openvino/test_openvino_model_package.py |
Tests packaging and registration behavior. |
olive/passes/openvino/model_package.py |
Implements compilation, asset staging, and manifests. |
olive/passes/openvino/examples/model_package_config_template.json |
Provides an example workflow. |
olive/passes/onnx/context_binary.py |
Adds OpenVINO compilation and plugin registration handling. |
olive/olive_config.json |
Registers the pass and supported providers. |
docs/README_OpenVINO.md |
Documents configuration, usage, and package format. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Mayuresh Varerkar (MayureshV1)
left a comment
There was a problem hiding this comment.
Preetha Veeramalai (@preetha-intel) . Can you please fix the below suggestions.
Also, overall guidance was to hold off merging this PR till Model Packaging support becomes a stable ABI, which is expected to be soon.
| if version.parse(OrtVersion).release <= version.parse("1.23.2").release: | ||
| assert self.accelerator_spec.execution_provider in get_available_providers(), ( | ||
| f"Execution provider {self.accelerator_spec.execution_provider} is not available. Available providers:" | ||
| f" {get_available_providers()}" | ||
| ) |
There was a problem hiding this comment.
do you need this check for your logic?
There was a problem hiding this comment.
No — this check isn't needed for the OpenVINO logic. The OpenVINO path returns via _run_openvino before this block, so it never executes it. The check is existing QNN code, unchanged from main, so I've left it as is. Happy to have the QNN owners decide whether it's still needed.
There was a problem hiding this comment.
what i mean is does openvino need if version.parse(OrtVersion).release <= version.parse("1.23.2").release: this ort version check?
There was a problem hiding this comment.
Xiaoyu Z (@xiaoyu-work) The <= 1.23.2 check is QNN-only (existing code, unchanged from main, now in _run_qnn). OpenVINO doesn't need it: _run_openvino returns before this block. The OpenVINO minimum (ORT >= 1.30) is enforced separately in OpenVINOModelPackage._check_ort_version for the model-package pass.
| } | ||
|
|
||
|
|
||
| class OpenVINOModelPackage(Pass): |
There was a problem hiding this comment.
i'm a little bit confused about this pass. we have a CLI generate-model-package to generate a model package. I'd like to split this into 2 steps: EPContextBinaryGenerator and generate model package. the generating model package logic should be generic and all other ep can use it as well.
There was a problem hiding this comment.
does this pass only create npu package? if we want to add support for cpu, gpu as well to ort-package. will this pass work? that's why i'm thinking having a generic packaging logic will be better here.
There was a problem hiding this comment.
The CLI generate-model-package is a GenAI specific model packaging which manually creates the manifest.json and component.json. But the OpenVINOModelPackage builds the package through ORT's ort.ModelPackage API. ORT owns the manifest, component and variant metadata, and the shared-asset hashing and URI.
I agree the packaging should be generic, and it already is in the pass. The ort.ModelPackage calls don't depend on OpenVINO, and only discovery and compile are OpenVINO-specific. Splitting it by moving the ORT-API packaging out of this pass can be done as the follow up once other EP's also want to accomodate the Olive but the CLI's GenAI packaging and the pass's ORT packaging are two different formats, so it can't be one generic CLI path without making it a different mode.
|
I've moved the context binary logic into #2732. can you check this pr? |
9bc198e to
a68d4b7
Compare
a68d4b7 to
b67a720
Compare


Adds an Olive pass that compiles an ONNX model into per-device OpenVINO NPU EPContext variants and assembles an ONNX Runtime .ortpackage. Weights are stored once as a shared asset and referenced by every variant.