Skip to content

DNN: let Quant and Dequant of ONNX_importer support the Constant input.#22666

Merged
alalek merged 2 commits intoopencv:4.xfrom
zihaomu:support_onnx_qdq_model
Oct 31, 2022
Merged

DNN: let Quant and Dequant of ONNX_importer support the Constant input.#22666
alalek merged 2 commits intoopencv:4.xfrom
zihaomu:support_onnx_qdq_model

Conversation

@zihaomu
Copy link
Copy Markdown
Member

@zihaomu zihaomu commented Oct 20, 2022 •
edited
Loading

Merge with extra: opencv_extra/#1014

For zeropoint and scale can be 1-D tensor, only exists in Quant, Deaunt and weight of QLinearConv. We have supported 1-D tensor case in QLinearConv. For Quant and Deaunt, we can support the constant input case, which is very common situation in per-channel QDQ quantized format. Model examples can be found at PR of extra.

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

@zihaomu zihaomu requested a review from rogday October 20, 2022 00:54
zihaomu
zihaomu commented Oct 20, 2022
View reviewed changes
modules/dnn/src/onnx/onnx_importer.cpp

template <typename T>
static T getScaleFromMat(Mat m)
static T getScalarFromMat(Mat m)
Copy link
Copy Markdown
Member Author

@zihaomu zihaomu Oct 20, 2022

Choose a reason for hiding this comment

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

The old name has an error. Scale => Scalar.

@zihaomu zihaomu added the category: dnn (onnx) ONNX suport issues in DNN module label Oct 20, 2022
@zihaomu zihaomu force-pushed the support_onnx_qdq_model branch from 473c9d0 to c966332 Compare October 20, 2022 03:06
@asmorkalov asmorkalov added this to the 4.7.0 milestone Oct 20, 2022
rogday
rogday suggested changes Oct 20, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@rogday rogday left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution!

modules/dnn/src/onnx/onnx_importer.cpp Outdated
if(zpMat.total() > 1) scalarParams = false;
}

if (constBlobs.find(node_proto.input(0)) == constBlobs.end()) // Variable input.
Copy link
Copy Markdown
Member

@rogday rogday Oct 20, 2022

Choose a reason for hiding this comment

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

if (scalar)
{
    layerParams.set(scales, zeropoint, depth)
    if (const) { runLayer() }
    else { addLayer() }
    return
}
assert(const)
axis = ...;
calculate()

so that we don't repeat getting scalar zeropoint and scales 3 times.

Copy link
Copy Markdown
Member Author

@zihaomu zihaomu Oct 21, 2022

Choose a reason for hiding this comment

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

Fixed.

@zihaomu zihaomu force-pushed the support_onnx_qdq_model branch 3 times, most recently from 306b6e7 to b55ac6d Compare October 21, 2022 13:27
zihaomu
zihaomu commented Oct 21, 2022
View reviewed changes
modules/dnn/include/opencv2/dnn/all_layers.hpp
Comment on lines -425 to +426
float scale;
int zeropoint;
bool is1D;
std::vector<float> scales;
std::vector<int> zeropoints;
Copy link
Copy Markdown
Member Author

@zihaomu zihaomu Oct 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Replace scale with scales, so that we can support the scales of 1-D tensor. And this change affects the ABI compatibility.

Copy link
Copy Markdown
Member

@fengyuentau fengyuentau Oct 22, 2022

Choose a reason for hiding this comment

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

ABI compatibility

API?

Copy link
Copy Markdown
Member Author

@zihaomu zihaomu Oct 22, 2022

Choose a reason for hiding this comment

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

more details can be found in the un-passed ci.

@zihaomu
Copy link
Copy Markdown
Member Author

zihaomu commented Oct 24, 2022

For me, this PR is done.

@rogday
Copy link
Copy Markdown
Member

rogday commented Oct 24, 2022

For me, this PR is done.

Where do you get the axis for per axis quantization?

@zihaomu
Copy link
Copy Markdown
Member Author

zihaomu commented Oct 24, 2022

For me, this PR is done.

Where do you get the axis for per axis quantization?

Hi, @rogday. My mistake. For now, I just implement the default axis. I will re-think this implementation.

@zihaomu zihaomu force-pushed the support_onnx_qdq_model branch from b55ac6d to cf48b28 Compare October 29, 2022 08:56
@zihaomu zihaomu force-pushed the support_onnx_qdq_model branch from cf48b28 to 1a66ed8 Compare October 29, 2022 08:58
@zihaomu
Copy link
Copy Markdown
Member Author

zihaomu commented Oct 31, 2022

The axis has been supported.

rogday
rogday reviewed Oct 31, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@rogday rogday left a comment

Choose a reason for hiding this comment

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

Looks good, 1 question.

rogday
rogday approved these changes Oct 31, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@rogday rogday left a comment

Choose a reason for hiding this comment

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

👍

@alalek alalek merged commit 903bf01 into opencv:4.x Oct 31, 2022
@zihaomu zihaomu deleted the support_onnx_qdq_model branch November 6, 2022 01:53
@fengyuentau fengyuentau mentioned this pull request Nov 16, 2022
Closed
4 tasks
@alalek alalek mentioned this pull request Jan 8, 2023
Merged
a-sajjad72 pushed a commit to a-sajjad72/opencv that referenced this pull request Mar 30, 2023
@zihaomu
Merge pull request opencv#22666 from zihaomu:support_onnx_qdq_model
347f6c9 
DNN: let Quant and Dequant of ONNX_importer support the Constant input.

* let Quant and Dequant support the Constant input.

* fix negative value of axis.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fengyuentau fengyuentau fengyuentau left review comments

+1 more reviewer

@rogday rogday rogday approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@rogday rogday

Labels

category: dnn (onnx) ONNX suport issues in DNN module feature

Projects

None yet

Milestone

4.7.0

Development

Successfully merging this pull request may close these issues.

5 participants

@zihaomu @rogday @fengyuentau @asmorkalov @alalek