- Notifications
You must be signed in to change notification settings - Fork227
Fix AddOliveMetadata pass to preserve external data files#1974
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
f4ca5c4
to82285a9
CompareRefactored the AddOliveMetadata ONNX pass to ensure all externaldata files (e.g., .bin, .xml) are preserved when adding metadata to ONNXmodels.Key changes:- Always copy entire input directory to output location to preserve all external files alongside the main ONNX model- Simplified pass logic by treating all models as directory-based- Load ONNX models without external data to preserve file structure- Return directory-based ONNXModelHandler for consistent behaviorAdded comprehensive unit tests:- test_add_metadata_with_external_data_files: Verifies external file preservation for directory-based models with mock .bin/.xml- test_add_metadata_single_file_to_directory_conversion: Ensures correct metadata handling for single-file ONNX modelsThis fix addresses issues where external data files were lost duringmetadata addition, ensuring model integrity for ONNX/OpenVINO modelswith external dependencies.
a5f1540
toa0f83c1
CompareUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
onnx_files = list(input_model_path.glob("*.onnx")) | ||
if not onnx_files: | ||
raise ValueError(f"No ONNX file found in model directory: {input_model_path}") | ||
input_onnx_file = onnx_files[0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Will this work for multi-modal scenario?
It would be safer to use only if a single modal is found. In other cases throw a relevant error and ask the user to explicitly provider a model file name.
ashrit-msJul 18, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Just want to confirm if you referring to multi mod'e'l scenarios or multi mod'a'l scenarios?
For multi model scenarios, Olive automatically applies the pass to each component of a composite model.
Uh oh!
There was an error while loading.Please reload this page.
Describe your changes
Refactored the AddOliveMetadata ONNX pass to ensure all external data files (e.g., .bin, .xml) are preserved when adding metadata to ONNX models.
Key changes:
Added unit tests:
This fix addresses issues where external data files were lost during metadata addition, ensuring model integrity for ONNX/OpenVINO models with external dependencies.