Skip to content
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 QDQReshape op support #10533

Merged
merged 13 commits into from
Feb 15, 2022
Merged

Conversation

YUNQIUGUO
Copy link
Contributor

Description: Describe your changes.

Add QDQReshape op support and basic test case

Motivation and Context

More operator support

if (IsQuantizedOp(node_unit)) {
AddQuantizationScaleAndZeroPointToSkip(model_builder, *node_unit.Inputs()[0].quant_param); // x_scale, x_zp
AddQuantizationScaleAndZeroPointToSkip(model_builder, *node_unit.Outputs()[0].quant_param); // y_scale, y_zp
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

The perm initializer needs always to be ignored here

initializers, node_unit.Inputs()[0], node_unit.ModelPath(), x_scale, x_zero_point));
ORT_RETURN_IF_ERROR(IsValidInputQuantizedType(model_builder, input, x_scale, x_zero_point));
}

return AddReshapeOperator(model_builder, node_unit, input, shape);
Copy link
Contributor

Choose a reason for hiding this comment

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

You need to pass the x_scale and x_zero_point into AddReshapeOperator, otherwise those will be set to 0

if (!GetType(node_unit.Inputs()[0].node_arg, input_type))
return false;

if (input_type != ONNX_NAMESPACE::TensorProto_DataType_FLOAT &&
Copy link
Contributor

Choose a reason for hiding this comment

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

These are not required here, this is only for these operators

static const std::unordered_set<std::string> internal_quantized_op_types =
{
"Transpose",
"Resize",
"Concat",
"MaxPool",
};

@@ -867,10 +867,20 @@ class ReshapeOpBuilder : public BaseOpBuilder {
Status AddToModelBuilderImpl(ModelBuilder& model_builder, const NodeUnit& node_unit) const override;
static bool CanSkipReshape(const ModelBuilder& model_builder, const NodeUnit& node_unit,
size_t input_rank, size_t output_rank);
static bool IsQuantizedOp(const NodeUnit& node_unit) ORT_MUST_USE_RESULT; // TODO, see if we want to move this to BaseOpBuilder
Copy link
Contributor

Choose a reason for hiding this comment

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

see if we want to move this to BaseOpBuilder

doesn't have to be in this PR, but when will the decision be made?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, @gwang-msft do you think we should move it to baseopbuilder now or keep it here at individual opbuilder level as we don't have that many qdq ops supported?

Copy link
Contributor

Choose a reason for hiding this comment

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

We can move it as a virtual function for BaseOpBuilder and by default return false, each individual builder will override this if necessary
Same for BaseOpSupportChecker

@@ -971,11 +982,11 @@ void ReshapeOpBuilder::AddInitializersToSkip(ModelBuilder& model_builder, const
// Add new shape
Shape shape_dimen = {static_cast<uint32_t>(shape.size())};
std::string shape_name = model_builder.GetUniqueName(node_unit.Name() + input + "newshape");
OperandType shape_operand_type(Type::TENSOR_INT32, shape_dimen);
OperandType shape_operand_type(Type::TENSOR_INT32, shape_dimen, scale, zero_point);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the new shape of the reshape op, it does not need scale and zero_point

@@ -947,7 +957,8 @@ void ReshapeOpBuilder::AddInitializersToSkip(ModelBuilder& model_builder, const
/* static */ Status ReshapeOpBuilder::AddReshapeOperator(ModelBuilder& model_builder,
const NodeUnit& node_unit,
const std::string& input,
const std::vector<int32_t>& shape) {
const std::vector<int32_t>& shape,
float scale, int32_t zero_point) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It will be easier to get the scale and zero_point from the input inside this function, instead of passing them in, use something like this

+  // For reshape, the output type should be the same as the input type except the shape is different
+  auto output_operand_type = operand_types.at(input);
+  output_operand_type.SetDimensions(shaper[output]);
+
   // Since Reshape is not running using hardware in NNAPI for some CPU (e.g. Qualcomm SD for now)
   // We will try to see if we the skip the Reshape to prevent context switching between
   // NNAPI CPU impl and NNAPI hardware accelerator impl
   if (CanSkipReshape(model_builder, node_unit, input_rank, output_rank)) {
     // Since reshape can be skipped, only register the dimension and type, with same index and new name
-    const OperandType output_operand_type(operand_types.at(input).type, shaper[output], scale, zero_point);
     model_builder.RegisterOperand(output, operand_indices.at(input), output_operand_type, false);
   } else {
     // We still need to perform a reshape here
     // Add new shape
     Shape shape_dimen = {static_cast<uint32_t>(shape.size())};
     std::string shape_name = model_builder.GetUniqueName(node_unit.Name() + input + "newshape");
-    OperandType shape_operand_type(Type::TENSOR_INT32, shape_dimen, scale, zero_point);
+    OperandType shape_operand_type(Type::TENSOR_INT32, shape_dimen);
     ORT_RETURN_IF_ERROR(model_builder.AddOperandFromPersistMemoryBuffer(shape_name, shape.data(), shape_operand_type));
     input_indices.push_back(operand_indices.at(shape_name));
-
-    const OperandType output_operand_type(operand_types.at(input).type, shaper[output], scale, zero_point);
     ORT_RETURN_IF_ERROR(model_builder.AddOperation(ANEURALNETWORKS_RESHAPE, input_indices, {output}, {output_operand_type}, {false}));
   }

@YUNQIUGUO YUNQIUGUO merged commit 8e47bb9 into master Feb 15, 2022
@YUNQIUGUO YUNQIUGUO deleted the yguo/nnapi-qdq-reshape-support-pr branch February 15, 2022 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants