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

Enquiry about parsing optional input #22

Closed
erizmr opened this issue Nov 7, 2020 · 7 comments · Fixed by #25
Closed

Enquiry about parsing optional input #22

erizmr opened this issue Nov 7, 2020 · 7 comments · Fixed by #25
Assignees
Labels
Enhancement New feature or request

Comments

@erizmr
Copy link
Contributor

erizmr commented Nov 7, 2020

Hi jackwish,

Thanks for sharing your awesome frame!
I am trying to enable a new operator Resize from resize_bilinear, the ONNX description of Resize is shown below:
image
Only one of 'scales' and 'size' should be specified in this case, I am not sure how to 'skip' one of the arguments (also the roi) when implementing the parse part, because it seems the inputs are store in a list without a key.
I am wondering if you could share any suggestions about it?

Thanks,
Mingrui

@zhenhuaw-me zhenhuaw-me added this to Proposed in v0.4 - Quantization Enhancement via automation Nov 9, 2020
@zhenhuaw-me zhenhuaw-me added the Enhancement New feature or request label Nov 9, 2020
@zhenhuaw-me zhenhuaw-me moved this from Proposed to To Do in v0.4 - Quantization Enhancement Nov 9, 2020
@zhenhuaw-me zhenhuaw-me self-assigned this Nov 9, 2020
@zhenhuaw-me
Copy link
Owner

Hi @erizmr thank you for reporting this issue. This is really a good example of the operator semantic divergence between TFLite and ONNX.

I checked the Resize example (resize_upsample_sizes_nearest_round_prefer_ceil_asymmetric example of Resize) of ONNX, it seems we can simply leave the tensor field as empty when creating the operator.

node = onnx.helper.make_node(
    'Resize',
    inputs=['X', '', '', 'sizes'],
    outputs=['Y'],
    mode='nearest',
    coordinate_transformation_mode='asymmetric',
    nearest_mode='round_prefer_ceil'
)

In tflite2onnx, we may have a helper to create empty tensor and make it as inputs of an operator. The challenge here is, graph level handling (e.g. data layout handling) may be impacted as we seem to assume tensors are not empty.

Do you have a simplified TFLite model that contains the resize_bilinear operator at hand? We may start with that to check what may happen.

@erizmr
Copy link
Contributor Author

erizmr commented Nov 9, 2020

Thansk for your reply. @jackwish
Here is a simple TFLITE model for resize_bilinear generated by tf.compat.v1.image.resize_bilinear:
model_resize_bilinear.tflite

import tensorflow as tf
# operator inputs
img = tf.keras.Input(dtype='float32', name='input', shape=([4, 4, 3]), batch_size=1)
scale = (8, 8)
# operator
resize = tf.compat.v1.image.resize_bilinear(img, scale, half_pixel_centers=True)
# build Keras model
model = tf.keras.Model(inputs=[img], outputs=[resize])
# convert to TFLite model
converter = tf.lite.TFLiteConverter.from_keras_model(model)
tflite_model = converter.convert()
# save it
with open('model_resize_bilinear.tflite', 'wb') as f:
  f.write(tflite_model)

There are two inputs: input(called images in docs TensorFlow, NHWC format) and size (a tuple of new_height and new_width after resizing, int32 only)

Maybe we can start with this simple model.

@zhenhuaw-me
Copy link
Owner

@erizmr thank you for the update. May I know which version of the TensorFlow you are using to generate this TFLite model? It's interesting that the new_height and new_width attributes are not listed in the TFLite model API doc which is automatically generated from TensorFlow 2.3.0.

Background: any attributes in TFLite model shall have a reader method in its parser, e.g. NewShape of Reshape.

@erizmr
Copy link
Contributor Author

erizmr commented Nov 10, 2020

The model is generated from TensorFlow 2.3.1. The doc for the method tf.compat.v1.image.resize_bilinear used is shown in the link.

I also checked the definition for this operator here:

It seems there is indeed no attrs named new_height and new_width, which is quite interesting.

@zhenhuaw-me
Copy link
Owner

That's the definition of TensorFlow operator which is not always same as its TFLite peer.

@erizmr
Copy link
Contributor Author

erizmr commented Nov 11, 2020

Hi @jackwish, I tried the empty tensors(from np.array([]) ) idea you have mentioned before. It actually works. Here is an example:

Maybe I can send a pull request to share the code if you wish.

@zhenhuaw-me
Copy link
Owner

Cool! A PR would be great! Thanks for the update @erizmr

@zhenhuaw-me zhenhuaw-me linked a pull request Nov 11, 2020 that will close this issue
v0.4 - Quantization Enhancement automation moved this from To Do to Done Nov 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

2 participants