| CARVIEW |
Navigation Menu
-
-
Notifications
You must be signed in to change notification settings - Fork 56.2k
G-API: Support OpenVINO Execution Provider for ONNXRT Backend #24024
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
G-API: Support OpenVINO Execution Provider for ONNXRT Backend #24024
Conversation
TolyaTalamanov
force-pushed
the
at/add-onnx-openvino-execution-provider
branch
from
bbc17ed to
b817d4d
Compare
| options.num_of_threads = *ovep.num_of_threads; | ||
| } | ||
|
|
||
| session_options->AppendExecutionProvider_OpenVINO(options); |
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.
This thing might throw, make sense to wrap it and show the understandable error message...
| session_options->AppendExecutionProvider_OpenVINO(options); | ||
| } catch (const std::exception &e) { | ||
| std::stringstream ss; | ||
| ss << "ONNX Backend: Failed to enable OpenVINO Execution Provider: " |
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.
Usually throw when onnxruntime_providers_openvino can't be found.
|
@dmatveev Could you have a look, please? |
|
The open question is how it can be tested because it requires |
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.
`Generally looks good
| @@ -143,6 +143,41 @@ class ONNXCompiled { | |||
| void run(); | |||
| }; | |||
|
|
|||
| static void appendExecutionProvider(Ort::SessionOptions *session_options, | |||
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.
y append, not just add?
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.
hmm, you actually make a good point, what if it's possible to use multiple execution providers at the same time...
In that case I'd probably need to hold the list of them.
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.
| namespace ep = cv::gapi::onnx::ep; | ||
| switch (execution_provider.index()) { | ||
| case ep::EP::index_of<ep::OpenVINO>(): { | ||
| GAPI_LOG_INFO(NULL, "OpenVINO Execution Provider is selected."); |
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.
Maybe it makes sense to display for which network (if it is not mentioned in the log earlier).
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.
As an option, yes
| default: | ||
| break; |
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.
Any message for this case?
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 think some sort of log would be useful
|
Tested manually with openvino_toolkit_ubuntu18_2022.1.1.7030.39aba80957e. |
…ino-execution-provider G-API: Support OpenVINO Execution Provider for ONNXRT Backend opencv#24024 ### Pull Request Readiness Checklist See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request - [ ] I agree to contribute to the project under Apache 2 License. - [ ] To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV - [ ] The PR is proposed to the proper branch - [ ] There is a reference to the original bug report and related work - [ ] There is accuracy test, performance test and test data in opencv_extra repository, if applicable Patch to opencv_extra has the same branch name. - [ ] The feature is well documented and sample code can be built with the project CMake
…ino-execution-provider G-API: Support OpenVINO Execution Provider for ONNXRT Backend opencv#24024 ### Pull Request Readiness Checklist See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request - [ ] I agree to contribute to the project under Apache 2 License. - [ ] To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV - [ ] The PR is proposed to the proper branch - [ ] There is a reference to the original bug report and related work - [ ] There is accuracy test, performance test and test data in opencv_extra repository, if applicable Patch to opencv_extra has the same branch name. - [ ] The feature is well documented and sample code can be built with the project CMake
Pull Request Readiness Checklist
See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request
Patch to opencv_extra has the same branch name.