-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[NNAPI QDQ] Add QDQ Resize support #10442
Conversation
onnxruntime/core/providers/nnapi/nnapi_builtin/builders/op_support_checker.cc
Show resolved
Hide resolved
return false; | ||
if (!HasValidQuantizationZeroPoints(initializers, node_unit, {0}, false /* is_input */)) | ||
return false; | ||
} |
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.
As this is also validating inputs/output maybe it could be called something like IsValidQuantizedOp.
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.
Or maybe all the validation should be in a separate func given we seem to do some here and some in ResizeOpBuilder::AddToModelBuilderImpl
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.
Yes, there will be a separated change to merge all these HasValidQuantizationScales and HasValidQuantizationZeroPoints into one single helper function, plus the input and output type check,
Will be added in a later PR.
The validation of quantized IO params are different in SupportChecker and OPbuilder, the one in SupportChecker only checks if the quantized scale and zp are initializers and some other checks such as per-channel setting, the one in OPBuilder will also verify if these quantization params are matching the existing numbers in the NNAPI model, this part cannot be done in SupportChecker.
Will figure out better names for these in a later PR
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.
}; | ||
|
||
/* static */ bool ResizeOpBuilder::IsQuantizedOp(const NodeUnit& node_unit) { | ||
static bool is_quant_op_type = |
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.
static variable? so this function returns the same result each time?
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.
same comment for ResizeOpSupportChecker::IsQuantizedOp()
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.
Yep, this is incorrect, typical result of late night coding :(, updated both places
auto check_matmul_graph = [&](InferenceSessionWrapper& session) { | ||
auto op_to_count = CountOpsInGraph(session.GetGraph()); | ||
EXPECT_EQ(op_to_count["Resize"], 1); | ||
EXPECT_EQ(op_to_count["QuantizeLinear"], 0); | ||
EXPECT_EQ(op_to_count["DequantizeLinear"], 0); | ||
}; | ||
|
||
TransformerTester(build_test_case, check_matmul_graph, | ||
TransformerTester(BuildQDQResizeTestCase(input1_shape, sizes_shape), | ||
check_matmul_graph, |
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.
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 update in my next PR
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.
Description: [NNAPI QDQ] Add QDQ Conv support
Motivation and Context
This is part of task 6, to add support of QDQ Resize which does not have
QLinear
version