You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by "jikechao (via GitHub)" <gi...@apache.org> on 2023/04/09 16:20:59 UTC
[GitHub] [tvm] jikechao opened a new pull request, #14546: [Tensorflow] Fix conv2d_transpose for NHWC layout
jikechao opened a new pull request, #14546:
URL: https://github.com/apache/tvm/pull/14546
If "data_format" == "NHWC", the kernel_layout should be "HWOI" rather than "HWIO".
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vvchernov commented on pull request #14546: [Tensorflow] Fix conv2d_transpose for NHWC layout
Posted by "vvchernov (via GitHub)" <gi...@apache.org>.
vvchernov commented on PR #14546:
URL: https://github.com/apache/tvm/pull/14546#issuecomment-1501406313
Hello @jikechao! Good fix. It looks like misprinting or thoughtless ctr+c, ctr+v from conv condition. I agree with @yongwww CI test is needed to check 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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tvm-bot commented on pull request #14546: [Tensorflow] Fix conv2d_transpose for NHWC layout
Posted by "tvm-bot (via GitHub)" <gi...@apache.org>.
tvm-bot commented on PR #14546:
URL: https://github.com/apache/tvm/pull/14546#issuecomment-1501164069
<!---bot-comment-->
Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @-ing them in a comment.
<!--bot-comment-ccs-start-->
* No users to tag found in teams: `tensorflow` <sub>See [#10317](https://github.com/apache/tvm/issues/10317) for details</sub><!--bot-comment-ccs-end-->
<sub>Generated by [tvm-bot](https://github.com/apache/tvm/blob/main/ci/README.md#github-actions)</sub>
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] jikechao commented on pull request #14546: [Tensorflow] Fix conv2d_transpose for NHWC layout
Posted by "jikechao (via GitHub)" <gi...@apache.org>.
jikechao commented on PR #14546:
URL: https://github.com/apache/tvm/pull/14546#issuecomment-1507812011
@echuraev CI fixed, could you merge 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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] echuraev commented on pull request #14546: [Tensorflow] Fix conv2d_transpose for NHWC layout
Posted by "echuraev (via GitHub)" <gi...@apache.org>.
echuraev commented on PR #14546:
URL: https://github.com/apache/tvm/pull/14546#issuecomment-1502734003
@jikechao Thank you for your PR.
> Specifically, when the op_name=='conv2d_transpose' and layout='NHWC', the layout was reset to 'NCHW' in this line: https://github.com/apache/tvm/blob/main/python/tvm/relay/frontend/tensorflow_ops.py#:~:text=if%20opname%20%3D%3D%20%22conv_transpose%22%20and,%22data_format%22%5D%20%3D%20%22NCHW%22
The link in your previous message is broken and doesn't highlight any line in the code. Also, could you please add a unit test for your changes?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] echuraev merged pull request #14546: [Tensorflow] Fix conv2d_transpose for NHWC layout
Posted by "echuraev (via GitHub)" <gi...@apache.org>.
echuraev merged PR #14546:
URL: https://github.com/apache/tvm/pull/14546
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] jikechao commented on pull request #14546: [Tensorflow] Fix conv2d_transpose for NHWC layout
Posted by "jikechao (via GitHub)" <gi...@apache.org>.
jikechao commented on PR #14546:
URL: https://github.com/apache/tvm/pull/14546#issuecomment-1503228091
@echuraev Indeed, this link can't redirect to the highlight place by clicking it directly. As a workaround, you can open this link by copying this link and past it into a new browser window.
I'll add a unit test sooner!
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] yongwww commented on pull request #14546: [Tensorflow] Fix conv2d_transpose for NHWC layout
Posted by "yongwww (via GitHub)" <gi...@apache.org>.
yongwww commented on PR #14546:
URL: https://github.com/apache/tvm/pull/14546#issuecomment-1501167748
@jikechao it would be good to add test cases for this fix
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vvchernov commented on pull request #14546: [Tensorflow] Fix conv2d_transpose for NHWC layout
Posted by "vvchernov (via GitHub)" <gi...@apache.org>.
vvchernov commented on PR #14546:
URL: https://github.com/apache/tvm/pull/14546#issuecomment-1502746676
Hello @jikechao! Thank you! You are right about condition fix, but it is still needed CI test to check that it works ok now and prevent further issue around 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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] echuraev commented on pull request #14546: [Tensorflow] Fix conv2d_transpose for NHWC layout
Posted by "echuraev (via GitHub)" <gi...@apache.org>.
echuraev commented on PR #14546:
URL: https://github.com/apache/tvm/pull/14546#issuecomment-1508037482
@jikechao Thank you for your work :)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] jikechao commented on pull request #14546: [Tensorflow] Fix conv2d_transpose for NHWC layout
Posted by "jikechao (via GitHub)" <gi...@apache.org>.
jikechao commented on PR #14546:
URL: https://github.com/apache/tvm/pull/14546#issuecomment-1502691944
@yongwww @vvchernov Thanks for your review.
I tried to construct a test case to trigger this bug but failed. By analyzing the source code, I found that the wrong logic statement is in a dead code. Specifically, when the `op_name=='conv2d_transpose' and layout='NHWC'`, the layout was reset to 'NCHW' in this [line](https://github.com/apache/tvm/blob/778749629c420121fbb9e28e9b356871e95554e8/python/tvm/relay/frontend/tensorflow_ops.py#:~:text=if%20opname%20%3D%3D%20%22conv_transpose%22%20and,%22data_format%22%5D%20%3D%20%22NCHW%22). Thus, the condition `if attr["data_format"] == "NHWC"` always be `False`.
I deleted the wrong logic in the dead code. Please help me review it again. Thanks!
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] jikechao commented on pull request #14546: [Tensorflow] Fix conv2d_transpose for NHWC layout
Posted by "jikechao (via GitHub)" <gi...@apache.org>.
jikechao commented on PR #14546:
URL: https://github.com/apache/tvm/pull/14546#issuecomment-1501165611
As we can see in frontend about [conv2d_transpose](https://github.com/apache/tvm/blob/main/python/tvm/relay/frontend/keras.py#:~:text=keras_layer.input_shape-,if%20data_layout%20%3D%3D%20%22NHWC%22%3A,kernel_layout%20%3D%20%22HWOI%22,-else%3A)
, the kernel_layout for conv2d_transpose was set as "HWOI" when the data_layout='NHWC', while the Tensorflow frontend set the kernel_layout='HWIO' mistankenly. I submitted this PR to fix it.
@leandron @vvchernov Please help me review this PR. Thanks!
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org