LayernormSimpleRNN moved to addons #841

ulf1 · 2020-01-08T18:02:46Z

Code moved from this PR tensorflow/tensorflow#35469

qlzh727

Please update the code with the assumption of layernorm=True, and I will take a look again.

tensorflow_addons/rnn/BUILD

tensorflow_addons/rnn/layernorm_simplernn.py

…normal self.bias term after scaling with layernorm for centering. docstring with explanatory formulas added to cell's call method

ulf1 · 2020-01-09T22:31:40Z

The unit tests fail with this message

//tensorflow_addons/rnn:cell_test                                        FAILED in 2.8s
  /root/.cache/bazel/_bazel_root/e051f2f195071208ea7f081f88730f4f/execroot/__main__/bazel-out/k8-opt/testlogs/tensorflow_addons/rnn/cell_test/test.log

Executed 61 out of 61 tests: 60 tests pass and 1 fails locally.
There were tests whose specified size is too big. Use the --test_verbose_timeout_warnings command line option to see which ones these are.
+ exit 3
make: *** [unit-test] Error 3
makefile:29: recipe for target 'unit-test' failed

and

//tensorflow_addons/rnn:cell_test                                        FAILED in 3.7s
  /root/.cache/bazel/_bazel_root/e051f2f195071208ea7f081f88730f4f/execroot/__main__/bazel-out/k8-opt/testlogs/tensorflow_addons/rnn/cell_test/test.log

Executed 61 out of 61 tests: 60 tests pass and 1 fails locally.
There were tests whose specified size is too big. Use the --test_verbose_timeout_warnings command line option to see which ones these are.
INFO: Build completed, 1 test FAILED, 1522 total actions
+ exit 3
make: *** [gpu-unit-test] Error 3
makefile:32: recipe for target 'gpu-unit-test' failed

Is this error about this in BUILD?

py_test(
    name = "cell_test",
    size = "small",
    srcs = ["cell_test.py"],
    deps = [
        ":rnn",
    ],
)

ulf1 · 2020-01-11T07:46:40Z

@seanpmorgan can you run kokoro:force-run again, please?

ulf1 · 2020-01-11T10:25:37Z

Python2 kokoro tests (GPU, CPU) are still failing with

There were tests whose specified size is too big. Use the --test_verbose_timeout_warnings command line option to see which ones these are.

In BUILD the timeout is currently at size = "large", what works for the Python3 tests in kokoro. Should I just delete all unit tests? Are the Python2 tests still relevant?

tensorflow_addons/rnn/cell.py

qlzh727 · 2020-01-13T06:13:10Z

tensorflow_addons/rnn/cell.py

+      bias_regularizer: Regularizer function applied to the bias vector
+         (`use_bias=True`) or for the beta vector of the layer normalization
+         layer (`use_layernorm=True`). Default: `None`.
+      gamma_regularizer: Regularizer function applied to the gamma vector


Not sure where the gamma regularizer and constrant are useful or not. I chose to not expose those two in the layernorm lstm, so that we don't have a big number of params that confuse users.

It's for training of the scaling parameter in LayerNormalization.

https://github.com/tensorflow/tensorflow/blob/master/tensorflow/python/keras/layers/normalization.py
line 1033 - 1040

Why to set self.gamma_constraint?

When your scaling parameters shrinks towards zero (e.g., other weights increase, too unscaled inputs), or becomes too large

You know from experience with the problem, that the scaling parameter always was in a certain range

Why to set self.gamma_regularizer?

When your scaling parameter explodes (e.g., input values and hidden state values are too small to generate new hidden values that are big enough)

The gamma constraint and regularizer are hyperparameters a user could try when model training terminates with weird scaling parameters.

tensorflow_addons/rnn/cell.py

qlzh727

Mostly good. Thanks for the change.

tensorflow_addons/rnn/cell.py

qlzh727 · 2020-01-21T22:45:25Z

tensorflow_addons/rnn/cell.py

+            epsilon=layernorm_epsilon,
+            center=False,
+            scale=True,
+            beta_initializer=None,


None seems to be a weird default. Should it be zeros?

if center=False then beta is is not used at all

Should we remove all three inputs that are like beta_initializer=None?

ok, I think let layernorm to use its default value should be fine.

tensorflow_addons/rnn/cell.py

tensorflow_addons/rnn/cell_test.py

tensorflow_addons/rnn/cell.py

qlzh727

Sorry for the late reply, and thanks for the contribution.

LayernormSimpleRNN moved to addons

8226633

ulf1 requested a review from qlzh727 as a code owner January 8, 2020 18:02

googlebot added the cla: yes label Jan 8, 2020

