You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2021/04/19 15:53:50 UTC

[GitHub] [tvm] mbrookhart opened a new pull request #7883: Resize refactor

mbrookhart opened a new pull request #7883:
URL: https://github.com/apache/tvm/pull/7883


   This refactors the resize kernel to make it compatible with the many options in ONNX's tests. :crossed_fingers: I didn't break anything elsewhere.
   
   cc @masahi @jwfromm @yongwww @electriclilies 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] mbrookhart commented on pull request #7883: [ONNX][TOPI][RELAY] Resize refactor

Posted by GitBox <gi...@apache.org>.
mbrookhart commented on pull request #7883:
URL: https://github.com/apache/tvm/pull/7883#issuecomment-824129779


   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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] mbrookhart commented on a change in pull request #7883: [ONNX][TOPI][RELAY] Resize refactor

Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #7883:
URL: https://github.com/apache/tvm/pull/7883#discussion_r616158860



##########
File path: python/tvm/topi/image/resize.py
##########
@@ -59,6 +59,33 @@ def get_2d_pixel(data, layout, boxes, image_height, image_width, n, c, y, x, cc,
     return data(n, c, y, x, cc).astype("float")
 
 
+def get_iny_inx(
+    y, x, image_height, image_width, target_height, target_width, coordinate_transformation_mode
+):
+    scale_y = te.div(image_height.astype("float"), target_height.astype("float"))
+    scale_x = te.div(image_width.astype("float"), target_width.astype("float"))
+    if coordinate_transformation_mode == "half_pixel":
+        in_y = (y + 0.5) * scale_y - 0.5
+        in_x = (x + 0.5) * scale_x - 0.5
+    elif coordinate_transformation_mode == "align_corners":
+        in_y = y * (image_height - 1).astype("float") / (target_height - 1)
+        in_x = x * (image_width - 1).astype("float") / (target_width - 1)
+    elif coordinate_transformation_mode == "asymmetric":
+        in_y = y * scale_y
+        in_x = x * scale_x
+    elif coordinate_transformation_mode == "pytorch_half_pixel":

Review 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 assymetric, 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 assymetric, align_corners, and half pixel, but it doesn't look like either importer is actually testing half_pixel
   
   
   
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] masahi commented on a change in pull request #7883: [ONNX][TOPI][RELAY] Resize refactor

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #7883:
URL: https://github.com/apache/tvm/pull/7883#discussion_r616257175



##########
File path: python/tvm/topi/image/resize.py
##########
@@ -59,6 +59,33 @@ def get_2d_pixel(data, layout, boxes, image_height, image_width, n, c, y, x, cc,
     return data(n, c, y, x, cc).astype("float")
 
 
+def get_iny_inx(
+    y, x, image_height, image_width, target_height, target_width, coordinate_transformation_mode
+):
+    scale_y = te.div(image_height.astype("float"), target_height.astype("float"))
+    scale_x = te.div(image_width.astype("float"), target_width.astype("float"))
+    if coordinate_transformation_mode == "half_pixel":
+        in_y = (y + 0.5) * scale_y - 0.5
+        in_x = (x + 0.5) * scale_x - 0.5
+    elif coordinate_transformation_mode == "align_corners":
+        in_y = y * (image_height - 1).astype("float") / (target_height - 1)
+        in_x = x * (image_width - 1).astype("float") / (target_width - 1)
+    elif coordinate_transformation_mode == "asymmetric":
+        in_y = y * scale_y
+        in_x = x * scale_x
+    elif coordinate_transformation_mode == "pytorch_half_pixel":

Review 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.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] masahi commented on a change in pull request #7883: [ONNX][TOPI][RELAY] Resize refactor

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #7883:
URL: https://github.com/apache/tvm/pull/7883#discussion_r616257175



