-
Notifications
You must be signed in to change notification settings - Fork 56
CVS-174235 : OV SDK Version Check Removed for EpCtx Consumption Flow on NPU #837
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@ankitm3k As per the report in this ticket https://jira.devtools.intel.com/browse/CVS-174362 the error message from OV plugin for CPU and GPU is not specific to version mismatch . Is it okay to just remove the existing check ? I think we should come up with a custom throw based on OV plugins error. Something like " recompile model with appropriate OV version " |
The requirement for this activity to make the import flow OV version agnostic for the EPCtx blobs which is fulfilled in this PR. |
8325a2f to
40544c5
Compare
0382e7a to
6de292f
Compare
6de292f to
67d4087
Compare
| // If the model stream is not an XML (i.e. precompiled blob), the OpenVINO SDK version that it was | ||
| // exported with must match the version that is currently running. | ||
| native_blob_path = std::move(blob_filepath); | ||
| ORT_ENFORCE((attrs.count(EP_SDK_VER) == 1) && (attrs.at(EP_SDK_VER).s() == openvino_sdk_version_), |
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.
Can we specifically remove this check for NPU device only for now?
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.
fixed as requested
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.
Thanks @ankitm3k . Functionally the change looks good and meets the requirement.
Unfortunately, the side effect of this device specific behavior is to pass the session context to ctx_helper. Is there any way to avoid. Should we just pass device_type?
@javier-intel , @ericcraw, @preetha-intel .. any suggestions?
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.
I have amended the change to accept std::string device_type to keep things simple & clean
b98ffde to
20e3420
Compare
|
@vthaniel please validate this on priority as requested by @MayureshV1 |
MayureshV1
left a comment
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.
Thanks Ankit. The changes look good now.
0d00c57 to
e51c5e0
Compare
Description
This PR removes the OV SDK Version Check for EpCtx flow for pre-compiled blobs import on NPU.
Note: CPU/ GPU still have the SDK version check.
https://jira.devtools.intel.com/browse/CVS-174235