Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix discrepancy between doc and impl for tf.reverse #13672

Merged
merged 4 commits into from
Dec 27, 2017

Conversation

yongtang
Copy link
Member

This fix tries to address the discrepancy between doc and implementations for tf.reverse. In the documentation, both int32 and int64 could be used for axis.

However, the actual implementation of the tf.reverse does not support int64 and caused missing kernel error:

ubuntu@ubuntu:~$ python
Python 2.7.12 (default, Nov 19 2016, 06:48:10)
[GCC 5.4.0 20160609] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import tensorflow as tf
>>> v = tf.reverse_v2([1, 2, 3], tf.constant([0], tf.int64))
>>> sess = tf.Session()
>>> sess.run(v)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python2.7/dist-packages/tensorflow/python/client/session.py", line 889, in run
    run_metadata_ptr)
  File "/usr/local/lib/python2.7/dist-packages/tensorflow/python/client/session.py", line 1120, in _run
    feed_dict_tensor, options, run_metadata)
  File "/usr/local/lib/python2.7/dist-packages/tensorflow/python/client/session.py", line 1317, in _do_run
    options, run_metadata)
  File "/usr/local/lib/python2.7/dist-packages/tensorflow/python/client/session.py", line 1336, in _do_call
    raise type(e)(node_def, op, message)
tensorflow.python.framework.errors_impl.InvalidArgumentError: No OpKernel was registered to support Op 'ReverseV2' with these attrs.  Registered devices: [CPU], Registered kernels:
  device='CPU'; T in [DT_STRING]; Tidx in [DT_INT32]
  device='CPU'; T in [DT_BOOL]; Tidx in [DT_INT32]
  device='CPU'; T in [DT_COMPLEX128]; Tidx in [DT_INT32]
  device='CPU'; T in [DT_COMPLEX64]; Tidx in [DT_INT32]
  device='CPU'; T in [DT_DOUBLE]; Tidx in [DT_INT32]
  device='CPU'; T in [DT_FLOAT]; Tidx in [DT_INT32]
  device='CPU'; T in [DT_HALF]; Tidx in [DT_INT32]
  device='CPU'; T in [DT_INT8]; Tidx in [DT_INT32]
  device='CPU'; T in [DT_UINT8]; Tidx in [DT_INT32]
  device='CPU'; T in [DT_INT16]; Tidx in [DT_INT32]
  device='CPU'; T in [DT_UINT16]; Tidx in [DT_INT32]
  device='CPU'; T in [DT_INT32]; Tidx in [DT_INT32]
  device='CPU'; T in [DT_INT64]; Tidx in [DT_INT32]

         [[Node: ReverseV2 = ReverseV2[T=DT_INT32, Tidx=DT_INT64](ReverseV2/tensor, Const)]]

This fix addresses this issue and added the int64 support for axis in tf.reverse.

Signed-off-by: Yong Tang yong.tang.github@outlook.com

This fix tries to address the discrepancy between doc and
implementations for `tf.reverse`. In the documentation,
both int32 and int64 could be used for axis.

However, the actual implementation of the `tf.reverse` does
not support int64 and caused missing kernel error:
```python
ubuntu@ubuntu:~$ python
Python 2.7.12 (default, Nov 19 2016, 06:48:10)
[GCC 5.4.0 20160609] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import tensorflow as tf
>>> v = tf.reverse_v2([1, 2, 3], tf.constant([0], tf.int64))
>>> sess = tf.Session()
>>> sess.run(v)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python2.7/dist-packages/tensorflow/python/client/session.py", line 889, in run
    run_metadata_ptr)
  File "/usr/local/lib/python2.7/dist-packages/tensorflow/python/client/session.py", line 1120, in _run
    feed_dict_tensor, options, run_metadata)
  File "/usr/local/lib/python2.7/dist-packages/tensorflow/python/client/session.py", line 1317, in _do_run
    options, run_metadata)
  File "/usr/local/lib/python2.7/dist-packages/tensorflow/python/client/session.py", line 1336, in _do_call
    raise type(e)(node_def, op, message)
tensorflow.python.framework.errors_impl.InvalidArgumentError: No OpKernel was registered to support Op 'ReverseV2' with these attrs.  Registered devices: [CPU], Registered kernels:
  device='CPU'; T in [DT_STRING]; Tidx in [DT_INT32]
  device='CPU'; T in [DT_BOOL]; Tidx in [DT_INT32]
  device='CPU'; T in [DT_COMPLEX128]; Tidx in [DT_INT32]
  device='CPU'; T in [DT_COMPLEX64]; Tidx in [DT_INT32]
  device='CPU'; T in [DT_DOUBLE]; Tidx in [DT_INT32]
  device='CPU'; T in [DT_FLOAT]; Tidx in [DT_INT32]
  device='CPU'; T in [DT_HALF]; Tidx in [DT_INT32]
  device='CPU'; T in [DT_INT8]; Tidx in [DT_INT32]
  device='CPU'; T in [DT_UINT8]; Tidx in [DT_INT32]
  device='CPU'; T in [DT_INT16]; Tidx in [DT_INT32]
  device='CPU'; T in [DT_UINT16]; Tidx in [DT_INT32]
  device='CPU'; T in [DT_INT32]; Tidx in [DT_INT32]
  device='CPU'; T in [DT_INT64]; Tidx in [DT_INT32]

         [[Node: ReverseV2 = ReverseV2[T=DT_INT32, Tidx=DT_INT64](ReverseV2/tensor, Const)]]

```
This fix addresses this issue and added the int64 support
for axis in `tf.reverse`.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
@tensorflow-jenkins
Copy link
Collaborator

Can one of the admins verify this patch?

@caisq
Copy link
Contributor

caisq commented Oct 13, 2017

@tensorflow-jenkins test this please

@martinwicke martinwicke added awaiting review Pull request awaiting review stat:awaiting tensorflower Status - Awaiting response from tensorflower labels Nov 8, 2017
@martinwicke
Copy link
Member

ping @aselle

@martinwicke martinwicke added stat:awaiting tensorflower Status - Awaiting response from tensorflower and removed stat:awaiting tensorflower Status - Awaiting response from tensorflower labels Dec 12, 2017
Copy link
Contributor

@aselle aselle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks for the submission

@aselle aselle removed the awaiting review Pull request awaiting review label Dec 16, 2017
@drpngx drpngx added awaiting testing (then merge) kokoro:force-run Tests on submitted change and removed stat:awaiting tensorflower Status - Awaiting response from tensorflower labels Dec 27, 2017
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Dec 27, 2017
@drpngx drpngx merged commit 4897107 into tensorflow:master Dec 27, 2017
@yongtang yongtang deleted the 10082017-reverse-int64 branch December 27, 2017 06:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants