Skip to content

G-API: Support OpenVINO Execution Provider for ONNXRT Backend#24024

Merged
asmorkalov merged 4 commits intoopencv:4.xfrom
TolyaTalamanov:at/add-onnx-openvino-execution-provider
Jul 21, 2023
Merged

G-API: Support OpenVINO Execution Provider for ONNXRT Backend#24024
asmorkalov merged 4 commits intoopencv:4.xfrom
TolyaTalamanov:at/add-onnx-openvino-execution-provider

Conversation

@TolyaTalamanov
Copy link
Copy Markdown
Contributor

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

@TolyaTalamanov TolyaTalamanov force-pushed the at/add-onnx-openvino-execution-provider branch from bbc17ed to b817d4d Compare July 19, 2023 14:23
options.num_of_threads = *ovep.num_of_threads;
}

session_options->AppendExecutionProvider_OpenVINO(options);
Copy link
Copy Markdown
Contributor Author

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: "
Copy link
Copy Markdown
Contributor Author

@TolyaTalamanov TolyaTalamanov Jul 19, 2023

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.

@TolyaTalamanov
Copy link
Copy Markdown
Contributor Author

@dmatveev Could you have a look, please?

@TolyaTalamanov
Copy link
Copy Markdown
Contributor Author

The open question is how it can be tested because it requires onnxruntime to be built with --use_openvino that enables OpenVINO Execution Provider otherwise AppendExecutionProvider_OpenVINO will fail with exception

@dmatveev dmatveev self-assigned this Jul 20, 2023
Copy link
Copy Markdown
Contributor

@dmatveev dmatveev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

`Generally looks good

void run();
};

static void appendExecutionProvider(Ort::SessionOptions *session_options,
Copy link
Copy Markdown
Contributor

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?

Copy link
Copy Markdown
Contributor Author

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.

Copy link
Copy Markdown
Contributor Author

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.");
Copy link
Copy Markdown
Contributor

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).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an option, yes

Comment on lines +176 to +177
default:
break;
Copy link
Copy Markdown
Contributor

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?

Copy link
Copy Markdown
Contributor Author

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

@asmorkalov
Copy link
Copy Markdown
Contributor

Tested manually with openvino_toolkit_ubuntu18_2022.1.1.7030.39aba80957e.

@asmorkalov asmorkalov merged commit 5261961 into opencv:4.x Jul 21, 2023
@asmorkalov asmorkalov mentioned this pull request Jul 27, 2023
thewoz pushed a commit to thewoz/opencv that referenced this pull request Jan 4, 2024
…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
thewoz pushed a commit to thewoz/opencv that referenced this pull request May 29, 2024
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants