Implementation of Normalizations #14

Smokrow · 2019-01-15T00:42:31Z

First draft of a file for Normalizations implementing #6 .
I have Implemented GroupNorm, InstanceNorm and LayerNorm and a first testcase for GroupNorm( I will add a few more and will also implement some for Layer/Instance norm).

Could you give me a quick feedback on the implementation or if something is missing?

Thx in advance

…rm and LayerNorm and first testcase

seanpmorgan

@Smokrow Tyvm for the PR. In general looks good, few changes requested.

Regarding test cases, as you mentioned there are some more things we'll want to check, including some of the tests from: https://github.com/tensorflow/tensorflow/blob/v1.12.0/tensorflow/contrib/layers/python/layers/normalization_test.py

tensorflow_addons/layers/python/normalizations.py

guillaumekln · 2019-01-19T12:54:53Z

Looks like a layer normalization layer has been added to tf.keras.layers.experimental.LayerNormalization, so it should probably be removed from this PR.

seanpmorgan · 2019-02-04T22:18:42Z

@karmel as discussed in our monthly meeting... LayerNorm was added to keras experimental which kind of stomps on the implementation we were going to add. Could you please update us on the keras experimental roadmap?

Additionally, addons was going to add GroupNorm as a generalized normalization case (See below image). Any thoughts on how we should proceed?

https://arxiv.org/pdf/1803.08494.pdf

karmel · 2019-02-05T19:54:34Z

Notes from discussions with the Keras team--

In general, this makes more sense in Addons than in experimental. It was added to experimental as a stopgap for some migration work, but we all agree this is a better fit for Addons than experimental, as the scope of use-cases is fairly narrow.

In the future, we will make sure to de-dupe with Addons before adding to experimental, and to prefer Addons unless we are actually adding a Layer/etc. that will end up in core, but has some API kinks to work out.

So, for this PR, if you could go ahead and dedupe with the experimental implementation and push here, we will remove the experimental implementation and use this one instead.

Thanks, all, for working on this.

seanpmorgan · 2019-02-07T03:14:22Z

@Smokrow Would you be able to modify the test coverage to be as extensive as the reference in tf.keras experimental. Then we can merge and request the removal of LayerNormalization from core

Smokrow · 2019-02-08T20:41:37Z

On it. Thx for clearifying 👍

facaiy · 2019-02-10T00:11:42Z

Hi, could you resolve conflicts?

Smokrow · 2019-02-10T07:06:52Z

This is still not finished.

Smokrow · 2019-02-10T22:35:31Z

@seanpmorgan I am having some trouble with running the tests.

I am getting the following while running the test_groupnorm_flat and test_groupnorm_conv

Traceback (most recent call last):
  File "/usr/local/lib/python2.7/dist-packages/absl/third_party/unittest3_backport/case.py", line 37, in testPartExecutor
    yield
  File "/usr/local/lib/python2.7/dist-packages/absl/third_party/unittest3_backport/case.py", line 162, in run
    testMethod()
  File "normalizations_test.py", line 82, in test_groupnorm_conv
    model.fit(np.random.random((10,20, 20, 3)))
  File "/usr/local/lib/python2.7/dist-packages/tensorflow/python/keras/engine/training.py", line 989, in fit
    steps_name='steps_per_epoch')
  File "/usr/local/lib/python2.7/dist-packages/tensorflow/python/keras/engine/training_arrays.py", line 330, in model_iteration
    batch_outs = f(ins_batch)
  File "/usr/local/lib/python2.7/dist-packages/tensorflow/python/keras/backend.py", line 3123, in __call__
    outputs = self._graph_fn(*converted_inputs)
  File "/usr/local/lib/python2.7/dist-packages/tensorflow/python/eager/function.py", line 452, in __call__
    return self._call_flat(args)
  File "/usr/local/lib/python2.7/dist-packages/tensorflow/python/eager/function.py", line 521, in _call_flat
    outputs = self._inference_function.call(ctx, args)
  File "/usr/local/lib/python2.7/dist-packages/tensorflow/python/eager/function.py", line 291, in call
    (len(args), len(list(self.signature.input_arg))))
ValueError: Arguments and signature arguments do not match: 15 17

Looks like it is deep down in the Eager execution but I am quite confused whats going on down there 😄
Greets

facaiy · 2019-02-12T12:31:37Z

I check out the feature branch, and fail to run bazel tests.

@Smokrow Hi, Moritz. Could you fix bazel configuration and make sure we can reproduce your problem? It would be greatly helpful for debug :-)

Smokrow · 2019-02-17T22:57:14Z

@seanpmorgan ready for review

facaiy

I leave some comments, and most of them are trivial code style problem. Thanks for the PR :-)

