[ONNX][TOPI][RELAY] Resize refactor #7883
Conversation
…size, which needs a dynamic implementation of crop and resize
| Spline Coefficient for Bicubic Interpolation | ||
|
|
||
| bicubic_exclude: int | ||
| Flag to exclude exterior of the image during bicubic interpolation |
There was a problem hiding this comment.
should this be a bool rather than int?
There was a problem hiding this comment.
ONNX is using an Int, so I did this to be consistent, but it might be clearer to use a bool. A wash?
There was a problem hiding this comment.
if onnx uses an in then im cool with it.
| elif coordinate_transformation_mode == "asymmetric": | ||
| in_y = y * scale_y | ||
| in_x = x * scale_x | ||
| elif coordinate_transformation_mode == "pytorch_half_pixel": |
There was a problem hiding this comment.
Do we need to update the pytorch and tf frontends to use these new modes?
There was a problem hiding this comment.
I'm honestly not sure what the frameworks are expecting or what tests they currently do or do not pass?
There was a problem hiding this comment.
The previous kernel only supported a small combination of these options, and the topi tests only support two. It looks like the TF importer only supports align_corners and asymmetric, but TF 1.15 also supports half pixel: https://www.tensorflow.org/versions/r1.15/api_docs/python/tf/image/resize_bilinear I don't see any of these options in TF 2.0
Pytorch seems to support asymmetric, align_corners, and half pixel, but it doesn't look like either importer is actually testing half_pixel
There was a problem hiding this comment.
Hmm I thought we were testing half_pixel in PT frontend but indeed it doesn't seem so. I think half_pixel corresponds to bilinear + align_corners=False but this combination is not there below
tvm/tests/python/frontend/pytorch/test_forward.py
Lines 1701 to 1703 in b24fbe7
There was a problem hiding this comment.
Tried adding verify_model(Upsample(size=(50, 50), mode="bilinear", align_corners=False), inp) to the test, it worked fortunately.
There was a problem hiding this comment.
Also when I added half_pixel option, I intended this option to correspond exactly to ONNX pytorch_half_pixel. In PT ONNX exporter, pytorch_half_pixel is introduced for bilinear + align_corners=False case, and that is also when half_pixel is used in our PT frontend. See https://github.com/pytorch/pytorch/blob/c371542efc31b1abfe6f388042aa3ab0cef935f2/torch/onnx/symbolic_helper.py#L517-L518
So we probably don't need pytorch_half_pixel. We can update half_pixel if necessary.
There was a problem hiding this comment.
The ONNX tests definitely have some failures if I use half_pixel when ONNX suggests pytorch_half_pixel, the issue is the if_then_else forcing some values to 0
There was a problem hiding this comment.
Ok I looked at the spec again, and indeed I realized they have two variants of half_pixel... I don't know what pytorch_half_pixel is for, probably only for the case where the resized shape is 1?
Does using pytorch_half_pixel for onnx tests that use half_pixel break them? If not we can probably override our definition of half_pixel to match pytorch_half_pixel.
There was a problem hiding this comment.
No, the only test ONNX currently has with a target shape 1 is the pytorch_half_pixel test. I separated them based on the ONNX Operator documentation and the ORT implementation, but it doesn't look like I have a test that hits the difference with a half_pixel mode.
There was a problem hiding this comment.
I would prefer to leave the separation for that future use case that's assuming half_pixel instead of pytorch_half_pixel for a resize to 1, even if we don't see it yet.
|
I'm hitting a small and intermittent atol issue with the TF tests on GPU. Before bumping it from 1e-5 to 2e-5, I'm trying to figure out if I actually changed the behavior for these options. |
|
I changed the order of operations on align_corner from out = a / b * x to out = x * a / b. That seems to have caused some rounding errors on GPU, if I revert the change the intermittency goes away. |
|
Thanks @mbrookhart, @masahi. This is now merged. |
* adds rounding mode for nearest neighbor, passing onnx unit tests for nearest neighbor * passing all linear test. passing all nearest tests except crop and resize, which needs a dynamic implementation of crop and resize * most of the bicubic tests are working * fix exclude outside * remove dead code * fix lint * fix defaults to match old implementation * fix lint * fix gpu tests * fix lint again * change order of operations to prevent GPU rounding errors
* adds rounding mode for nearest neighbor, passing onnx unit tests for nearest neighbor * passing all linear test. passing all nearest tests except crop and resize, which needs a dynamic implementation of crop and resize * most of the bicubic tests are working * fix exclude outside * remove dead code * fix lint * fix defaults to match old implementation * fix lint * fix gpu tests * fix lint again * change order of operations to prevent GPU rounding errors
* adds rounding mode for nearest neighbor, passing onnx unit tests for nearest neighbor * passing all linear test. passing all nearest tests except crop and resize, which needs a dynamic implementation of crop and resize * most of the bicubic tests are working * fix exclude outside * remove dead code * fix lint * fix defaults to match old implementation * fix lint * fix gpu tests * fix lint again * change order of operations to prevent GPU rounding errors
* adds rounding mode for nearest neighbor, passing onnx unit tests for nearest neighbor * passing all linear test. passing all nearest tests except crop and resize, which needs a dynamic implementation of crop and resize * most of the bicubic tests are working * fix exclude outside * remove dead code * fix lint * fix defaults to match old implementation * fix lint * fix gpu tests * fix lint again * change order of operations to prevent GPU rounding errors
* adds rounding mode for nearest neighbor, passing onnx unit tests for nearest neighbor * passing all linear test. passing all nearest tests except crop and resize, which needs a dynamic implementation of crop and resize * most of the bicubic tests are working * fix exclude outside * remove dead code * fix lint * fix defaults to match old implementation * fix lint * fix gpu tests * fix lint again * change order of operations to prevent GPU rounding errors
* adds rounding mode for nearest neighbor, passing onnx unit tests for nearest neighbor * passing all linear test. passing all nearest tests except crop and resize, which needs a dynamic implementation of crop and resize * most of the bicubic tests are working * fix exclude outside * remove dead code * fix lint * fix defaults to match old implementation * fix lint * fix gpu tests * fix lint again * change order of operations to prevent GPU rounding errors
* adds rounding mode for nearest neighbor, passing onnx unit tests for nearest neighbor * passing all linear test. passing all nearest tests except crop and resize, which needs a dynamic implementation of crop and resize * most of the bicubic tests are working * fix exclude outside * remove dead code * fix lint * fix defaults to match old implementation * fix lint * fix gpu tests * fix lint again * change order of operations to prevent GPU rounding errors
This refactors the resize kernel to make it compatible with the many options in ONNX's tests. 🤞 I didn't break anything elsewhere.
cc @masahi @jwfromm @yongwww @electriclilies