ulf1 mentioned this pull request Jan 8, 2020

LayernormSimpleRNN added tensorflow/tensorflow#35469

Closed

seanpmorgan added the kokoro:force-run label Jan 8, 2020

kokoro-team removed the kokoro:force-run label Jan 8, 2020

code-format run

1fa64cf

qlzh727 requested changes Jan 9, 2020

View reviewed changes

ulf1 added 5 commits January 9, 2020 08:18

use super instead of calling the parent class

5a49582

deactivate layernorm's bias term (beta) for centering, and apply the …

1d32041

…normal self.bias term after scaling with layernorm for centering. docstring with explanatory formulas added to cell's call method

use_layernorm=True set as default

3f24525

import alligned with cell.py, examples in docstring corrected

764419d

import aligned with cell_test.py

8da0cc0

seanpmorgan added the kokoro:force-run label Jan 9, 2020

kokoro-team removed the kokoro:force-run label Jan 9, 2020

code for LayernormSimpleRNN moved into cell.py and cell_test.py

6fff49f

seanpmorgan added the kokoro:force-run label Jan 9, 2020

kokoro-team removed the kokoro:force-run label Jan 9, 2020

pylint errors corrected

a1b86b5

bazel's timeout increased from small to large for cell_test.py

8179a2e

Squadrick added the kokoro:force-run label Jan 11, 2020

kokoro-team removed the kokoro:force-run label Jan 11, 2020

WindQAQ reviewed Jan 11, 2020

View reviewed changes

tensorflow_addons/rnn/cell.py Outdated Show resolved Hide resolved

ulf1 added 2 commits January 12, 2020 08:06

test with training deactivated

140d955

non-ascii char replaced

c2d9971

seanpmorgan added the kokoro:force-run label Jan 12, 2020

kokoro-team removed the kokoro:force-run label Jan 12, 2020

qlzh727 requested changes Jan 13, 2020

View reviewed changes

ulf1 added 6 commits January 13, 2020 08:49

dict syntax for python2 changed

9369772

Renamed to LayerNorm...

348567c

direct parent class call replaced with super

179221b

error due to import change corrected

5cdedc2

uncomment line

a5f71bb

unit test added

7a3c5f1

Squadrick added the kokoro:force-run label Jan 13, 2020

kokoro-team removed the kokoro:force-run label Jan 13, 2020

ulf1 added 5 commits January 13, 2020 18:24

Name change in unit test file

022eef4

Still the class name change

1cce210

deleted dtype and trainable args for parent class

0ae1d43

remove self for super parent class calls

55e1847

compare arrays with assertAllEqual

0809732

seanpmorgan added the kokoro:force-run label Jan 14, 2020

kokoro-team removed the kokoro:force-run label Jan 14, 2020

ulf1 added 2 commits January 15, 2020 09:38

use_layernorm removed

088d9c6

dict removed from return statement

944ae38

ulf1 requested a review from qlzh727 January 16, 2020 14:41

qlzh727 requested changes Jan 21, 2020

View reviewed changes

ulf1 added 3 commits January 22, 2020 14:00

LayerNormSimpleRNN removed, use kwargs, comments removed

61be862

forward **kwargs to other layers

bb1655f

a more pythonic dict loop

dc22b6e

seanpmorgan added the kokoro:force-run label Jan 23, 2020

kokoro-team removed the kokoro:force-run label Jan 23, 2020

ulf1 requested a review from qlzh727 February 5, 2020 18:25

qlzh727 approved these changes Feb 5, 2020

View reviewed changes

qlzh727 merged commit cdb43ff into tensorflow:master Feb 5, 2020

ulf1 mentioned this pull request May 23, 2022

Implement LayernormGRU kmedian/keras-layernorm-rnn#2

Closed

LayernormSimpleRNN moved to addons #841

LayernormSimpleRNN moved to addons #841

Uh oh!

Conversation

ulf1 commented Jan 8, 2020

Uh oh!

qlzh727 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ulf1 commented Jan 9, 2020

Uh oh!

ulf1 commented Jan 11, 2020

Uh oh!

ulf1 commented Jan 11, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

qlzh727 Jan 13, 2020

Choose a reason for hiding this comment

Uh oh!

ulf1 Jan 13, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

qlzh727 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

qlzh727 Jan 21, 2020

Choose a reason for hiding this comment

Uh oh!

ulf1 Jan 22, 2020

Choose a reason for hiding this comment

Uh oh!

ulf1 Jan 23, 2020

Choose a reason for hiding this comment

Uh oh!

qlzh727 Feb 5, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

qlzh727 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants