NotificationsYou must be signed in to change notification settings
Fork56.4k
Star85.4k

dnn cleanup: On-fly-quantization removal#24980

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

asmorkalov merged 3 commits intoopencv:5.xfromfengyuentau:on-fly-quantization-removal

Feb 16, 2024

Merged

dnn cleanup: On-fly-quantization removal#24980

asmorkalov merged 3 commits intoopencv:5.xfromfengyuentau:on-fly-quantization-removal

Feb 16, 2024

Conversation

Copy link

Member

fengyuentau commentedFeb 8, 2024•
edited
Loading

On-fly-quantization is first introduced via#20228. We decided to remove it but keep int8 layers implementation because on-fly-quantization is less practical given the fact that there has been so many dedicated tools for model quantization.

Pull Request Readiness Checklist

See details athttps://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

remove quantize & tryQuantize

5e68b56

fengyuentau added category: dnn cleanupCode cleanup (e.g, drop legacy C-API, legacy unmaintained code) labels

Feb 8, 2024

fengyuentau added this to the5.0 milestone

Feb 8, 2024

fengyuentau self-assigned this

Feb 8, 2024

Copy link

MemberAuthor

fengyuentau commentedFeb 8, 2024

Should we keepnetwasQuantized? It is used only in two ways:

Check whether backend supports quantized model;
Calculate blob size ingetMemoryConsumption.

Copy link

MemberAuthor

fengyuentau commentedFeb 8, 2024

Ubuntu2004-x64-CUDA / BuildAndTest:

[ RUN      ] DNNTestNetwork.FastNeuralStyle_eccv16/0, where GetParam() = CUDA/CUDA/home/ci/opencv/modules/dnn/test/test_common.impl.hpp:75: FailureExpected: (normL1) <= (l1), actual: 0.000708958 vs 0.0007First run  |ref| = 348.99359130859375

I took a loot at the model and it has nothing to do with quantization, so this should not be related to the changes in this PR.

Copy link

Contributor

vpisarev commentedFeb 8, 2024

@fengyuentau, since the error is still very low, probably it makes sense just slightly increase the tolerance threshold, e.g. from 0.0007 to 0.001

Copy link

MemberAuthor

fengyuentau commentedFeb 8, 2024

@fengyuentau, since the error is still very low, probably it makes sense just slightly increase the tolerance threshold, e.g. from 0.0007 to 0.001

Yes, sure. I want to find out whether it is sporadic and do it in antoher PR.

Kumataro mentioned this pull request

Feb 12, 2024

ts: Extended getTypeName() to support new types#24992

Merged

6 tasks

asmorkalov self-requested a review

February 12, 2024 12:15

Copy link

Contributor

asmorkalov commentedFeb 12, 2024

Related:#24993

asmorkalov reviewed

Feb 12, 2024

View reviewed changes

modules/dnn/include/opencv2/dnn/dnn.hpp

Comment on lines -626 to -632

		CV_WRAPvoidgetInputDetails(CV_OUT std::vector<float>& scales, CV_OUT std::vector<int>& zeropoints)const;

		/** @brief Returns output scale and zeropoint for a quantized Net.
		* @param scales output parameter for returning output scales.
		* @param zeropoints output parameter for returning output zeropoints.
		*/
		CV_WRAPvoidgetOutputDetails(CV_OUT std::vector<float>& scales, CV_OUT std::vector<int>& zeropoints)const;

Copy link

Contributor

asmorkalovFeb 12, 2024

Choose a reason for hiding this comment

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

I suppose thatgetInputDetails andgetOutputDetails should be presumed. We support pre-quantized networks and the mentioned API is not related to on-fly quantization.

Copy link

MemberAuthor

fengyuentauFeb 13, 2024

Choose a reason for hiding this comment

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

They are only used intest_int8_layers.cpp which quantizes nets on the fly (thus you need to quantize the inputs manually with theinputDetails). I don't think they are useful anymore given the fact that we are removing on-fly quantization.

modules/dnn/test/test_int8_layers.cppShow resolvedHide resolved

asmorkalov requested a review fromvpisarev

February 14, 2024 06:50

Copy link

Contributor

vpisarev commentedFeb 14, 2024

@fengyuentau, thank you! We discussed it briefly with@asmorkalov. Indeed, the functionality shall be removed and the tests should be commented off. It's recommended to use// to comment off tests to make the patch easier to read./* and*/, applied to big pieces of code are difficult to analyze in a patch.

revert comment changes; use #if 0

dbfe0d1

Copy link

MemberAuthor

fengyuentau commentedFeb 16, 2024

I thinknetWasQuantized should be removed as well since we are going to have type inference and whether a net (quantized or not) should be supported by any backend should be determined by whether all the layers are supported in the backend.

Copy link

MemberAuthor

fengyuentau commentedFeb 16, 2024

@fengyuentau, thank you! We discussed it briefly with@asmorkalov. Indeed, the functionality shall be removed and the tests should be commented off. It's recommended to use// to comment off tests to make the patch easier to read./* and*/, applied to big pieces of code are difficult to analyze in a patch.

I used#if 0 as sugguested by@opencv-alalek.