G-API: Integration branch for ONNX & Python-related changes by dmatveev · Pull Request #23597 · opencv/opencv

dmatveev · 2023-05-09T19:33:29Z

Changes overview

1. Expose ONNX backend's Normalization and Mean-value parameters in Python

Since Python G-API bindings rely on Generic infer to express Inference, the Generic specialization of onnx::Params was extended with new methods to control normalization (/255) and mean-value; these methods were exposed in the Python bindings
Found some questionable parts in the existing API which I'd like to review/discuss (see comments)

UPD:

Thanks to @TolyaTalamanov normalization inconsistencies have been identified with squeezenet1.0-9 ONNX model itself; tests using these model were updated to DISABLE normalization and NOT using mean/value.
Questionable parts were removed and tests still pass.

Details (taken from @TolyaTalamanov's comment):

squeezenet1.0.*onnx - doesn't require scaling to [0,1] and mean/std because the weights of the first convolution already scaled. ONNX documentation is broken. So the correct approach to use this models is:

ONNX: apply preprocessing from the documentation: https://github.com/onnx/models/blob/main/vision/classification/imagenet_preprocess.py#L8-L44 but without normalization step:

# DON'T DO IT:
# mean_vec = np.array([0.485, 0.456, 0.406])
# stddev_vec = np.array([0.229, 0.224, 0.225])
# norm_img_data = np.zeros(img_data.shape).astype('float32')
# for i in range(img_data.shape[0]):
#     norm_img_data[i,:,:] = (img_data[i,:,:]/255 - mean_vec[i]) / stddev_vec[i]
#     # add batch channel
#     norm_img_data = norm_img_data.reshape(1, 3, 224, 224).astype('float32')
#     return norm_img_data

# INSTEAD
return img_data.reshape(1, 3, 224, 224)

G-API: Convert image from BGR to RGB and then pass to apply as-is with configuring parameters:

net = cv.gapi.onnx.params('squeezenet', model_filename)
net.cfgNormalize('data_0', False)

Note: Results might be difference because G-API doesn't apply central crop but just do resize to model resolution.

squeezenet1.1.*onnx - requires scaling to [0,1] and mean/std - onnx documentation is correct.

ONNX: apply preprocessing from the documentation: https://github.com/onnx/models/blob/main/vision/classification/imagenet_preprocess.py#L8-L44
G-API: Convert image from BGR to RGB and then pass to apply as-is with configuring parameters:

net = cv.gapi.onnx.params('squeezenet', model_filename)
net.cfgNormalize('data_0', True) // default
net.cfgMeanStd('data_0', [0.485, 0.456, 0.406], [0.229, 0.224, 0.225])

Note: Results might be difference because G-API doesn't apply central crop but just do resize to model resolution.

2. Expose Fluid & kernel package-related functionality in Python

cv::gapi::combine()
cv::GKernelPackage::size() (mainly for testing purposes)
cv::gapi::imgproc::fluid::kernels()

Added a test for the above.

3. Fixed issues with Python stateful kernel handling

Fixed error message when outMeta() of custom python operation fails.

4. Fixed various issues in Python tests

test_gapi_streaming.py - fixed behavior of Desync test to avoid sporadic issues
test_gapi_infer_onnx.py - fixed model lookup (it was still using the ONNX Zoo layout but was NOT using the proper env var we use to point to one).

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

- Mean/StDev - Normalize flag

…ic infer * Some design review is required still, see new TODOs and FIXMEs

dmatveev

Commented on the questionable parts in rev. 0

dmatveev · 2023-05-09T19:34:14Z

modules/gapi/src/backends/onnx/gonnxbackend.cpp

                // Adding const input is necessary because the definition of input_names
                // includes const input.
+                // FIXME(DM): This is actually questionable!
                for (const auto& a : pp.const_inputs)
                {
                    pp.input_names.push_back(a.first);


@TolyaTalamanov @mpashchenkov this may require your review

That's weird, const inputs must be set explicitly via cfgConstInputs maybe @mpashchenkov knows details...

I believe it came with #22017, I am wondering what's the right approach there.

I removed this code for now, and tests pass. Propose to keep it as-is.

- cv::gapi::combine() is now exposed as cv.gapi.combine(); - GKernelPackage::size() was exposed in Python for testing purposes; - Fluid `imgproc` kernel package was exposed as cv.gapi.imgproc.fluid.kernels().

* Removed PyObjectHolder usage * Throw understandable error message * Added tests

TolyaTalamanov · 2023-05-15T07:23:31Z

modules/gapi/include/opencv2/gapi/infer/bindings_onnx.hpp

+                         const cv::Scalar &m,
+                         const cv::Scalar &s);
+    GAPI_WRAP
+    PyParams& cfgNormalize(const std::string &layer_name, bool flag);


I propose we disable normalization at all if cfgMeanStd is provided because using these two is very confusing...

Wait, shouldn't we in fact enable normalization when we pass mean/std?

Discussed locally, quick summary:

squeezenet1.0.*onnx - doesn't require scaling to [0,1] and mean/std because the weights of the first convolution already scaled. ONNX documentation is broken. So the correct approach to use this models is:

ONNX: apply preprocessing from the documentation: https://github.com/onnx/models/blob/main/vision/classification/imagenet_preprocess.py#L8-L44 but without normalization step:

# DON'T DO IT: # mean_vec = np.array([0.485, 0.456, 0.406]) # stddev_vec = np.array([0.229, 0.224, 0.225]) # norm_img_data = np.zeros(img_data.shape).astype('float32') # for i in range(img_data.shape[0]): # norm_img_data[i,:,:] = (img_data[i,:,:]/255 - mean_vec[i]) / stddev_vec[i] # # add batch channel # norm_img_data = norm_img_data.reshape(1, 3, 224, 224).astype('float32') # return norm_img_data # INSTEAD return img_data.reshape(1, 3, 224, 224)

G-API: Convert image from BGR to RGB and then pass to apply as-is with configuring parameters:

net = cv.gapi.onnx.params('squeezenet', model_filename) net.cfgNormalize('data_0', False)

Note: Results might be difference because G-API doesn't apply central crop but just do resize to model resolution.

squeezenet1.1.*onnx - requires scaling to [0,1] and mean/std - onnx documentation is correct.

ONNX: apply preprocessing from the documentation: https://github.com/onnx/models/blob/main/vision/classification/imagenet_preprocess.py#L8-L44

G-API: Convert image from BGR to RGB and then pass to apply as-is with configuring parameters:

net = cv.gapi.onnx.params('squeezenet', model_filename) net.cfgNormalize('data_0', True) // default net.cfgMeanStd('data_0', [0.485, 0.456, 0.406], [0.229, 0.224, 0.225])

Note: Results might be difference because G-API doesn't apply central crop but just do resize to model resolution.

Thanks @TolyaTalamanov

It seems with the exposed knobs we cover most of the cases so can go forward in the current state

modules/gapi/src/backends/onnx/bindings_onnx.cpp

dmatveev · 2023-05-26T22:52:33Z

@TolyaTalamanov I see you fixed stateful kernels here, please update the PR description accordingly (add a new chapter please).
If you don't have permissions then send the text to me please.

Thanks!

0) Fixed compilation warnings on onnx.hpp 1) Explicitly disabled normalization in C++ & Python tests which use squeezenet1.0-9.onnx 2) Aligned the Python ONNX test to take model from ONNX Zoo (rely on the same test environment used for C++ ONNX tests)

