GeLU activation as a layer

[AakashKumarNain]

I closed the PR that I opened it yesterday which proposed gelu as an activation function. This one is a much better implementation as the activation function here is represented as a keras layer

[WindQAQ]

@AakashKumarNain Hi, it's a great PR! Maybe my explanation makes you confused... what I mean is to both keep activations.gelu and layers.GeLU haha. But no worries, I think you could re-open the previous PR :-) I would like to take both of them into addons.

Besides, I think we could also to add a test method like this one:
https://github.com/tensorflow/tensorflow/blob/master/tensorflow/python/keras/layers/advanced_activations_test.py#L94-L102
Thanks for the contribution again!

[AakashKumarNain]

@WindQAQ no worries. I have a very simple query. When I run sanity-check and unit-test, is it possible to make it run for a particular set of files only? For example, I just want it to test for GeLU, is there any simple way to do it?
PS: Sorry for the noob question but I had to ask

[WindQAQ]

unit-test can be done by running

bazel test -c opt -k \
--test_timeout 300,450,1200,3600 \
--test_output=all \
//tensorflow_addons/layers:gelu_test

(please type use to auto-complete the rule, say //tensorflow_addons/xxxxxx)

for sanity check, I think you refer to linter check right? If so, pylint --rcfile=tools/ci_build/pylintrc --output-format=parseable [YOUR FILES] would be useful.

[AakashKumarNain]

I don't have bazel installed locally. Can you please the equivalent docker command or something that I need to change in the files present in tools directory? Thanks

[WindQAQ]

In my case, I always attach to the container to run tests. So I will first lanuch a container

docker run --rm -it -v ${PWD}:/addons -w /addons tensorflow/tensorflow:custom-op-ubuntu16 /bin/bash
pip install --upgrade pip # Pip must be upgraded to install manylinux2010 pkg
./configure.sh  # Links project with TensorFlow dependency

And run the commands I paste above.

[mostafaelhoushi]

I was about to create a pull request for Gelu too, but rather as an op implemented as C++:
https://github.com/mostafaelhoushi/tensorflow-addons

Here are the main files I have added:
gelu.cc
gelu_op.cc
gelu.py
gelu_test.py

I will wait for this pull request to be merged so that I can see how to rebase on top of it.

[seanpmorgan]

I was about to create a pull request for Gelu too, but rather as an op implemented as C++:
https://github.com/mostafaelhoushi/tensorflow-addons

Here are the main files I have added:
gelu.cc
gelu_op.cc
gelu.py
gelu_test.py

I will wait for this pull request to be merged so that I can see how to rebase on top of it.

@mostafaelhoushi looks like there is a PR #427 which implements the C++ op. Would you mind doing a review?

@AakashKumarNain Thanks very much! This looks great. Waiting for the activation to be merged first then we can review this and merge

[AakashKumarNain]

@WindQAQ @seanpmorgan since #427 has been merged, should we close #420 and update this layer implementation to use #427 ?

[AakashKumarNain]

@facaiy @WindQAQ I have updated the code. All test cases passed on my end, so I am positive that this commit should be enough to close this PR

[AakashKumarNain]

@seanpmorgan all checks passed! Wooo!! 🎉

[facaiy]

Great, Aakash! @WindQAQ Tzu-Wei, could you take a look again?

[WindQAQ]

LGTM. Probably need only one test case (TestGeLU) with two test methods, say test_gelu and test_layer_as_activation. See https://github.com/tensorflow/tensorflow/blob/master/tensorflow/python/keras/layers/advanced_activations_test.py for more details. Thanks!

[AakashKumarNain]

LGTM. Probably need only one test case (TestGeLU) with two test methods, say test_gelu and test_layer_as_activation. See https://github.com/tensorflow/tensorflow/blob/master/tensorflow/python/keras/layers/advanced_activations_test.py for more details. Thanks!

@WindQAQ @facaiy I wanted to put them in a single Test case but I found out that keras_parameterized.run_all_keras_modes, keras_parameterized.run_with_all_model_types, and test_utils.run_all_in_graph_and_eager_modes don't play along very well. In short, keras_parameterized doesn't allow any other decorator. But as @facaiy pointed out that layer_test has already tested it in both modes, I think it is safe to remove it. What do you say?

[WindQAQ]

@AakashKumarNain Yep, then just follow Facai's suggestions :P

[AakashKumarNain]

@facaiy @WindQAQ I made the necessary changes. I think we are good to go

[WindQAQ]

Thanks

[AakashKumarNain]

Thank you @WindQAQ @facaiy @seanpmorgan @Squadrick