##########
File path: python/tvm/topi/image/resize.py
##########
@@ -59,6 +59,33 @@ def get_2d_pixel(data, layout, boxes, image_height, image_width, n, c, y, x, cc,
     return data(n, c, y, x, cc).astype("float")
 
 
+def get_iny_inx(
+    y, x, image_height, image_width, target_height, target_width, coordinate_transformation_mode
+):
+    scale_y = te.div(image_height.astype("float"), target_height.astype("float"))
+    scale_x = te.div(image_width.astype("float"), target_width.astype("float"))
+    if coordinate_transformation_mode == "half_pixel":
+        in_y = (y + 0.5) * scale_y - 0.5
+        in_x = (x + 0.5) * scale_x - 0.5
+    elif coordinate_transformation_mode == "align_corners":
+        in_y = y * (image_height - 1).astype("float") / (target_height - 1)
+        in_x = x * (image_width - 1).astype("float") / (target_width - 1)
+    elif coordinate_transformation_mode == "asymmetric":
+        in_y = y * scale_y
+        in_x = x * scale_x
+    elif coordinate_transformation_mode == "pytorch_half_pixel":

Review 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.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] jwfromm commented on pull request #7883: [ONNX][TOPI][RELAY] Resize refactor

Posted by GitBox <gi...@apache.org>.
jwfromm commented on pull request #7883:
URL: https://github.com/apache/tvm/pull/7883#issuecomment-824372527


   Thanks @mbrookhart, @masahi. This is now merged.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] mbrookhart commented on a change in pull request #7883: [ONNX][TOPI][RELAY] Resize refactor

Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #7883:
URL: https://github.com/apache/tvm/pull/7883#discussion_r615979167