… unpack

…o dm/gapi_onnx_py_integration

TolyaTalamanov

Great, thanks! 👍

dmatveev · 2023-05-30T09:13:37Z

@asmorkalov can you please merge this one? Thanks!

@TolyaTalamanov

…tion G-API: Integration branch for ONNX & Python-related changes opencv#23597 # Changes overview ## 1. Expose ONNX backend's Normalization and Mean-value parameters in Python * Since Python G-API bindings rely on `Generic` infer to express Inference, the `Generic` specialization of `onnx::Params` was extended with new methods to control normalization (`/255`) and mean-value; these methods were exposed in the Python bindings * Found some questionable parts in the existing API which I'd like to review/discuss (see comments) UPD: 1. Thanks to @TolyaTalamanov normalization inconsistencies have been identified with `squeezenet1.0-9` ONNX model itself; tests using these model were updated to DISABLE normalization and NOT using mean/value. 2. Questionable parts were removed and tests still pass. ### Details (taken from @TolyaTalamanov's comment): `squeezenet1.0.*onnx` - doesn't require scaling to [0,1] and mean/std because the weights of the first convolution already scaled. ONNX documentation is broken. So the correct approach to use this models is: 1. ONNX: apply preprocessing from the documentation: https://github.com/onnx/models/blob/main/vision/classification/imagenet_preprocess.py#L8-L44 but without normalization step: ``` # DON'T DO IT: # mean_vec = np.array([0.485, 0.456, 0.406]) # stddev_vec = np.array([0.229, 0.224, 0.225]) # norm_img_data = np.zeros(img_data.shape).astype('float32') # for i in range(img_data.shape[0]): # norm_img_data[i,:,:] = (img_data[i,:,:]/255 - mean_vec[i]) / stddev_vec[i] # # add batch channel # norm_img_data = norm_img_data.reshape(1, 3, 224, 224).astype('float32') # return norm_img_data # INSTEAD return img_data.reshape(1, 3, 224, 224) ``` 2. G-API: Convert image from BGR to RGB and then pass to `apply` as-is with configuring parameters: ``` net = cv.gapi.onnx.params('squeezenet', model_filename) net.cfgNormalize('data_0', False) ``` **Note**: Results might be difference because `G-API` doesn't apply central crop but just do resize to model resolution. --- `squeezenet1.1.*onnx` - requires scaling to [0,1] and mean/std - onnx documentation is correct. 1. ONNX: apply preprocessing from the documentation: https://github.com/onnx/models/blob/main/vision/classification/imagenet_preprocess.py#L8-L44 2. G-API: Convert image from BGR to RGB and then pass to `apply` as-is with configuring parameters: ``` net = cv.gapi.onnx.params('squeezenet', model_filename) net.cfgNormalize('data_0', True) // default net.cfgMeanStd('data_0', [0.485, 0.456, 0.406], [0.229, 0.224, 0.225]) ``` **Note**: Results might be difference because `G-API` doesn't apply central crop but just do resize to model resolution. ## 2. Expose Fluid & kernel package-related functionality in Python * `cv::gapi::combine()` * `cv::GKernelPackage::size()` (mainly for testing purposes) * `cv::gapi::imgproc::fluid::kernels()` Added a test for the above. ## 3. Fixed issues with Python stateful kernel handling Fixed error message when `outMeta()` of custom python operation fails. ## 4. Fixed various issues in Python tests 1. `test_gapi_streaming.py` - fixed behavior of Desync test to avoid sporadic issues 2. `test_gapi_infer_onnx.py` - fixed model lookup (it was still using the ONNX Zoo layout but was NOT using the proper env var we use to point to one). ### Pull Request Readiness Checklist See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request - [x] I agree to contribute to the project under Apache 2 License. - [x] 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 - [x] The PR is proposed to the proper branch - [x] There is a reference to the original bug report and related work - [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable Patch to opencv_extra has the same branch name. - [x] The feature is well documented and sample code can be built with the project CMake

