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

Typing the gelu activation function. #928

Merged
merged 26 commits into from
Jan 28, 2020
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
8c5f509
Added a test to check for type information.
gabrieldemarmiesse Jan 24, 2020
f144705
It works, it just needs an exception list.
gabrieldemarmiesse Jan 24, 2020
6a42b45
Added return type.
gabrieldemarmiesse Jan 24, 2020
038c1a5
Added the exception list.
gabrieldemarmiesse Jan 24, 2020
912de0b
Using the signature is cleaner.
gabrieldemarmiesse Jan 24, 2020
0092ca9
Typechecked is not compatible with autograph.
gabrieldemarmiesse Jan 24, 2020
4a45358
Added the github actions step.
gabrieldemarmiesse Jan 24, 2020
0fc8cd6
Added the python version.
gabrieldemarmiesse Jan 24, 2020
e6ce2a1
Typo
gabrieldemarmiesse Jan 24, 2020
e82a203
Typo.
gabrieldemarmiesse Jan 24, 2020
406090f
Install tf addons.
gabrieldemarmiesse Jan 24, 2020
dced016
Typo.
gabrieldemarmiesse Jan 24, 2020
dfc46bc
Arguments order.
gabrieldemarmiesse Jan 24, 2020
8958ebc
Added option to install tf-addons without building.
gabrieldemarmiesse Jan 24, 2020
aeba883
typeguard.
gabrieldemarmiesse Jan 24, 2020
54dff52
Everything should work now.
gabrieldemarmiesse Jan 24, 2020
6000d1f
Forgot to remove variable.
gabrieldemarmiesse Jan 24, 2020
d1009fa
More extensive about types.
gabrieldemarmiesse Jan 24, 2020
aebc685
Unused function.
gabrieldemarmiesse Jan 24, 2020
0ef5d0a
Typing gelu.
gabrieldemarmiesse Jan 24, 2020
0520433
Merge branch 'master' into gelu_types
gabrieldemarmiesse Jan 27, 2020
9117054
Removed the do_not_convert.
gabrieldemarmiesse Jan 27, 2020
960fc47
Used black.
gabrieldemarmiesse Jan 27, 2020
ba688ca
Removed unused import.
gabrieldemarmiesse Jan 27, 2020
71a161b
Merge branch 'master' into gelu_types
seanpmorgan Jan 28, 2020
7a4822f
Update types.py
seanpmorgan Jan 28, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion tensorflow_addons/activations/gelu.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,18 @@
# ==============================================================================

import tensorflow as tf
from typeguard import typechecked
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unused import?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It reminds me that I need to re-enable this check with flake8


from tensorflow_addons.utils import types
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@typechecked
def gelu(x: types.TensorLike, approximate: bool = True) -> tf.Tensor:
"""Gaussian Error Linear Unit.

Computes gaussian error linear:
Expand Down
22 changes: 21 additions & 1 deletion tensorflow_addons/utils/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,28 @@
# ==============================================================================
"""Types for typing functions signatures."""

from typing import Union, Callable
from typing import Union, Callable, List

import numpy as np
import tensorflow as tf


Number = Union[float,
int,
np.float16,
np.float32,
np.float64,
np.int8,
np.int16,
np.int32,
np.int64,
np.uint8,
np.uint16,
np.uint32,
np.uint64]

Initializer = Union[None, dict, str, Callable]
Regularizer = Union[None, dict, str, Callable]
Constraint = Union[None, dict, str, Callable]

TensorLike = Union[List[Union[Number, list]], Number, np.ndarray, tf.Tensor]