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

tf.divide handles "name" argument differently #6741

Closed
ppwwyyxx opened this issue Jan 9, 2017 · 8 comments
Closed

tf.divide handles "name" argument differently #6741

ppwwyyxx opened this issue Jan 9, 2017 · 8 comments
Assignees

Comments

@ppwwyyxx
Copy link
Contributor

ppwwyyxx commented Jan 9, 2017

If installed from binary pip package, provide:

  1. A link to the pip package you installed:
    https://ci.tensorflow.org/view/Nightly/job/nightly-matrix-linux-gpu/TF_BUILD_IS_OPT=OPT,TF_BUILD_IS_PIP=PIP,TF_BUILD_PYTHON_VERSION=PYTHON2,label=gpu-linux/lastSuccessfulBuild/artifact/pip_test/whl/tensorflow_gpu-0.12.1-cp27-none-linux_x86_64.whl
  2. The output from python -c "import tensorflow; print(tensorflow.__version__)".
    0.12.head

If possible, provide a minimal reproducible example (We usually don't have time to read hundreds of lines of your code)

a = tf.get_variable('a', shape=[10])
b = tf.get_variable('b', shape=[10])
print tf.divide(a, b, name='x').name   # x/truediv:0

Normally one would expect (like almost every other existing function in tf) that with the name='x' option, the output op will be named 'x:0'.
But instead it was named "x/truediv:0".

@yaroslavvb
Copy link
Contributor

yaroslavvb commented Jan 10, 2017

So what's happening here is that divide tries to do version-sensitive Python2 or Python 3 style divide. It calls a/b which drops the name, and then gets redirected to Python2 or Python3 version of division. A work-around is to use tf.div or tf.truediv instead of tf.divide.

@yaroslavvb
Copy link
Contributor

@aselle for comment if this is likely to get fixed .... it seems tf.divide loses the name which makes annoying for graph/timeline visualization

@yaroslavvb yaroslavvb assigned yaroslavvb and aselle and unassigned yaroslavvb Jan 10, 2017
@aselle
Copy link
Contributor

aselle commented Jan 10, 2017

I think I have a fix. The problem is that operator overload div cannot pass name, but I do want to use a class' division dispatch to honor the Python2/Python3 modes. I can do that with a dummy class. I have a fix that I'm testing internally. WIll keep this bug updated.

@drpngx drpngx closed this as completed in ca61311 Jan 11, 2017
@aselle
Copy link
Contributor

aselle commented Jan 11, 2017

@ppwwyyxx, master now has the fix, You can cherry pick it if you need it in an older version, otherwise in the mean-time you can use the workaround. Thanks!

@ppwwyyxx
Copy link
Contributor Author

Great! Thanks a lot!

@ppwwyyxx
Copy link
Contributor Author

ppwwyyxx commented Mar 20, 2017

Similar problem was found in tf.case and tf.train.piecewise_constant.
Are they likely to be fixed?

@yaroslavvb
Copy link
Contributor

I think they are likely to get fixed if they are filed as separate issues (so they get triaged properly). Good to reference this issue so people can refer back to the commit that fixes things

@ppwwyyxx
Copy link
Contributor Author

👍 I was hesitated to open too many similar issues..
If that's fine I'll open one for each

copybara-service bot pushed a commit that referenced this issue Nov 6, 2023
Imported from GitHub PR openxla/xla#6741

Added a helper function to add suffix to CloneWithNewOperand function.
Rework pr openxla/xla#6289 to not change existing API but introduce an override.
Copybara import of the project:

--
764cd152008ff78247706fb37009018b54483ea9 by TJ <tjx@nvidia.com>:

rework pr openxla/xla#6289 to not change
existing API but introduce an override

Merging this change closes #6741

PiperOrigin-RevId: 579757627
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants