Skip to content

[Frontend][TFlite] Add parser support for relu6, leaky_relu, relu_n1_to_1, log_softmax - #4805

Merged
anijain2305 merged 3 commits into
apache:masterfrom
inadob:activation
Jun 16, 2020
Merged

[Frontend][TFlite] Add parser support for relu6, leaky_relu, relu_n1_to_1, log_softmax#4805
anijain2305 merged 3 commits into
apache:masterfrom
inadob:activation

Conversation

@inadob

Copy link
Copy Markdown
Contributor

This patch is made on top of #4789.

  • Add helper functions in the parser to quantize and dequantize with Relay
  • Add qnn tests for all 4 activation functions

@inadobinadob changed the title [Frontend][TFlite] Add parser support for relu6, leeaky_relu, relu_n1_to_1, log_softmax[Frontend][TFlite] Add parser support for relu6, leaky_relu, relu_n1_to_1, log_softmaxFeb 3, 2020

@anijain2305anijain2305 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Relu and Clip implementation does not look right.

We can keep the computation in integer domain. The way to do that is to subtract the input zero point, and then call Relu, then requantize to the output scale (only if output scale/zero point are different from input scale/zero point).

return out

def convert_leaky_relu(self, op):
"""Convert TFLite LEAKY_RELU"""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not important, I think it should be """Convert TFLite Leaky_ReLU""" to align with """One iteration of Leaky_ReLU""".

return out

def convert_relu_n1_to_1(self, op):
"""Convert TFLite RELU_N1_TO_1"""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not important, I think it should be """Convert TFLite ReLU_n1_to_1""" to align with """One iteration of ReLU_n1_to_1""".

@tqchentqchen added the status: need update need update based on feedbacks label Feb 12, 2020
@tqchen

Copy link
Copy Markdown
Member

@inadob please followup to address the review comments :)

@inadob

inadob commented Feb 28, 2020

Copy link
Copy Markdown
ContributorAuthor

Relu and Clip implementation does not look right.

We can keep the computation in integer domain. The way to do that is to subtract the input zero point, and then call Relu, then requantize to the output scale (only if output scale/zero point are different from input scale/zero point).

That's fine for the standard ReLU op but I am not entirely sure whether we can follow this logic when we apply ReLU6 and ReLU1. Is there a way to recreate them in Relay without clip?.

What I did, in this case, was to shift the data by subtracting the zero point, do clip, shift back and finally requantize if needed. The problem here was that Clip() from RELAY_PASS_PATTERN needs the clipping range to be in double and I couldn't just fix this by casting to float64 (it seems double is not supported in TVM).

@anijain@u99127

@anijain2305

Copy link
Copy Markdown
Contributor

The problem here was that Clip() from RELAY_PASS_PATTERN needs the clipping range to be in double and I couldn't just fix this by casting to float64 (it seems double is not supported in TVM).

Can you show me code example here? I have typically used float numbers like 1.0 or 6.0 to work with a_min or a_max

@inadob

Copy link
Copy Markdown
ContributorAuthor

Can you show me code example here? I have typically used float numbers like 1.0 or 6.0 to work with a_min or a_max

Expr ClipQnnCanonicalize(const Attrs& attrs, const Array<Expr>& new_args,
const Array<tvm::relay::Type>& arg_types) {
CHECK_EQ(new_args.size(), 7);
auto& input_tensor = new_args[0];
auto& input_scale = new_args[1]; // in fpauto& input_zero_point = new_args[2]; // in int32auto& clip_min = new_args[3]; // value is in fpauto& clip_max = new_args[4]; // value is in fpauto& output_scale = new_args[5];
auto& output_zero_point = new_args[6];
// Get the input dtype and shape.CHECK_EQ(arg_types.size(), 8);
auto tensor_type = arg_types[0].as<TensorTypeNode>();
CHECK(tensor_type != nullptr);
auto input_dtype = tensor_type->dtype;
auto input_shape = tensor_type->shape;
// shift the input by subtracting the input zero_pointauto shifted_input = Subtract(Cast(input_tensor, DataType::Int(32)), input_zero_point);
// do the clipping in int32// auto clipped_tensor = Clip(shifted_input, clip_min, clip_max)auto clipped_tensor = Clip(shifted_input, Cast(clip_min, DataType::Float(64)), Cast(clip_max, DataType::Float(64)))
// shift the input back by adding the zero_point
clipped_tensor = Add(clipped_tensor, input_zero_point);
// requantize the output if neededauto requantized_output = clipped_tensor;
if (!IsEqualScalar(input_scale, output_scale) ||
!IsEqualScalar(input_zero_point, output_zero_point)) {
requantized_output = Requantize(clipped_tensor, input_shape, input_scale, input_zero_point, output_scale,
output_zero_point, DataType::Int(32));
}
// Go back to lower precision.auto q_min = GetQmin(input_dtype);
auto q_max = GetQmax(input_dtype);
requantized_output = Clip(requantized_output, q_min, q_max); returnCast(requantized_output, input_dtype);
}

