| CARVIEW |
Navigation Menu
-
-
Notifications
You must be signed in to change notification settings - Fork 56.3k
dnn onnx: add instance norm layer #24378
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
fengyuentau
added
feature
category: dnn
category: dnn (onnx)
|
This is 3rd layer which will do the same compute. There are already MVN and LayerNorm, can they be fixed? |
I prefer to keep all of those (keep mvn for caffe models). Dedicated layers are more flexible when it comes to backend support and optimization. And we don't have to break a single layer into several layers, for example, |
|
@fengyuentau, MVN already provides OpenCL, CUDA and OpenVINO optimizations. Let's rename it instead and use in ONNX importer. |
We still have CANN and some other backends which has its own graph. Merging several layers into one may have trouble catching layer type and corresponding parameters. Dedicated layers are more maintainable. |
|
@fengyuentau, agree, but what about fixing LayerNorm? Can it be replaced in ONNX importer instead if MVN? |
I propose to keep it as-is. There is a plan adding CANN backend support for it #23550. If the layer type is missed, it may most probably fool the CANN graph simplifier and lead to performance drop. |
|
This way, can InstanceNorm replace LayerNorm in the simplifier completely?
|
|
Speaking of OpenVINO, is there a documentation on its operator spec? Something like the ONNX operator spec documentation. |
https://docs.openvino.ai/2023.1/openvino_docs_operations_specifications.html |
Thank you! |
|
@dkurt, @fengyuentau, I think, ONNX with its InstanceNormalization is much more popular nowadays vs. Caffe & MVN. I would be nice, I think, to have a dedicated layer with conventional name (InstanceNormalization), but internally it can call MVN kernel (which could be declared in some internal headers) |
|
In this way, we can extract the kernel, put it in |
|
@fengyuentau Please pay attention on the merge conflict. |
fengyuentau
force-pushed
the
instance_norm
branch
from
b216e6d to
74723a2
Compare
|
@dkurt could you take a look again? |
|
@asmorkalov, I expected that we should merge #24409 first |
dnn: add shared fastNorm kernel for mvn, instance norm and layer norm #24409 Relates #24378 (comment) TODO: - [x] add fastNorm - [x] refactor layer norm with fastNorm - [x] refactor mvn with fastNorm - [ ] add onnx mvn in importer (in a new PR?) - [ ] refactor instance norm with fastNorm (in another PR #24378, need to merge this one first though) ### 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
|
@dkurt Done. |
|
@fengyuentau please rebase to get PR 24409 included. |
|
|
||
| virtual bool supportBackend(int backendId) CV_OVERRIDE { | ||
| return backendId == DNN_BACKEND_OPENCV; | ||
| } |
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.
Breaks coverage with other backends that initialized in MVN layer (OpenVINO, CUDA).
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 will try to add them back.
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.
OpenVINO and CUDA backend are added.
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 you handle OpenCL backend as well?
opencv/modules/dnn/src/layers/mvn_layer.cpp
Line 208 in ea47cb3
| bool forward_ocl(InputArrayOfArrays inputs_, OutputArrayOfArrays outputs_, OutputArrayOfArrays internals_) |
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 can give it a shot.
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.
May I ask why returning false in forward_ocl in mvn_layer.cpp if the input umat type is int16?
opencv/modules/dnn/src/layers/mvn_layer.cpp
Lines 229 to 230 in fa56623
| if (inputs[0].depth() == CV_16S) | |
| return false; |
Initially I thought it was CV_16F, but it is CV_16S indeed. So the opencl backend supports int8 quantization as well? But the weirder thing is we don't support int16 quantization right?
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.
CV_16S is an original data type for FP16 in OpenCV (before CV_16F which is used in dnn for FP16 inference on CPU). However every OpenCL kernel uses CV_16S which actually just a short int buffer for float16. See convertFp16
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 with adding OpenCL backend (no fp16 though).
|
Update: Resolved. @dkurt Do you know how is memory allocated in CUDA op? For example, here in csl::WorkspaceBuilder builder;
builder.require<float>(max_outer_size);
if (normalize_variance)
builder.require<float>(max_outer_size);
scratch_mem_in_bytes = builder.required_workspace_size(); |
|
All tests including the ones on different backends (OpenVINO, CUDA and OpenCL) are passed đź‘Ť |
|
By the way, I though dnn with OpenCL on macOS is broken because it just did not work, but actually I found it is disable and no log is output during building opencv: opencv/modules/dnn/CMakeLists.txt Line 20 in e95c005
It has been disabled back to 2018 with a comment saying it was temporarily, but it actually lasts till today and goes on. Is it time to enable it? cc @vpisarev |
dnn: add shared fastNorm kernel for mvn, instance norm and layer norm opencv#24409 Relates opencv#24378 (comment) TODO: - [x] add fastNorm - [x] refactor layer norm with fastNorm - [x] refactor mvn with fastNorm - [ ] add onnx mvn in importer (in a new PR?) - [ ] refactor instance norm with fastNorm (in another PR opencv#24378, need to merge this one first though) ### 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
dnn onnx: add instance norm layer opencv#24378 Resolves opencv#24377 Relates opencv#24092 (comment) | Perf | multi-thread | single-thread | | - | - | - | | x: [2, 64, 180, 240] | 3.95ms | 11.12ms | Todo: - [x] speed up by multi-threading - [x] add perf - [x] add backend: OpenVINO - [x] add backend: CUDA - [x] add backend: OpenCL (no fp16) - [ ] add backend: CANN (will be done via opencv#24462) ### 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 ``` force_builders=Linux OpenCL,Win64 OpenCL,Custom buildworker:Custom=linux-4 build_image:Custom=ubuntu:18.04 modules_filter:Custom=none disable_ipp:Custom=ON ```
dnn: add shared fastNorm kernel for mvn, instance norm and layer norm opencv#24409 Relates opencv#24378 (comment) TODO: - [x] add fastNorm - [x] refactor layer norm with fastNorm - [x] refactor mvn with fastNorm - [ ] add onnx mvn in importer (in a new PR?) - [ ] refactor instance norm with fastNorm (in another PR opencv#24378, need to merge this one first though) ### 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
dnn onnx: add instance norm layer opencv#24378 Resolves opencv#24377 Relates opencv#24092 (comment) | Perf | multi-thread | single-thread | | - | - | - | | x: [2, 64, 180, 240] | 3.95ms | 11.12ms | Todo: - [x] speed up by multi-threading - [x] add perf - [x] add backend: OpenVINO - [x] add backend: CUDA - [x] add backend: OpenCL (no fp16) - [ ] add backend: CANN (will be done via opencv#24462) ### 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 ``` force_builders=Linux OpenCL,Win64 OpenCL,Custom buildworker:Custom=linux-4 build_image:Custom=ubuntu:18.04 modules_filter:Custom=none disable_ipp:Custom=ON ```
dnn: add shared fastNorm kernel for mvn, instance norm and layer norm opencv#24409 Relates opencv#24378 (comment) TODO: - [x] add fastNorm - [x] refactor layer norm with fastNorm - [x] refactor mvn with fastNorm - [ ] add onnx mvn in importer (in a new PR?) - [ ] refactor instance norm with fastNorm (in another PR opencv#24378, need to merge this one first though) ### 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
dnn onnx: add instance norm layer opencv#24378 Resolves opencv#24377 Relates opencv#24092 (comment) | Perf | multi-thread | single-thread | | - | - | - | | x: [2, 64, 180, 240] | 3.95ms | 11.12ms | Todo: - [x] speed up by multi-threading - [x] add perf - [x] add backend: OpenVINO - [x] add backend: CUDA - [x] add backend: OpenCL (no fp16) - [ ] add backend: CANN (will be done via opencv#24462) ### 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 ``` force_builders=Linux OpenCL,Win64 OpenCL,Custom buildworker:Custom=linux-4 build_image:Custom=ubuntu:18.04 modules_filter:Custom=none disable_ipp:Custom=ON ```
Resolves #24377
Relates #24092 (comment)
Todo:
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.