Fix crash with tf.transpose when a is complex and conjugate is True #46973
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rohan100jain
requested changes
Mar 27, 2021
yongtang
force-pushed
the
46891-tf.transpose-conjugate
branch
from
b28c1f3
to
dc69ff1
Compare
|
@rohan100jain @cantonios thanks for the review. The PR has been updated. Please take a look. |
cantonios
approved these changes
Mar 31, 2021
google-ml-butler
bot
added
kokoro:force-run
cantonios
requested changes
Apr 5, 2021
There was a problem hiding this comment.
@yongtang I updated the entire test to TF v2, so can you now merge your tests into testComplex64() and testComplex128()?
Something like this would work:
self._testBoth(np.array(np.complex(1, 2)).astype(np.complex128))
self._testBoth(np.complex(1, 2) * np.arange(0, 21).astype(np.complex128))
This PR tries to address the issue raised in 46891 where tf.transpose will crash when a is complex and conjugate is True. The issue comes from: https://github.com/tensorflow/tensorflow/blob/57bbc5e0d4b93483b8ae853352173516f1c08018/tensorflow/core/kernels/transpose_functor.h#L169 However, as ndims < 2 has already been handled properly: https://github.com/tensorflow/tensorflow/blob/57bbc5e0d4b93483b8ae853352173516f1c08018/tensorflow/core/kernels/transpose_functor_cpu.cc#L103-L105 The check could be removed. This PR fixes 46891. Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
yongtang
force-pushed
the
46891-tf.transpose-conjugate
branch
from
dc69ff1
to
20245fc
Compare
|
Thanks @cantonios for the help. The PR has been updated. Please take a look and let me know if there are any issues. |
cantonios
reviewed
Apr 6, 2021
…omplex128() with additional update to sort test cases in increasing rank order Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
yongtang
force-pushed
the
46891-tf.transpose-conjugate
branch
from
20245fc
to
6cbc31d
Compare
|
Thanks @cantonios for the help. The PR has been updated. Please let me know if there are any additional issues. |
cantonios
approved these changes
Apr 6, 2021
google-ml-butler
bot
added
kokoro:force-run
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR tries to address the issue raised in #46891 where
tf.transpose will crash when a is complex and conjugate is True.
The issue comes from:
tensorflow/tensorflow/core/kernels/transpose_functor.h
Line 169 in 57bbc5e
However, as ndims < 2 has already been handled properly:
tensorflow/tensorflow/core/kernels/transpose_functor_cpu.cc
Lines 103 to 105 in 57bbc5e
The check could be removed.
This PR fixes #46891.
Signed-off-by: Yong Tang yong.tang.github@outlook.com