Typing the gelu activation function. #928
Conversation
gabrieldemarmiesse
requested review from
facaiy,
seanpmorgan and
a team
as code owners
| from tensorflow_addons.utils.resource_loader import LazySO | ||
|
|
||
| _activation_so = LazySO("custom_ops/activations/_activation_ops.so") | ||
|
|
||
|
|
||
| @tf.keras.utils.register_keras_serializable(package='Addons') | ||
| def gelu(x, approximate=True): | ||
| @tf.autograph.experimental.do_not_convert |
There was a problem hiding this comment.
Hmmmm, this has the potential to be a larger issue.
To start I'm not sure why this function is being converted with autograph in the first place? Simple op call with no @tf.function conversion.
Given that it is for some reason converted, do we see typechecked as forever incompatible with autograph? If so I lean toward not type checking functions that are converting as opposed to the experimental do_not_convert since I'm not sure the long term implications of disabling autograph conversions as the core code base moves forward.
There was a problem hiding this comment.
Given that it is for some reason converted, do we see typechecked as forever incompatible with autograph?
No idea, but it can be easily changed. What's a lot of work to do is the typing. Let's drop the check for now on functions creating the graph. It should be fine for class constructors though since they're not used with autograph.
| @@ -14,13 +14,16 @@ | |||
| # ============================================================================== | |||
|
|
|||
| import tensorflow as tf | |||
| from typeguard import typechecked | |||
There was a problem hiding this comment.
It reminds me that I need to re-enable this check with flake8
I need to disable autograph to use the decorator. It's fine to use normal tracing.