And I am getting a complaint about double dtype...

/workspace/src/relay/qnn/op/clip.cc:61:117: error: cannot convert 'tvm::relay::Expr {aka tvm::RelayExpr}' to 'double' for argument '2' to 'tvm::relay::Expr tvm::relay::Clip(tvm::relay::Expr, double, double)'
auto clipped_tensor = Clip(shifted_input, Cast(clip_min, DataType::Float(64)), Cast(clip_max, DataType::Float(64)))

The commented out line where I directly do clip() without casting to float64 didn't work too.

@tqchen

Copy link
Copy Markdown
Member

@anijain2305@inadob please followup :)

@tqchen

Copy link
Copy Markdown
Member

ping

@inadob

Copy link
Copy Markdown
ContributorAuthor

@anijain can you please help me with the quantized operators

@anijain2305

anijain2305 commented May 5, 2020

Copy link
Copy Markdown
Contributor

Sorry for the delay. @inadob Can you take a look at this PR - #5479

I have added support for fused quantized activations there. We dont need to create a new op in C++ from scratch. It is pretty easy to do the same thing in python. Let me know what you think

Comment threadpython/tvm/relay/frontend/tflite.py
@anijain2305

Copy link
Copy Markdown
Contributor

Ping @inadob Let us know if you are working on this. Or else I can take a chance at this.

@inadob

inadob commented Jun 1, 2020

Copy link
Copy Markdown
ContributorAuthor

Ping @inadob Let us know if you are working on this. Or else I can take a chance at this.

@anijain2305 Yes, I am working on it - I will be ready in the next few days. Sorry for the delay

@anijain2305

Copy link
Copy Markdown
Contributor

@inadob Your changes look better now. Can you please rebase? (git submodule update --init --recursive)

@anijain2305

Copy link
Copy Markdown
Contributor

It seems you still have old `3rdparty/dmlc-core'. You can check that by clicking on "Files changed" tab.

@inadob

Copy link
Copy Markdown
ContributorAuthor

It seems you still have old `3rdparty/dmlc-core'. You can check that by clicking on "Files changed" tab.

It's fixed now.

@anijain2305

Copy link
Copy Markdown
Contributor

@inadob Can you please fix the CI error?

@anijain2305

Copy link
Copy Markdown
Contributor

Ping @inadob :)

@tqchen

Copy link
Copy Markdown
Member

Glad that it is moving, @anijain2305 please manage this PR

inadob added 2 commits June 13, 2020 13:25
…g_softmax
* add implementation in parser
* add qnn tests for each operator
* add 'clip' as in the quantized fused operations
* remove redundant assertions and imports
@inadob
inadobforce-pushed the activation branch 2 times, most recently from 4781b9e to 126361cCompareJune 13, 2020 16:45
@inadob

Copy link
Copy Markdown
ContributorAuthor

@anijain2305
CI has been fixed :)

@anijain2305
anijain2305 merged commit 52bf113 into apache:masterJun 16, 2020
@anijain2305

Copy link
Copy Markdown
Contributor

Thanks @inadob This is merged!

@tqchentqchen added status: accepted and removed status: need update need update based on feedbacks labels Jun 16, 2020
trevor-m pushed a commit to trevor-m/tvm that referenced this pull request Jun 30, 2020
…to_1, log_softmax (apache#4805)
* [Frontend][TFLite]Add support for relu6, leaky_relu, relu_n1_to_1, log_softmax
* add implementation in parser
* add qnn tests for each operator
* Implement clip operation for quantized relu6, relu1
* add 'clip' as in the quantized fused operations
* remove redundant assertions and imports
* Fix floating value quantization for RELU6 and RELU1
zhiics pushed a commit to neo-ai/tvm that referenced this pull request Jul 2, 2020
…to_1, log_softmax (apache#4805)
* [Frontend][TFLite]Add support for relu6, leaky_relu, relu_n1_to_1, log_softmax
* add implementation in parser
* add qnn tests for each operator
* Implement clip operation for quantized relu6, relu1
* add 'clip' as in the quantized fused operations
* remove redundant assertions and imports
* Fix floating value quantization for RELU6 and RELU1
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@inadob@tqchen@anijain2305@wyc-ruiker@FrozenGene