@TolyaTalamanov

…tion G-API: Integration branch for ONNX & Python-related changes opencv#23597 # Changes overview ## 1. Expose ONNX backend's Normalization and Mean-value parameters in Python * Since Python G-API bindings rely on `Generic` infer to express Inference, the `Generic` specialization of `onnx::Params` was extended with new methods to control normalization (`/255`) and mean-value; these methods were exposed in the Python bindings * Found some questionable parts in the existing API which I'd like to review/discuss (see comments) UPD: 1. Thanks to @TolyaTalamanov normalization inconsistencies have been identified with `squeezenet1.0-9` ONNX model itself; tests using these model were updated to DISABLE normalization and NOT using mean/value. 2. Questionable parts were removed and tests still pass. ### Details (taken from @TolyaTalamanov's comment): `squeezenet1.0.*onnx` - doesn't require scaling to [0,1] and mean/std because the weights of the first convolution already scaled. ONNX documentation is broken. So the correct approach to use this models is: 1. ONNX: apply preprocessing from the documentation: https://github.com/onnx/models/blob/main/vision/classification/imagenet_preprocess.py#L8-L44 but without normalization step: ``` # DON'T DO IT: # mean_vec = np.array([0.485, 0.456, 0.406]) # stddev_vec = np.array([0.229, 0.224, 0.225]) # norm_img_data = np.zeros(img_data.shape).astype('float32') # for i in range(img_data.shape[0]): # norm_img_data[i,:,:] = (img_data[i,:,:]/255 - mean_vec[i]) / stddev_vec[i] # # add batch channel # norm_img_data = norm_img_data.reshape(1, 3, 224, 224).astype('float32') # return norm_img_data # INSTEAD return img_data.reshape(1, 3, 224, 224) ``` 2. G-API: Convert image from BGR to RGB and then pass to `apply` as-is with configuring parameters: ``` net = cv.gapi.onnx.params('squeezenet', model_filename) net.cfgNormalize('data_0', False) ``` **Note**: Results might be difference because `G-API` doesn't apply central crop but just do resize to model resolution. --- `squeezenet1.1.*onnx` - requires scaling to [0,1] and mean/std - onnx documentation is correct. 1. ONNX: apply preprocessing from the documentation: https://github.com/onnx/models/blob/main/vision/classification/imagenet_preprocess.py#L8-L44 2. G-API: Convert image from BGR to RGB and then pass to `apply` as-is with configuring parameters: ``` net = cv.gapi.onnx.params('squeezenet', model_filename) net.cfgNormalize('data_0', True) // default net.cfgMeanStd('data_0', [0.485, 0.456, 0.406], [0.229, 0.224, 0.225]) ``` **Note**: Results might be difference because `G-API` doesn't apply central crop but just do resize to model resolution. ## 2. Expose Fluid & kernel package-related functionality in Python * `cv::gapi::combine()` * `cv::GKernelPackage::size()` (mainly for testing purposes) * `cv::gapi::imgproc::fluid::kernels()` Added a test for the above. ## 3. Fixed issues with Python stateful kernel handling Fixed error message when `outMeta()` of custom python operation fails. ## 4. Fixed various issues in Python tests 1. `test_gapi_streaming.py` - fixed behavior of Desync test to avoid sporadic issues 2. `test_gapi_infer_onnx.py` - fixed model lookup (it was still using the ONNX Zoo layout but was NOT using the proper env var we use to point to one). ### Pull Request Readiness Checklist See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request - [x] I agree to contribute to the project under Apache 2 License. - [x] 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 - [x] The PR is proposed to the proper branch - [x] There is a reference to the original bug report and related work - [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable Patch to opencv_extra has the same branch name. - [x] The feature is well documented and sample code can be built with the project CMake

