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 2020/10/14 01:56:22 UTC
[GitHub] [incubator-tvm] jwfromm opened a new pull request #6681: [Relay][Frontend][Onnx] Allow A to B broadcasting of batch_matmul and reverse strided slice
jwfromm opened a new pull request #6681:
URL: https://github.com/apache/incubator-tvm/pull/6681
This PR contains two otherwise unrelated bug fixes / improvements that allow us to import a class of segmentation models using the Onnx frontend.
The first is the ability to broadcast the batch dimension of the left hand side argument of `batch_matmul` to to the right hand side argument, where we previously only allowed rhs to lhs broadcasting. There are a few bug fixes in relay to enable this.
The second is a fix to our strided slice shape inference to allow reverse slicing, specifically when the end is a negative number, indicating that we should iterate all the way to the first element. A test case in relay is added to catch this in the future.
Finally, I removed some unneeded type inference and shape checking in padding.
----------------------------------------------------------------
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] [incubator-tvm] mbrookhart commented on a change in pull request #6681: [Relay][Frontend][Onnx] Allow A to B broadcasting of batch_matmul and reverse strided slice
Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #6681:
URL: https://github.com/apache/incubator-tvm/pull/6681#discussion_r505048637
##########
File path: python/tvm/relay/frontend/onnx.py
##########
@@ -532,10 +534,18 @@ def flatten_to_3d(x, x_shape):
b = _op.transpose(b, [0, 2, 1])
# Perform a batch matmul.
output = _op.nn.batch_matmul(a, b)
+ # Determine the output batch dimension.
+ if a_rank >= b_rank:
+ out_batch = _op.strided_slice(a_shape, [0], [infer_shape(a_shape)[0] - 2])
+ else:
+ out_batch = _op.strided_slice(b_shape, [0], [infer_shape(b_shape)[0] - 2])
Review comment:
LGTM!
----------------------------------------------------------------
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] [incubator-tvm] masahi commented on pull request #6681: [Relay][Frontend][Onnx] Allow A to B broadcasting of batch_matmul and reverse strided slice
Posted by GitBox <gi...@apache.org>.
masahi commented on pull request #6681:
URL: https://github.com/apache/incubator-tvm/pull/6681#issuecomment-708878955
Thanks @jwfromm @mbrookhart
----------------------------------------------------------------
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] [incubator-tvm] masahi commented on pull request #6681: [Relay][Frontend][Onnx] Allow A to B broadcasting of batch_matmul and reverse strided slice
Posted by GitBox <gi...@apache.org>.
masahi commented on pull request #6681:
URL: https://github.com/apache/incubator-tvm/pull/6681#issuecomment-708130142
hey @comaniac @merrymercy can you check if the error from evolutionary search test https://ci.tlcpack.ai/blue/organizations/jenkins/tvm/detail/PR-6681/1/pipeline/ can be flaky?
----------------------------------------------------------------
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] [incubator-tvm] masahi merged pull request #6681: [Relay][Frontend][Onnx] Allow A to B broadcasting of batch_matmul and reverse strided slice
Posted by GitBox <gi...@apache.org>.
masahi merged pull request #6681:
URL: https://github.com/apache/incubator-tvm/pull/6681
----------------------------------------------------------------
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] [incubator-tvm] jwfromm commented on a change in pull request #6681: [Relay][Frontend][Onnx] Allow A to B broadcasting of batch_matmul and reverse strided slice
Posted by GitBox <gi...@apache.org>.
jwfromm commented on a change in pull request #6681:
URL: https://github.com/apache/incubator-tvm/pull/6681#discussion_r505043967
##########
File path: python/tvm/relay/frontend/onnx.py
##########
@@ -532,10 +534,18 @@ def flatten_to_3d(x, x_shape):
b = _op.transpose(b, [0, 2, 1])
# Perform a batch matmul.
output = _op.nn.batch_matmul(a, b)
+ # Determine the output batch dimension.
+ if a_rank >= b_rank:
+ out_batch = _op.strided_slice(a_shape, [0], [infer_shape(a_shape)[0] - 2])
+ else:
+ out_batch = _op.strided_slice(b_shape, [0], [infer_shape(b_shape)[0] - 2])
Review comment:
Can you check out the changes in the most recent commit? It addresses the corner case but does insert some extra operations. Do you think its worth 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] [incubator-tvm] jwfromm edited a comment on pull request #6681: [Relay][Frontend][Onnx] Allow A to B broadcasting of batch_matmul and reverse strided slice
Posted by GitBox <gi...@apache.org>.
jwfromm edited a comment on pull request #6681:
URL: https://github.com/apache/incubator-tvm/pull/6681#issuecomment-708121440
It seems very unlikely that these changes would cause that error. I'll try to rebuild. It looks like it passes locally as well.
----------------------------------------------------------------
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] [incubator-tvm] jwfromm commented on pull request #6681: [Relay][Frontend][Onnx] Allow A to B broadcasting of batch_matmul and reverse strided slice
Posted by GitBox <gi...@apache.org>.
jwfromm commented on pull request #6681:
URL: https://github.com/apache/incubator-tvm/pull/6681#issuecomment-708106363
@mbrookhart @masahi can you guys take a look at this PR?
----------------------------------------------------------------
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] [incubator-tvm] merrymercy edited a comment on pull request #6681: [Relay][Frontend][Onnx] Allow A to B broadcasting of batch_matmul and reverse strided slice
Posted by GitBox <gi...@apache.org>.
merrymercy edited a comment on pull request #6681:
URL: https://github.com/apache/incubator-tvm/pull/6681#issuecomment-708199204
----------------------------------------------------------------
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] [incubator-tvm] merrymercy commented on pull request #6681: [Relay][Frontend][Onnx] Allow A to B broadcasting of batch_matmul and reverse strided slice
Posted by GitBox <gi...@apache.org>.
merrymercy commented on pull request #6681:
URL: https://github.com/apache/incubator-tvm/pull/6681#issuecomment-708199204
@masahi It seems to be an internal error of the auto-scheduler rather than a flaky test.
----------------------------------------------------------------
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] [incubator-tvm] jwfromm commented on pull request #6681: [Relay][Frontend][Onnx] Allow A to B broadcasting of batch_matmul and reverse strided slice
Posted by GitBox <gi...@apache.org>.
jwfromm commented on pull request #6681:
URL: https://github.com/apache/incubator-tvm/pull/6681#issuecomment-708121440
It seems very unlikely that these changes would cause that error. I'll try to rebuild.
----------------------------------------------------------------
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] [incubator-tvm] mbrookhart commented on a change in pull request #6681: [Relay][Frontend][Onnx] Allow A to B broadcasting of batch_matmul and reverse strided slice
Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #6681:
URL: https://github.com/apache/incubator-tvm/pull/6681#discussion_r505024398
##########
File path: python/tvm/relay/frontend/onnx.py
##########
@@ -532,10 +534,18 @@ def flatten_to_3d(x, x_shape):
b = _op.transpose(b, [0, 2, 1])
# Perform a batch matmul.
output = _op.nn.batch_matmul(a, b)
+ # Determine the output batch dimension.
+ if a_rank >= b_rank:
+ out_batch = _op.strided_slice(a_shape, [0], [infer_shape(a_shape)[0] - 2])
+ else:
+ out_batch = _op.strided_slice(b_shape, [0], [infer_shape(b_shape)[0] - 2])
Review comment:
I think the only place this might fail is if the input shapes are both 3D, but the a shape starts with a 1, i.e a.shape = (1, 6, 6) and b.shape = (3, 6, 6) and the output shape actually needs to be (3, 6, 6).
I'm not sure how to handle that case correctly, though.
----------------------------------------------------------------
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] [incubator-tvm] masahi commented on pull request #6681: [Relay][Frontend][Onnx] Allow A to B broadcasting of batch_matmul and reverse strided slice
Posted by GitBox <gi...@apache.org>.
masahi commented on pull request #6681:
URL: https://github.com/apache/incubator-tvm/pull/6681#issuecomment-708120348
There is an error from auto schedular test. Not sure if it is related or flaky
----------------------------------------------------------------
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