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

[Frontend][TFLite] L2_POOL_2D operator #5452

Merged
merged 5 commits into from
Apr 29, 2020

Conversation

maheshambule
Copy link
Contributor

@maheshambule maheshambule commented Apr 27, 2020

Adds support for L2_POOL_2D operator in TFLITE frontened.
Ref:
https://www.tensorflow.org/lite/guide/ops_compatibility

@u99127, @siju-samuel, @FrozenGene Please review.

Comment on lines +494 to +501
in_data = tf.placeholder(
dtype=tf.float32, name="input", shape=input_shape)
out = tf.sqrt(tf.nn.avg_pool(
tf.square(in_data), ksize=ksize, strides=strides,
padding=padding, data_format=data_format))
out = with_fused_activation_function(out, fused_func_name)

compare_tflite_with_tvm(x, 'input', [in_data], [out])
Copy link
Contributor

Choose a reason for hiding this comment

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

Very quick drive-by review :
Don't we need a qnn test here ? I haven't managed to read up L2_POOL_2D in the tflite documentation yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TFLite doesn't have support for quantized L2_POOL_2D yet.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks , possibly worth a belts and braces check to ensure we catch this in the frontend to ensure we think through qnn support rather than getting an unknown failure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added check.

tests/python/frontend/tflite/test_forward.py Outdated Show resolved Hide resolved
@@ -1770,6 +1775,12 @@ def convert_pool2d(self, op, pool_type):
assert self.has_same_qnn_params(input_tensor, output_tensor), \
"qnn.op.max_pool2d requires input and output qnn params to be same"
out = _op.nn.max_pool2d(in_expr, **params)
elif pool_type == "l2":
# l2_pool_2d is equivalent to square_root(avg_pool(square(in_data)))
Copy link
Member

Choose a reason for hiding this comment

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

Add a protection check to throw error if you receive quantized inputs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TFLite does not support quantized L2_POOL_2D. That's why the check is not added. Now I have added a comment for the same.

Copy link
Contributor

Choose a reason for hiding this comment

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

Putting in the check has the following benefits:

  1. Better user experience as the rest of the stack doesn't get basically undefined behaviour because of random input..
  2. A signal to us as a community in the form of a bug report that this is appearing.

Ramana

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, make sense. I have added the check.

Copy link
Member

@siju-samuel siju-samuel left a comment

Choose a reason for hiding this comment

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

LGTM

@maheshambule
Copy link
Contributor Author

@u99127, @siju-samuel, @FrozenGene, Thanks. The review comments are addressed. Could you please help in merging the PR.

@FrozenGene FrozenGene merged commit a38f61a into apache:master Apr 29, 2020
@FrozenGene
Copy link
Member

Thanks @maheshambule @siju-samuel @u99127 merged now

trevor-m pushed a commit to trevor-m/tvm that referenced this pull request Jun 9, 2020
* TFLITE fill and splitv ops

* l2_pool_2d op changes in comment

* TFLite l2_pool_2d op added test case in main

* TFLite L2_POOL_2D added check for quantized input
trevor-m pushed a commit to trevor-m/tvm that referenced this pull request Jun 18, 2020
* TFLITE fill and splitv ops

* l2_pool_2d op changes in comment

* TFLite l2_pool_2d op added test case in main

* TFLite L2_POOL_2D added check for quantized input
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request Jun 18, 2020
* TFLITE fill and splitv ops

* l2_pool_2d op changes in comment

* TFLite l2_pool_2d op added test case in main

* TFLite L2_POOL_2D added check for quantized input
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.

4 participants