Skip to content
This repository has been archived by the owner on Aug 15, 2019. It is now read-only.

CPU/GPU consistency for pow #1136

Merged
merged 2 commits into from
Jul 3, 2018
Merged

CPU/GPU consistency for pow #1136

merged 2 commits into from
Jul 3, 2018

Conversation

jgartman
Copy link
Contributor

@jgartman jgartman commented Jul 1, 2018

Description

This PR makes the output of the pow function more consistent on WebGL vs. the CPU tensorflow/tfjs#350. This change will cause the function to output nan when a negative base is taken to a fractional exponent.


For repository owners only:

Please remember to apply all applicable tags to your pull request.
Tags: FEATURE, BREAKING, BUG, PERF, DEV, DOC, SECURITY

For more info see: https://github.com/tensorflow/tfjs/blob/master/DEVELOPMENT.md


This change is Reviewable

@nsthorat
Copy link
Contributor

nsthorat commented Jul 3, 2018

Thanks for the PR! Just curious, do you know if this is how TensorFlow python behaves?


Review status: 0 of 1 LGTMs obtained


Comments from Reviewable

@jgartman
Copy link
Contributor Author

jgartman commented Jul 3, 2018

TensorFlow python does return nan for a negative base taken to a fractional exponent however there are still some other differences with the tfjs implementation. TF is stricter about both the arguments having the same type but based on this discussion #1129 it doesn't seem like that's a concern. TF will also throw an error when an int32 base and exponent are used and the exponent is negative but again I thought that was stricter than necessary here. Just to give a little background, the primary motivation for this PR was this discussion tensorflow/tfjs#346 about how to fix the gradient for pow.

Note that the testing I did with TF python was CPU only.


Review status: 0 of 1 LGTMs obtained


Comments from Reviewable

@nsthorat
Copy link
Contributor

nsthorat commented Jul 3, 2018

:lgtm_strong:


Review status: :shipit: complete! 1 of 1 LGTMs obtained


Comments from Reviewable

@nsthorat nsthorat merged commit 74a734a into tensorflow:master Jul 3, 2018
@nsthorat
Copy link
Contributor

nsthorat commented Jul 3, 2018

Thanks @jgartman!! P.S. you should still shoot me an email, I can't find your email anywhere :)

@jgartman
Copy link
Contributor Author

jgartman commented Jul 4, 2018

Will do @nsthorat and if you ever need my email its my firstname.lastname at gmail.

@jgartman jgartman deleted the powFix branch August 25, 2018 17:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants