Repository navigation
Add pybindings for Model Packaging - #33640
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. |
Mayuresh Varerkar (MayureshV1)
left a comment
Mayuresh Varerkar (MayureshV1)
left a comment
There was a problem hiding this comment.
Thanks Preetha Veeramalai (@preetha-intel), this looks good. I checked the bindings against model_package.h and the model package C API: the string and info-tree lifetimes are handled correctly (everything is copied before the next mutation), the OrtSession* adoption matches what CreateSession returns, and the session-options snapshot is only used while ModelPackageOptions is being constructed. The tests cover authoring → reopen → select → create_session end to end, plus releasing the context before the component.
I left a few suggestions inline. The first one (the type create_session returns) is probably worth settling before this ships, since public Python API shapes are hard to change later. I'll approve once you've had a chance to respond.
Note: the Azure pipelines haven't run yet. A maintainer will need to trigger /azp run.
|
All review feedback has been implemented and verified locally (CPU build with pybind, model package tests passing), hence approving. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Xiaoyu Z (@xiaoyu-work) I have added the lint fixes. Can you help retriggering the pipelines |
|
The wasm_debug build failure is not related to this PR code changes. Its failing because the wasm runtime couldnt access the mounted directory. |
|
Please help merge current main to include the fix for Web CI failure. |
|
Rechecked head The remaining failure is Web CI / wasm_Debug, Test (Node.js) (simd + threads), currently run attempt 6. Three This is a WASM external-file test/harness compatibility failure, not a transient directory-permission or network failure. It needs a harness/test adjustment or an upstream fix; another unchanged rerun is not appropriate. I have not queued a rerun. |
50320ae to
e19af2e
Compare
|
I have rebased with main for latest upstream changes |
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved synchronization gaps allow concurrent operations to access partially initialized session caches or freed package handles, creating critical memory-safety risks.
4 open issues
What changed in this PR
Adds Python access to ONNX Runtime’s model package APIs for authoring packages, inspecting and selecting variants, and creating inference sessions.
Changes:
- Exposes model package classes in non-minimal builds.
- Wraps package-created sessions as Python
InferenceSessionobjects, with unsupported recreation operations blocked. - Adds
ModelCompiler.set_ep_context_binary_informationand tests for the new APIs.
| File | Description |
|---|---|
onnxruntime/test/python/onnxruntime_test_python_model_package.py |
Adds authoring, selection, inference, and lifetime tests. |
onnxruntime/test/python/onnxruntime_test_python_compile_api.py |
Tests EPContext binary configuration and invalid arguments. |
onnxruntime/python/onnxruntime_pybind_state.cc |
Registers package bindings and the compiler setter. |
onnxruntime/python/onnxruntime_pybind_state_common.h |
Supports adopting initialized native sessions. |
onnxruntime/python/onnxruntime_pybind_model_package.cc |
Implements package authoring and consumption bindings. |
onnxruntime/python/onnxruntime_pybind_model_compiler.h |
Declares the EPContext binary configuration setter. |
onnxruntime/python/onnxruntime_pybind_model_compiler.cc |
Implements the setter with compilation-state protection. |
onnxruntime/python/onnxruntime_inference_collection.py |
Wraps package sessions and exposes the compiler setter. |
onnxruntime/__init__.py |
Exports the new package classes when available. |
cmake/onnxruntime_python.cmake |
Links the model package library into the Python module. |
Review effort: Balanced
| explicit BusyScope(PyModelPackage& owner) : owner_(owner) { owner_.busy_ = true; } | ||
| ~BusyScope() { owner_.busy_ = false; } |
| py::gil_scoped_release release; | ||
| OrtPybindThrowIfError(ToStatusAndRelease(Ort::GetModelPackageApi().CreateSession( | ||
| GetOrtEnv(), self, options ? &snapshot.options : nullptr, &result))); |
| std::lock_guard<std::mutex> lock{compilation_mutex_}; | ||
| ORT_RETURN_IF(compilation_in_progress_, | ||
| "Cannot change the EPContext binary information while compilation is in progress."); | ||
| return model_compile_options_.SetEpContextBinaryInformation(output_directory, model_name); |
| if __name__ == "__main__": | ||
| unittest.main() |
Description
This PR adds Python bindings for the Model Package APIs. Python applications can now create, inspect and edit model packages, and select a variant and create an inference session from one. It also exposes set_ep_context_binary_information on ModelCompiler.
Changes
Motivation and Context
The python API's are helpful in creating sample applications for public adoptions.