Add stricter type checking for tf.math.real #60540
Conversation
Fix for tf.math.real so that it only accepts tensors with numeric entries as input. This makes it consistent with its documentation at https://www.tensorflow.org/api_docs/python/tf/math/real and raises a TypeError saying input must have numeric entries when called incorrectly.
tensorflow/python/ops/math_ops.py
Outdated
| real_dtype = input.dtype.real_dtype | ||
| return gen_math_ops.real(input, Tout=real_dtype, name=name) | ||
| else: | ||
| elif tf.debugging.is_numeric_tensor(input): |
There was a problem hiding this comment.
We can't use tf. from within here, since it's not imported (and can't be, since that would be a circular dependency).
You can import the function itself from check_ops.py where it actually lives. Or, we could add a property to dtype itself for is_numeric, which is probably the most general solution. Those properties are defined in dtypes.cc, and we already have a kNumberTypes set in types.h that can be leveraged (see DataTypeIsComplex(...) in that file as an example).
There was a problem hiding this comment.
I just implemented these changes. Does it look fine now? Also, is there any specific ordering to functions in dtypes.cc or types.h to keep it consistent with other files? For the time being, I placed the code for is_numeric and IsNumericDataType right above the code for is_complex and IsComplexDataType.
There was a problem hiding this comment.
Thanks, looks good now. No special ordering, as long as its somewhat consistent.
Will run through further testing now.
There was a problem hiding this comment.
Hi. I'm still waiting on Py+CPP Test Suite - Ubuntu CPU, Python 3.9 and Code Check - Changed Files workflows to be run and approved. It seems odd that it's taking this long. Any ideas why?
There was a problem hiding this comment.
Yes, sorry, it broke our API compatibility tests. It shouldn't be a big issue because you only added a read-only attribute, but I'll need to go in and update the API checks, and do some more testing to make sure save/load models doesn't break.
There was a problem hiding this comment.
Okay, thanks. Let me know if I need to change anything else.
google-ml-butler
bot
added
kokoro:force-run
google-ml-butler
bot
added
kokoro:force-run
google-ml-butler
bot
added
kokoro:force-run
5 participants
Fix for tf.math.real so that it only accepts tensors with numeric entries as input. This makes it consistent with its documentation at https://www.tensorflow.org/api_docs/python/tf/math/real and raises a TypeError saying input must have numeric entries when called incorrectly. For more details, see issue #60526.