tensorflow_addons/layers/python/normalizations.py

facaiy · 2019-02-21T22:41:16Z

tensorflow_addons/layers/python/normalizations.py

+        group_axes.insert(1, self.groups)
+
+        # reshape inputs to new group shape
+        group_shape = [group_axes[0], self.groups] + group_axes[2:]


Can it handle both channel-first and channel-last format?

yes. at this point the ordering would be [batch, group, channels, steps]

@facaiy since you are setting up an axis to work on I am not quite sure if you can handle "channel first" and "channel last". When somebody sets his axis exactly on the channel axis I think he should be allowed to do that.

tensorflow_addons/layers/python/normalizations.py

tensorflow_addons/layers/python/normalizations_test.py

tensorflow_addons/layers/BUILD

tensorflow_addons/layers/python/normalizations.py

ppwwyyxx · 2019-03-02T20:29:19Z

tensorflow_addons/layers/python/normalizations.py

+        groups: Integer, the number of groups for Group Normalization.
+            Can be in the range [1, N] where N is the input dimension.
+            The input dimension must be divisible by the number of groups.
+        axis: Integer, the axis that should be normalized


In the case of 4D input tensor, the axis that's normalized is either C/G, H, W, or Batch, G, depend on your definition of "be normalized". But it is in no way "C".

BatchNorm layer takes axis=C, therefore if you want to make an analogy here, it would be axis=[Batch, G]. This analogy is a bit ugly, so I think a better way is to still define axis to be the channel dimension, and write a clearer documentation about what this layer actually does for 2D and 4D tensors, respectively.

Same comment applies to other norms.

I think it is better to remove this explanation completly since it does not really belong into the code docs. Currently there are colab notebooks planned for better documentation and explanation. I am currently writing one for layer/group/instance normalization layers. If you want I can reference you in the PR when it is finished.

removed wrong documentation

Removed explanation of layers. Will be added to colab

Smokrow · 2019-03-07T20:01:19Z

@seanpmorgan @facaiy ready for review 👍

facaiy · 2019-03-09T02:02:15Z

@Smokrow Thanks for your work, Moritz!

@ppwwyyxx Yuxin, would you mind taking a look again? Thank you very much for your review :-)

ppwwyyxx · 2019-03-09T02:12:12Z

I don't have other comments. The description for the "axis" argument is still not very accurate but it seems there are other plans to address it.

* Remove tf.logging as part of TF2 * Add normaliztion layers to init * Update READMEs

googlebot · 2019-03-09T19:55:45Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

ℹ️ Googlers: Go here for more info.

seanpmorgan · 2019-03-09T19:58:38Z

Thanks so much for the contribution Moritz. Made some minor formatting changes & integrations with the project if you want to check them out. Looking forward to the example/demo notebook. Thanks for the review @ppwwyyxx we'll be sure to address the axis ambiguity with the demos and if that is not sufficient we can iterate on the parameters as we're still early in release cycles.

* Implemented GroupNorm,InstanceNorm and LayerNorm

edited buildfile for normalizations. Implemented GroupNorm,InstanceNo…

c30b75a

…rm and LayerNorm and first testcase

seanpmorgan self-requested a review January 16, 2019 01:26

seanpmorgan requested changes Jan 16, 2019

View reviewed changes

tensorflow_addons/layers/python/normalizations.py Outdated Show resolved Hide resolved

tensorflow_addons/layers/python/normalizations.py Outdated Show resolved Hide resolved

tensorflow_addons/layers/python/normalizations.py Outdated Show resolved Hide resolved

lgeiger reviewed Jan 16, 2019

View reviewed changes

tensorflow_addons/layers/python/normalizations.py Outdated Show resolved Hide resolved

lgeiger mentioned this pull request Jan 16, 2019

Rename WeightNorm -> WeightNormalization #18

Merged

Resolved Comments

0e7674b

seanpmorgan added the layers label Feb 8, 2019

found bug in normalizations init

65a5495

Smokrow added 4 commits February 10, 2019 20:54

minor changes

892110c

Merge remote-tracking branch 'upstream/master' into dev/tests

57c60a7

added function for easy testing.

d5beb61

clean up

7a361ce

Smokrow added 2 commits February 13, 2019 22:09

found bug in BUILD File

2c174fc

fixed signature bug and added tests

7c32461

Smokrow added 3 commits February 18, 2019 00:17

Merge remote-tracking branch 'upstream/master' into dev/layer_norm

22f3073

Update maxout.py

0b04162

small change to variable name

095d91e

seanpmorgan self-assigned this Feb 19, 2019

facaiy requested changes Feb 21, 2019

View reviewed changes

