[NNVM] Add ONNX upsample converter #1591
Conversation
Jokeren
changed the title
|
Few comments in the context of Upsampling
|
|
@srkreddy1238, Thanks for your comment! I got a problem in finding the accurate definition of upsample in NNVM's doc. https://docs.tvm.ai/nnvm_top.html#overview-of-operators On the link above, I could not find instructions for upsample. |
|
I think ONNX definition of upsampling op is too generic (arbitrary dimension/scaling factors) without good use cases. In practice, upsampling op is almost always used for image-like inputs with the same scaling factor in spatial dimensions. @Jokeren @srkreddy1238 I think for now, supporting 4d inputs with nearest/bilinear should be enough. |
|
Great. So we could limit the ONNX support as below for now.
Hint: for order 3 and 2 we could handle expand dims and use existing upsampling followed by a squeeze. |
|
Of course, if there is indeed a need for such generic upsampling op, we can add support for it later. But then we need to update our TOPI upsampling op anyway. So for this PR, supporting @srkreddy1238's list above should be good. |
|
Thanks, I will fix the problem and update features in the next few days. |
|
@srkreddy1238 Can you take a look at the failed check? I double checked my local tests with both python3 and python2 and there's no such a problem. |
|
ONNX version ? |
|
Also try printing attr |
|
@Jokeren Please tell us when this is ready for review. |
|
@masahi , Sure |
|
Note there's a warning in a cpp file after adding the |
Jokeren
changed the title
|
@masahi , it looks good, except for the warning I mentioned before. Could you take a look? Thank you! |
| in_array = np.random.uniform(size=in_shape).astype('float32') | ||
| out_array = np.zeros(out_shape).astype('float32') | ||
|
|
||
| def upsample_bilinear(out_shape, out_array, in_shape, in_array): |
There was a problem hiding this comment.
Use function in topi/python/topi/testing/bilinear_resize_python.py instead of duplicating here. It's fine to import topi.
| in_array = np.random.uniform(size=in_shape).astype('float32') | ||
| out_array = np.zeros(out_shape).astype('float32') | ||
|
|
||
| def upsample_nearest(out_shape, out_array, in_array): |
There was a problem hiding this comment.
Use function in topi/python/topi/testing/upsampling_python.py
|
|
||
| @classmethod | ||
| def _impl_v7(cls, inputs, attr, params): | ||
| scales = attr.get('scales') |
There was a problem hiding this comment.
Add assert here to check
len(scales) == 4
scales[0] == 1.0 && scales[1] == 1.0
scales[2] == scales[3]
This is the only case we can support.
There was a problem hiding this comment.
Yes, we support only this case.
Should assert if it goes out of this assumption.
| scale = 2 | ||
| in_shape = (1, 1, 3, 3) | ||
| out_shape = (1, 1, 3 * scale, 3 * scale) | ||
| scale = float(scale) |
There was a problem hiding this comment.
Are you sure the result of float(scale) is float32? The "Cast" warning you got might be due to this.
| scale = 2 | ||
| in_shape = (1, 1, 3, 3) | ||
| out_shape = (1, 1, 3 * scale, 3 * scale) | ||
| scale = float(scale) |
There was a problem hiding this comment.
The same comment as in upsample nearest. Make sure scale is floar32.
|
@srkreddy1238 Can you give your review? |
|
|
||
| if __name__ == '__main__': | ||
| # verify_super_resolution_example() | ||
| # verify_squeezenet1_1() | ||
| # verify_lenet() | ||
| verify_resnet18() | ||
| #verify_resnet18() |
|
@Jokeren Have you solved the warning issue? |
|
The warning might be false alarm from integer analysis and it is fine to ignore it for now |
I suppose NNVM doesn't support the bilinear (trilinear) mode of upsample operator. And I am not sure if I need to check the of dimension of the input tensor, because ONNX supports the input tensor to be arbitrary dimension while NNVM assumes the input tensor is 4D. @tqchen