dmatveev added 3 commits April 30, 2023 08:02

G-API ONNX: Expose extra options for ONNX Network parameters in Python

8df80d7

- Mean/StDev - Normalize flag

G-API ONNX: Fix mean/std/norm parameters to work correctly with Gener…

f18f535

…ic infer * Some design review is required still, see new TODOs and FIXMEs

Merge remote-tracking branch 'opencv/4.x' into dm/gapi_onnx_rt_1.14.1

a870062

dmatveev commented May 9, 2023

View reviewed changes

dmatveev changed the title ~~G-API: Integration branch for ONNX & Python-related changes~~ WIP: G-API: Integration branch for ONNX & Python-related changes May 9, 2023

dmatveev self-assigned this May 9, 2023

dmatveev and others added 2 commits May 9, 2023 20:56

G-API: Expose more functionality in Python

3a8ea94

- cv::gapi::combine() is now exposed as cv.gapi.combine(); - GKernelPackage::size() was exposed in Python for testing purposes; - Fluid `imgproc` kernel package was exposed as cv.gapi.imgproc.fluid.kernels().

Fix stateful kernel python implementation

eaa5ee5

* Removed PyObjectHolder usage * Throw understandable error message * Added tests

TolyaTalamanov reviewed May 15, 2023

View reviewed changes

asutic reviewed May 16, 2023

View reviewed changes

modules/gapi/src/backends/onnx/bindings_onnx.cpp Show resolved Hide resolved

dmatveev added the category: g-api / gapi label May 23, 2023

dmatveev added this to the 4.8.0 milestone May 23, 2023

dmatveev added 3 commits May 26, 2023 23:53

G-API ONNX Test updates:

c62e10b

0) Fixed compilation warnings on onnx.hpp 1) Explicitly disabled normalization in C++ & Python tests which use squeezenet1.0-9.onnx 2) Aligned the Python ONNX test to take model from ONNX Zoo (rely on the same test environment used for C++ ONNX tests)

G-API ONNX: Remvoved questionable ConstInput code from Generic params…

8dec215

… unpack

Merge remote-tracking branch 'origin/dm/gapi_onnx_py_integration' int…

b0ce38f

…o dm/gapi_onnx_py_integration

dmatveev marked this pull request as ready for review May 27, 2023 00:11

dmatveev changed the title ~~WIP: G-API: Integration branch for ONNX & Python-related changes~~ G-API: Integration branch for ONNX & Python-related changes May 27, 2023

G-API ONNX Python: Fix model path used in ONNX Python test

e5b7f3a

dmatveev force-pushed the dm/gapi_onnx_py_integration branch from 5f0158f to e5b7f3a Compare May 29, 2023 10:58

G-API Python: Fix sporadic failures in Python desync tests

7413b08

TolyaTalamanov approved these changes May 29, 2023

View reviewed changes

asmorkalov merged commit fc5d412 into opencv:4.x May 30, 2023

asmorkalov mentioned this pull request May 31, 2023

(5.x) Merge 4.x #23718

Merged

Uh oh!

Conversation

dmatveev commented May 9, 2023 • edited by TolyaTalamanov Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes overview

1. Expose ONNX backend's Normalization and Mean-value parameters in Python

Details (taken from @TolyaTalamanov's comment):

2. Expose Fluid & kernel package-related functionality in Python

3. Fixed issues with Python stateful kernel handling

4. Fixed various issues in Python tests

Pull Request Readiness Checklist

Uh oh!

dmatveev left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dmatveev commented May 26, 2023

Uh oh!

TolyaTalamanov left a comment

Choose a reason for hiding this comment

Uh oh!

dmatveev commented May 30, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dmatveev commented May 9, 2023 •

edited by TolyaTalamanov

Loading