facaiy requested a review from seanpmorgan February 23, 2019 22:06

googlebot added the cla: yes label Mar 1, 2019

Smokrow added 2 commits March 1, 2019 20:05

fixed BUILD file

540492e

implemented batch_normalization from tf nn

f980aa5

Smokrow mentioned this pull request Mar 2, 2019

Rename tf.nn.batch_normalization tensorflow/tensorflow#26284

Closed

ppwwyyxx reviewed Mar 2, 2019

View reviewed changes

Smokrow added 9 commits March 4, 2019 11:44

added normalization and reshape test

576961d

added axis check

918eeb7

added manual layer test

d2c1afd

added tests to check normalization with numpy

4ebd907

Included some comments from @ppwwyyxx

37244c4

beautified

0669466

Merge branch 'master' into dev/layer_norm

25d5569

Update normalizations.py

b4613ae

removed wrong documentation

Update normalizations.py

fcd1639

Removed explanation of layers. Will be added to colab

seanpmorgan added 2 commits March 9, 2019 13:42

Merge branch 'master' into pr/Smokrow/14

e8ddabe

* Standardize formatting with project

429ded2

* Remove tf.logging as part of TF2 * Add normaliztion layers to init * Update READMEs

googlebot added cla: no and removed cla: yes labels Mar 9, 2019

seanpmorgan approved these changes Mar 9, 2019

View reviewed changes

seanpmorgan merged commit 423fdb6 into tensorflow:master Mar 9, 2019

seanpmorgan mentioned this pull request Mar 22, 2019

Tensorflow 2.0: where is tf.contrib.layers.layer_norm? tensorflow/tensorflow#26854

Closed

Squadrick pushed a commit to Squadrick/addons that referenced this pull request Mar 26, 2019

Implementation of Normalizations (tensorflow#14)

55ef770

* Implemented GroupNorm,InstanceNorm and LayerNorm

Smokrow mentioned this pull request Apr 7, 2019

Colab Notebook for Normalizations #144

Merged

seanpmorgan mentioned this pull request Apr 13, 2019

Graduate LayerNormalization layer to TF core. #172

Closed

breadbread1984 mentioned this pull request Mar 6, 2020

GELU causes segment fault #1228

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implementation of Normalizations #14

Implementation of Normalizations #14

Smokrow commented Jan 15, 2019 •

edited

Loading

seanpmorgan left a comment

guillaumekln commented Jan 19, 2019

seanpmorgan commented Feb 4, 2019

karmel commented Feb 5, 2019

seanpmorgan commented Feb 7, 2019

Smokrow commented Feb 8, 2019

facaiy commented Feb 10, 2019

Smokrow commented Feb 10, 2019

Smokrow commented Feb 10, 2019

facaiy commented Feb 12, 2019 •

edited

Loading

Smokrow commented Feb 17, 2019

facaiy left a comment

facaiy Feb 21, 2019

Smokrow Mar 1, 2019

Smokrow Mar 5, 2019

ppwwyyxx Mar 2, 2019

Smokrow Mar 7, 2019

Smokrow commented Mar 7, 2019

facaiy commented Mar 9, 2019

ppwwyyxx commented Mar 9, 2019

googlebot commented Mar 9, 2019

seanpmorgan commented Mar 9, 2019

Implementation of Normalizations #14

Implementation of Normalizations #14

Conversation

Smokrow commented Jan 15, 2019 • edited Loading

seanpmorgan left a comment

Choose a reason for hiding this comment

guillaumekln commented Jan 19, 2019

seanpmorgan commented Feb 4, 2019

karmel commented Feb 5, 2019

seanpmorgan commented Feb 7, 2019

Smokrow commented Feb 8, 2019

facaiy commented Feb 10, 2019

Smokrow commented Feb 10, 2019

Smokrow commented Feb 10, 2019

facaiy commented Feb 12, 2019 • edited Loading

Smokrow commented Feb 17, 2019

facaiy left a comment

Choose a reason for hiding this comment

facaiy Feb 21, 2019

Choose a reason for hiding this comment

Smokrow Mar 1, 2019

Choose a reason for hiding this comment

Smokrow Mar 5, 2019

Choose a reason for hiding this comment

ppwwyyxx Mar 2, 2019

Choose a reason for hiding this comment

Smokrow Mar 7, 2019

Choose a reason for hiding this comment

Smokrow commented Mar 7, 2019

facaiy commented Mar 9, 2019

ppwwyyxx commented Mar 9, 2019

googlebot commented Mar 9, 2019

seanpmorgan commented Mar 9, 2019

Smokrow commented Jan 15, 2019 •

edited

Loading

facaiy commented Feb 12, 2019 •

edited

Loading