CARVIEW |
Navigation Menu
-
-
Notifications
You must be signed in to change notification settings - Fork 56.2k
Fix for OpenVINO 2024.0. OpenVINO 2022.1 as minimal supported version. #25199
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
7675ec1
to
0fabe2b
Compare
Thanks, @olpipi! Our CI uses OpenVINO 2023.0.2. Can you determine on which minimal version |
Hi |
Please note that as a part of regular CI we test 2021.4.2 release (OpenVINO API 1.0): https://pullrequest.opencv.org/buildbot/builders/4_x_openvino-opencl-skl-lin64/builds/100055 If there is no namespace ov = ngraph; |
@dkurt OpenVINO 2021.4.2 has neither ov namespace nor "openvino/..." include pathes. So I'll fix it as you suggested. Can we somehow test it with 2024.0 release as well, except my local testing? |
@@ -55,11 +55,7 @@ | |||
|
|||
#ifdef HAVE_DNN_NGRAPH |
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.
HAVE_DNN_NGRAPH => HAVE_DNN_OPENVINO ?
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 cost a lot of changes as well as DNN_BACKEND_INFERENCE_ENGINE -> DNN_BACKEND_OPENVINO.
@@ -9,6 +9,12 @@ | |||
#include <opencv2/core/utils/configuration.private.hpp> | |||
#include <opencv2/core/utils/logger.hpp> | |||
|
|||
#include "op_inf_engine.hpp" | |||
|
|||
#ifdef HAVE_INF_ENGINE |
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.
#ifdef HAVE_INF_ENGINE | |
#ifdef HAVE_OPENVINO |
modules/dnn/src/net_openvino.cpp
Outdated
@@ -864,15 +804,8 @@ Net openvino_readNetwork( | |||
InferenceEngine::CNNNetwork ieNet; |
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.
it's removed in API 2.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.
This is a workaround to wrap ov::Model
from API 2.0: https://github.com/opencv/opencv/pull/25199/files#r1524623439
@@ -10,7 +10,7 @@ | |||
#include <opencv2/dnn/shape_utils.hpp> | |||
|
|||
#ifdef HAVE_INF_ENGINE |
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.
#ifdef HAVE_INF_ENGINE | |
#ifdef HAVE_OPENVINO |
@@ -28,6 +28,7 @@ | |||
#define INF_ENGINE_RELEASE_2021_4 2021040000 | |||
#define INF_ENGINE_RELEASE_2022_1 2022010000 | |||
#define INF_ENGINE_RELEASE_2023_0 2023000000 | |||
#define INF_ENGINE_RELEASE_2024_0 2024000000 |
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.
#define INF_ENGINE_RELEASE_2024_0 2024000000 | |
#define OPENVINO_RELEASE_2024_0 2024000000 |
and can we remove old ones?
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.
Let’s remove old ones, but keep IE notation to reduce changes.
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.
done
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.
Great job, many thanks!
Hi @dkurt. |
@olpipi, Need a comment from @opencv-alalek regarding https://pullrequest.opencv.org/buildbot/builders/4_x_openvino-opencl-skl-lin64/ (uses 2021.4.2 OpenVINO). Is that critical if we merge this PR to 4.x before CI upgrade? |
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.
Well done 👍
@olpipi Please squash commits into one before merge
OpenVINO builder on legacy CI will be upgraded later.
Remove support OpenVINO lower than 2022.1 release Remove legacy InferenceEngine wrappers
9e9b879
to
6da2ddc
Compare
Done. Squashed and rebased |
After OpenCV's 4.10.0 release including this patch, we tried to upgrade to OpenVINO 2024, too. However, compiling OpenVINO 2024.2.0 with basically only the CPU plugin, linking OpenCV against it and using
ĂŚt works without any issues if either OpenCV's backend is used or the OpenVINO's 2024 release is replaced by 2023 (we tested 2023.3.0), i.e. it is directly caused by using OpenVINO's 2024 release. We also tested different models, the error is always the same. Any ideas what's causing it? |
@CSBVision could you file a bug with small reproducer and particular OpenVINO versions. |
Sure, no problem, cf. #25916 |
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.