##########
File path: python/tvm/topi/image/resize.py
##########
@@ -59,6 +59,33 @@ def get_2d_pixel(data, layout, boxes, image_height, image_width, n, c, y, x, cc,
     return data(n, c, y, x, cc).astype("float")
 
 
+def get_iny_inx(
+    y, x, image_height, image_width, target_height, target_width, coordinate_transformation_mode
+):
+    scale_y = te.div(image_height.astype("float"), target_height.astype("float"))
+    scale_x = te.div(image_width.astype("float"), target_width.astype("float"))
+    if coordinate_transformation_mode == "half_pixel":
+        in_y = (y + 0.5) * scale_y - 0.5
+        in_x = (x + 0.5) * scale_x - 0.5
+    elif coordinate_transformation_mode == "align_corners":
+        in_y = y * (image_height - 1).astype("float") / (target_height - 1)
+        in_x = x * (image_width - 1).astype("float") / (target_width - 1)
+    elif coordinate_transformation_mode == "asymmetric":
+        in_y = y * scale_y
+        in_x = x * scale_x
+    elif coordinate_transformation_mode == "pytorch_half_pixel":

Review comment:
       I'm honestly not sure what the frameworks are expecting or what tests they currently do or do not pass?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] masahi commented on a change in pull request #7883: [ONNX][TOPI][RELAY] Resize refactor

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #7883:
URL: https://github.com/apache/tvm/pull/7883#discussion_r616233259



##########
File path: python/tvm/topi/image/resize.py
##########
@@ -59,6 +59,33 @@ def get_2d_pixel(data, layout, boxes, image_height, image_width, n, c, y, x, cc,
     return data(n, c, y, x, cc).astype("float")
 
 
+def get_iny_inx(
+    y, x, image_height, image_width, target_height, target_width, coordinate_transformation_mode
+):
+    scale_y = te.div(image_height.astype("float"), target_height.astype("float"))
+    scale_x = te.div(image_width.astype("float"), target_width.astype("float"))
+    if coordinate_transformation_mode == "half_pixel":
+        in_y = (y + 0.5) * scale_y - 0.5
+        in_x = (x + 0.5) * scale_x - 0.5
+    elif coordinate_transformation_mode == "align_corners":
+        in_y = y * (image_height - 1).astype("float") / (target_height - 1)
+        in_x = x * (image_width - 1).astype("float") / (target_width - 1)
+    elif coordinate_transformation_mode == "asymmetric":
+        in_y = y * scale_y
+        in_x = x * scale_x
+    elif coordinate_transformation_mode == "pytorch_half_pixel":

Review comment:
       Tried adding `verify_model(Upsample(size=(50, 50), mode="bilinear", align_corners=False), inp)` to the test, it worked fortunately.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] mbrookhart commented on a change in pull request #7883: [ONNX][TOPI][RELAY] Resize refactor

Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #7883:
URL: https://github.com/apache/tvm/pull/7883#discussion_r615977936



##########
File path: python/tvm/relay/op/image/image.py
##########
@@ -58,6 +61,16 @@ def resize(
         Refer to the ONNX Resize operator specification for details.
         [half_pixel, align_corners, asymmetric]
 
+    rounding_method: string, optional
+        indicates how to find the "nearest" pixel in nearest_neighbor method
+        [round, floor, ceil]
+
+    bicubic_alpha: float
+        Spline Coefficient for Bicubic Interpolation
+
+    bicubic_exclude: int
+            Flag to exclude exterior of the image during bicubic interpolation

Review comment:
       ONNX is using an Int, so I did this to be consistent, but it might be clearer to use a bool. A wash?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] mbrookhart commented on a change in pull request #7883: [ONNX][TOPI][RELAY] Resize refactor

Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #7883:
URL: https://github.com/apache/tvm/pull/7883#discussion_r617141030



##########
File path: python/tvm/topi/image/resize.py
##########
@@ -59,6 +59,33 @@ def get_2d_pixel(data, layout, boxes, image_height, image_width, n, c, y, x, cc,
     return data(n, c, y, x, cc).astype("float")
 
 
+def get_iny_inx(
+    y, x, image_height, image_width, target_height, target_width, coordinate_transformation_mode
+):
+    scale_y = te.div(image_height.astype("float"), target_height.astype("float"))
+    scale_x = te.div(image_width.astype("float"), target_width.astype("float"))
+    if coordinate_transformation_mode == "half_pixel":
+        in_y = (y + 0.5) * scale_y - 0.5
+        in_x = (x + 0.5) * scale_x - 0.5
+    elif coordinate_transformation_mode == "align_corners":
+        in_y = y * (image_height - 1).astype("float") / (target_height - 1)
+        in_x = x * (image_width - 1).astype("float") / (target_width - 1)
+    elif coordinate_transformation_mode == "asymmetric":
+        in_y = y * scale_y
+        in_x = x * scale_x
+    elif coordinate_transformation_mode == "pytorch_half_pixel":

Review 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.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] masahi commented on a change in pull request #7883: [ONNX][TOPI][RELAY] Resize refactor

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #7883:
URL: https://github.com/apache/tvm/pull/7883#discussion_r617079571



##########
File path: python/tvm/topi/image/resize.py
##########
@@ -59,6 +59,33 @@ def get_2d_pixel(data, layout, boxes, image_height, image_width, n, c, y, x, cc,
     return data(n, c, y, x, cc).astype("float")
 
 
+def get_iny_inx(
+    y, x, image_height, image_width, target_height, target_width, coordinate_transformation_mode
+):
+    scale_y = te.div(image_height.astype("float"), target_height.astype("float"))
+    scale_x = te.div(image_width.astype("float"), target_width.astype("float"))
+    if coordinate_transformation_mode == "half_pixel":
+        in_y = (y + 0.5) * scale_y - 0.5
+        in_x = (x + 0.5) * scale_x - 0.5
+    elif coordinate_transformation_mode == "align_corners":
+        in_y = y * (image_height - 1).astype("float") / (target_height - 1)
+        in_x = x * (image_width - 1).astype("float") / (target_width - 1)
+    elif coordinate_transformation_mode == "asymmetric":
+        in_y = y * scale_y
+        in_x = x * scale_x
+    elif coordinate_transformation_mode == "pytorch_half_pixel":

Review 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`. 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] masahi commented on a change in pull request #7883: [ONNX][TOPI][RELAY] Resize refactor

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #7883:
URL: https://github.com/apache/tvm/pull/7883#discussion_r616257175



##########
File path: python/tvm/topi/image/resize.py
##########
@@ -59,6 +59,33 @@ def get_2d_pixel(data, layout, boxes, image_height, image_width, n, c, y, x, cc,
     return data(n, c, y, x, cc).astype("float")
 
 
+def get_iny_inx(
+    y, x, image_height, image_width, target_height, target_width, coordinate_transformation_mode
+):
+    scale_y = te.div(image_height.astype("float"), target_height.astype("float"))
+    scale_x = te.div(image_width.astype("float"), target_width.astype("float"))
+    if coordinate_transformation_mode == "half_pixel":
+        in_y = (y + 0.5) * scale_y - 0.5
+        in_x = (x + 0.5) * scale_x - 0.5
+    elif coordinate_transformation_mode == "align_corners":
+        in_y = y * (image_height - 1).astype("float") / (target_height - 1)
+        in_x = x * (image_width - 1).astype("float") / (target_width - 1)
+    elif coordinate_transformation_mode == "asymmetric":
+        in_y = y * scale_y
+        in_x = x * scale_x
+    elif coordinate_transformation_mode == "pytorch_half_pixel":

Review 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.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] mbrookhart commented on a change in pull request #7883: [ONNX][TOPI][RELAY] Resize refactor

Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #7883:
URL: https://github.com/apache/tvm/pull/7883#discussion_r616158860



##########
File path: python/tvm/topi/image/resize.py
##########
@@ -59,6 +59,33 @@ def get_2d_pixel(data, layout, boxes, image_height, image_width, n, c, y, x, cc,
     return data(n, c, y, x, cc).astype("float")
 
 
+def get_iny_inx(
+    y, x, image_height, image_width, target_height, target_width, coordinate_transformation_mode
+):
+    scale_y = te.div(image_height.astype("float"), target_height.astype("float"))
+    scale_x = te.div(image_width.astype("float"), target_width.astype("float"))
+    if coordinate_transformation_mode == "half_pixel":
+        in_y = (y + 0.5) * scale_y - 0.5
+        in_x = (x + 0.5) * scale_x - 0.5
+    elif coordinate_transformation_mode == "align_corners":
+        in_y = y * (image_height - 1).astype("float") / (target_height - 1)
+        in_x = x * (image_width - 1).astype("float") / (target_width - 1)
+    elif coordinate_transformation_mode == "asymmetric":
+        in_y = y * scale_y
+        in_x = x * scale_x
+    elif coordinate_transformation_mode == "pytorch_half_pixel":

Review 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
   
   
   
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] jwfromm commented on a change in pull request #7883: [ONNX][TOPI][RELAY] Resize refactor

Posted by GitBox <gi...@apache.org>.
jwfromm commented on a change in pull request #7883:
URL: https://github.com/apache/tvm/pull/7883#discussion_r617891826



##########
File path: python/tvm/relay/op/image/image.py
##########
@@ -58,6 +61,16 @@ def resize(
         Refer to the ONNX Resize operator specification for details.
         [half_pixel, align_corners, asymmetric]
 
+    rounding_method: string, optional
+        indicates how to find the "nearest" pixel in nearest_neighbor method
+        [round, floor, ceil]
+
+    bicubic_alpha: float
+        Spline Coefficient for Bicubic Interpolation
+
+    bicubic_exclude: int
+            Flag to exclude exterior of the image during bicubic interpolation

Review comment:
       if onnx uses an in then im cool with it.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] jwfromm merged pull request #7883: [ONNX][TOPI][RELAY] Resize refactor

Posted by GitBox <gi...@apache.org>.
jwfromm merged pull request #7883:
URL: https://github.com/apache/tvm/pull/7883


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] masahi commented on a change in pull request #7883: [ONNX][TOPI][RELAY] Resize refactor

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #7883:
URL: https://github.com/apache/tvm/pull/7883#discussion_r617079571



##########
File path: python/tvm/topi/image/resize.py
##########
@@ -59,6 +59,33 @@ def get_2d_pixel(data, layout, boxes, image_height, image_width, n, c, y, x, cc,
     return data(n, c, y, x, cc).astype("float")
 
 
+def get_iny_inx(
+    y, x, image_height, image_width, target_height, target_width, coordinate_transformation_mode
+):
+    scale_y = te.div(image_height.astype("float"), target_height.astype("float"))
+    scale_x = te.div(image_width.astype("float"), target_width.astype("float"))
+    if coordinate_transformation_mode == "half_pixel":
+        in_y = (y + 0.5) * scale_y - 0.5
+        in_x = (x + 0.5) * scale_x - 0.5
+    elif coordinate_transformation_mode == "align_corners":
+        in_y = y * (image_height - 1).astype("float") / (target_height - 1)
+        in_x = x * (image_width - 1).astype("float") / (target_width - 1)
+    elif coordinate_transformation_mode == "asymmetric":
+        in_y = y * scale_y
+        in_x = x * scale_x
+    elif coordinate_transformation_mode == "pytorch_half_pixel":

Review 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?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] jwfromm commented on a change in pull request #7883: [ONNX][TOPI][RELAY] Resize refactor

Posted by GitBox <gi...@apache.org>.
jwfromm commented on a change in pull request #7883:
URL: https://github.com/apache/tvm/pull/7883#discussion_r615973599



##########
File path: python/tvm/relay/op/image/image.py
##########
@@ -58,6 +61,16 @@ def resize(
         Refer to the ONNX Resize operator specification for details.
         [half_pixel, align_corners, asymmetric]
 
+    rounding_method: string, optional
+        indicates how to find the "nearest" pixel in nearest_neighbor method
+        [round, floor, ceil]
+
+    bicubic_alpha: float
+        Spline Coefficient for Bicubic Interpolation
+
+    bicubic_exclude: int
+            Flag to exclude exterior of the image during bicubic interpolation

Review comment:
       should this be a bool rather than int?

##########
File path: python/tvm/topi/image/resize.py
##########
@@ -59,6 +59,33 @@ def get_2d_pixel(data, layout, boxes, image_height, image_width, n, c, y, x, cc,
     return data(n, c, y, x, cc).astype("float")
 
 
+def get_iny_inx(
+    y, x, image_height, image_width, target_height, target_width, coordinate_transformation_mode
+):
+    scale_y = te.div(image_height.astype("float"), target_height.astype("float"))
+    scale_x = te.div(image_width.astype("float"), target_width.astype("float"))
+    if coordinate_transformation_mode == "half_pixel":
+        in_y = (y + 0.5) * scale_y - 0.5
+        in_x = (x + 0.5) * scale_x - 0.5
+    elif coordinate_transformation_mode == "align_corners":
+        in_y = y * (image_height - 1).astype("float") / (target_height - 1)
+        in_x = x * (image_width - 1).astype("float") / (target_width - 1)
+    elif coordinate_transformation_mode == "asymmetric":
+        in_y = y * scale_y
+        in_x = x * scale_x
+    elif coordinate_transformation_mode == "pytorch_half_pixel":

Review comment:
       Do we need to update the pytorch and tf frontends to use these new modes?

##########
File path: python/tvm/topi/image/resize.py
##########
@@ -443,6 +476,7 @@ def resize_bicubic(
     target_width : integer
         The target resized image width
 
+

Review comment:
       this extra empty line probably isnt supposed to be there.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] mbrookhart commented on a change in pull request #7883: [ONNX][TOPI][RELAY] Resize refactor

Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #7883:
URL: https://github.com/apache/tvm/pull/7883#discussion_r616749985



##########
File path: python/tvm/topi/image/resize.py
##########
@@ -59,6 +59,33 @@ def get_2d_pixel(data, layout, boxes, image_height, image_width, n, c, y, x, cc,
     return data(n, c, y, x, cc).astype("float")
 
 
+def get_iny_inx(
+    y, x, image_height, image_width, target_height, target_width, coordinate_transformation_mode
+):
+    scale_y = te.div(image_height.astype("float"), target_height.astype("float"))
+    scale_x = te.div(image_width.astype("float"), target_width.astype("float"))
+    if coordinate_transformation_mode == "half_pixel":
+        in_y = (y + 0.5) * scale_y - 0.5
+        in_x = (x + 0.5) * scale_x - 0.5
+    elif coordinate_transformation_mode == "align_corners":
+        in_y = y * (image_height - 1).astype("float") / (target_height - 1)
+        in_x = x * (image_width - 1).astype("float") / (target_width - 1)
+    elif coordinate_transformation_mode == "asymmetric":
+        in_y = y * scale_y
+        in_x = x * scale_x
+    elif coordinate_transformation_mode == "pytorch_half_pixel":

Review 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




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] masahi commented on a change in pull request #7883: [ONNX][TOPI][RELAY] Resize refactor

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #7883:
URL: https://github.com/apache/tvm/pull/7883#discussion_r616231499



##########
File path: python/tvm/topi/image/resize.py
##########
@@ -59,6 +59,33 @@ def get_2d_pixel(data, layout, boxes, image_height, image_width, n, c, y, x, cc,
     return data(n, c, y, x, cc).astype("float")
 
 
+def get_iny_inx(
+    y, x, image_height, image_width, target_height, target_width, coordinate_transformation_mode
+):
+    scale_y = te.div(image_height.astype("float"), target_height.astype("float"))
+    scale_x = te.div(image_width.astype("float"), target_width.astype("float"))
+    if coordinate_transformation_mode == "half_pixel":
+        in_y = (y + 0.5) * scale_y - 0.5
+        in_x = (x + 0.5) * scale_x - 0.5
+    elif coordinate_transformation_mode == "align_corners":
+        in_y = y * (image_height - 1).astype("float") / (target_height - 1)
+        in_x = x * (image_width - 1).astype("float") / (target_width - 1)
+    elif coordinate_transformation_mode == "asymmetric":
+        in_y = y * scale_y
+        in_x = x * scale_x
+    elif coordinate_transformation_mode == "pytorch_half_pixel":

Review 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
   
   https://github.com/apache/tvm/blob/b24fbe7dee10195b6585634c0625828fe8624d5f/tests/python/frontend/pytorch/test_forward.py#L1701-L1703 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] mbrookhart commented on pull request #7883: [ONNX][TOPI][RELAY] Resize refactor

Posted by GitBox <gi...@apache.org>.
mbrookhart commented on pull request #7883:
URL: https://github.com/apache/tvm/pull/7883#issuecomment-824136011


   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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] mbrookhart commented on a change in pull request #7883: [ONNX][TOPI][RELAY] Resize refactor

Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #7883:
URL: https://github.com/apache/tvm/pull/7883#discussion_r617140617



##########
File path: python/tvm/topi/image/resize.py
##########
@@ -59,6 +59,33 @@ def get_2d_pixel(data, layout, boxes, image_height, image_width, n, c, y, x, cc,
     return data(n, c, y, x, cc).astype("float")
 
 
+def get_iny_inx(
+    y, x, image_height, image_width, target_height, target_width, coordinate_transformation_mode
+):
+    scale_y = te.div(image_height.astype("float"), target_height.astype("float"))
+    scale_x = te.div(image_width.astype("float"), target_width.astype("float"))
+    if coordinate_transformation_mode == "half_pixel":
+        in_y = (y + 0.5) * scale_y - 0.5
+        in_x = (x + 0.5) * scale_x - 0.5
+    elif coordinate_transformation_mode == "align_corners":
+        in_y = y * (image_height - 1).astype("float") / (target_height - 1)
+        in_x = x * (image_width - 1).astype("float") / (target_width - 1)
+    elif coordinate_transformation_mode == "asymmetric":
+        in_y = y * scale_y
+        in_x = x * scale_x
+    elif coordinate_transformation_mode == "pytorch_half_pixel":

Review 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.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] masahi commented on a change in pull request #7883: [ONNX][TOPI][RELAY] Resize refactor

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #7883:
URL: https://github.com/apache/tvm/pull/7883#discussion_r617079571



##########
File path: python/tvm/topi/image/resize.py
##########
@@ -59,6 +59,33 @@ def get_2d_pixel(data, layout, boxes, image_height, image_width, n, c, y, x, cc,
     return data(n, c, y, x, cc).astype("float")
 
 
+def get_iny_inx(
+    y, x, image_height, image_width, target_height, target_width, coordinate_transformation_mode
+):
+    scale_y = te.div(image_height.astype("float"), target_height.astype("float"))
+    scale_x = te.div(image_width.astype("float"), target_width.astype("float"))
+    if coordinate_transformation_mode == "half_pixel":
+        in_y = (y + 0.5) * scale_y - 0.5
+        in_x = (x + 0.5) * scale_x - 0.5
+    elif coordinate_transformation_mode == "align_corners":
+        in_y = y * (image_height - 1).astype("float") / (target_height - 1)
+        in_x = x * (image_width - 1).astype("float") / (target_width - 1)
+    elif coordinate_transformation_mode == "asymmetric":
+        in_y = y * scale_y
+        in_x = x * scale_x
+    elif coordinate_transformation_mode == "pytorch_half_pixel":

Review 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?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org