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.keras.backend.sqrt(tf.constant(-1.0)) is 0 which is misleading and tf.sqrt(tf.constant(-1.0)) is 'nan' which is the way it should be. #34165

Closed
gaddekumar opened this issue Nov 11, 2019 · 5 comments
Assignees
Labels
comp:ops OPs related issues TF 1.15 for issues seen on TF 1.15 type:docs-bug Document issues

Comments

@gaddekumar
Copy link

System information

  • Have I written custom code (as opposed to using a stock example script provided in TensorFlow): No
  • OS Platform and Distribution (e.g., Linux Ubuntu 16.04): Linux Ubuntu 16.04
  • Mobile device (e.g. iPhone 8, Pixel 2, Samsung Galaxy) if the issue happens on mobile device: No
  • TensorFlow installed from (source or binary): Source
  • TensorFlow version (use command below): 1.15
  • Python version: 3.7
  • Bazel version (if compiling from source): No
  • GCC/Compiler version (if compiling from source): No
  • CUDA/cuDNN version: No
  • GPU model and memory: Run in CPU
    Describe the current behavior
    tf.keras.backend.sqrt(tf.constant(-1.0)) returns 0 as clip_by_value is being done in the source code (Which is highly misleading, as can be seen only in the source and not in the function document) whereas tf.sqrt(tf.constant(-1.0)) returns 'nan' which is the expected behavior of any sqrt function. This causes some bugs which are very difficult to track.

Describe the expected behavior
Make sqrt functions return only the expected behavior and remove the clip_by_value.

Code to reproduce the issue
import tensorflow as tf
tf.enable_eager_execution()

tf.keras.backend.sqrt(tf.constant(-1.0)).numpy()
tf.sqrt(tf.constant(-1.0)).numpy()

Other info / logs
Include any logs or source code that would be helpful to diagnose the problem. If including tracebacks, please include the full traceback. Large logs and files should be attached.

@gowthamkpr gowthamkpr self-assigned this Nov 11, 2019
@gowthamkpr gowthamkpr added comp:ops OPs related issues type:support Support issues TF 1.15 for issues seen on TF 1.15 labels Nov 11, 2019
@gowthamkpr
Copy link

As you can see from the source code tf.keras.backend.sqrt clips the input value to zero if its less than 0 and to infinity if its equal to infinity as shown by the function here. This is the reason why the output is zero

But in tf.sqrt the sqrt function is from gen_math_ops.py which has a different behaviour i.e., behaviour similar to numpy and thats the reason why its value is defaulted to NaN

@gowthamkpr gowthamkpr added the stat:awaiting response Status - Awaiting response from author label Nov 11, 2019
@gaddekumar
Copy link
Author

But developers usually expect the tensorflow to return nan when tf.sqrt( negative numbers) is done, as thats the norm unlike returning 0, which defies the name of the function. Or, I would request you to update the documentation just to avoid the confusion as tensorflow is moving towards keras.

@gowthamkpr
Copy link

Sure @saikumarGadde

@tensorflowbutler tensorflowbutler removed the stat:awaiting response Status - Awaiting response from author label Nov 12, 2019
@gowthamkpr gowthamkpr added type:docs-bug Document issues and removed type:support Support issues labels Nov 16, 2019
@gowthamkpr gowthamkpr assigned MarkDaoust and unassigned gowthamkpr Nov 16, 2019
@abhiverma1924
Copy link

@MarkDaoust Is this issue is solved? Thanks.

@ymodak ymodak changed the title tf.keras.backend.sqrt(tf.constant(1.0)) is 0 which is misleading and tf.sqrt(tf.constant(-1.0)) is 'nan' which is the way it should be. tf.keras.backend.sqrt(tf.constant(-1.0)) is 0 which is misleading and tf.sqrt(tf.constant(-1.0)) is 'nan' which is the way it should be. Oct 6, 2020
@MarkDaoust
Copy link
Member

@saikumarGadde: Yes, you have a point but: we need to be very conservative about changing the behavior of existing functions. Also and what is the goal of using tf.keras.backend at all?

At this point tf.keras.backend should be considered an implementation detail. It was a tools for writing keras to be backend-agnostic. TensorFlow is effectively the only backend now.

That's why fchollet removed the documentation for these in:

91e5ad0#diff-e329ed6b8d30dca9a441689005047035

https://www.tensorflow.org/api_docs/python/tf/keras/backend?version=nightly

The functions will still be available in the package but they are undocumented to communicate that they should not be used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:ops OPs related issues TF 1.15 for issues seen on TF 1.15 type:docs-bug Document issues
Projects
None yet
Development

No branches or pull requests

5 participants