Add INT32 support to SUB #3037
Add INT32 support to SUB #3037
Conversation
There was a problem hiding this comment.
@HemanthSai7 Thank you for your interest in TFLM (LRTM) and for submitting this PR!
Please add the following to SubPrepare in the sub_common.cc file:
if (output->type == kTfLiteInt32) {
// Only support INT32 unquantized SUB for now.
TF_LITE_ENSURE_EQ(context, input1->quantization.type,
kTfLiteNoQuantization);
TF_LITE_ENSURE_EQ(context, input2->quantization.type,
kTfLiteNoQuantization);
TF_LITE_ENSURE_EQ(context, output->quantization.type,
kTfLiteNoQuantization);
}
| TF_LITE_MICRO_TEST(Int32SubNoActivation) { | ||
| int inout_shape[] = {4, 1, 2, 2, 1}; | ||
| const int32_t input1_values[] = {-2, 2147483646, -1, 1146622854}; | ||
| const int32_t input2_values[] = {3, 1, -2147483647, -726978367}; | ||
| const int32_t golden_values[] = {-5, 2147483645, 2147483646, 1873601221}; | ||
| const int kOutputDimsCount = 4; | ||
| int32_t output_data[kOutputDimsCount]; | ||
| tflite::testing::TestSubInt32(inout_shape, input1_values, inout_shape, | ||
| input2_values, inout_shape, golden_values, | ||
| kTfLiteActNone, output_data); | ||
| } | ||
|
|
There was a problem hiding this comment.
This code will need #if !defined(XTENSA) around it in order to pass the CI tests.
tensorflow/lite/micro/kernels/sub.cc
Outdated
| @@ -147,7 +184,7 @@ TfLiteStatus SubEval(TfLiteContext* context, TfLiteNode* node) { | |||
| TFLITE_DCHECK(node->user_data != nullptr); | |||
| const OpDataSub& data = *(static_cast<const OpDataSub*>(node->user_data)); | |||
|
|
|||
| if (output->type == kTfLiteFloat32) { | |||
| if (output->type == kTfLiteFloat32 || output->type == kTfLiteInt32) { | |||
| EvalSub(context, node, params, &data, input1, input2, output); | |||
There was a problem hiding this comment.
Could you add a TF_LITE_ENSURE_OK check here (line 188). It will make the code more consistent.
|
Also, another Google procedural requirement: All changed files should have the copyright year set the to current year. Only the current year should appear in the copyright. |
|
Thank you. I'll make the changes you suggested soon. |
Followed the CONTRIBUTING.md to first fetch upstream and then merge into branch
|
@ddavis-2015 I have made the changes and also updated the licensing year. |
| @@ -105,6 +105,27 @@ void TestSubFloat(int* input1_dims_data, const float* input1_data, | |||
| ElementCount(*output_dims), activation); | |||
| } | |||
|
|
|||
| void TestSubInt32(int* input1_dims_data, const int32_t* input1_data, | |||
There was a problem hiding this comment.
You will need #if !defined(XTENSA) around this method also to prevent the unused function warning (which is promoted to an error).
bug=fixes #2720