torch::nn::LayerNorm #28032

anjali411 · 2019-10-15T20:46:33Z

No description provided.

… var names, updated get var calls

…dding_branch

…ruct a module which has no default constructor

yf225 · 2019-10-18T03:31:22Z

test/cpp/api/functional.cpp


    ASSERT_TRUE(torch::allclose(output, expected));
  }
-  


Thanks for fixing the extra spaces :D

test/cpp/api/modules.cpp

yf225 · 2019-10-18T03:36:19Z

torch/csrc/api/src/nn/modules/normalization.cpp

+  if (options.elementwise_affine()) {
+    torch::nn::init::ones_(weight);
+    torch::nn::init::zeros_(bias);
+  }


This code block

if (options.elementwise_affine()) { torch::nn::init::ones_(weight); torch::nn::init::zeros_(bias); }

should be at the end the reset() function, because the expectation is that all initialization logic lives in the reset() function :D

yf225 · 2019-10-18T03:40:40Z

@anjali411 This PR has some conflicts with master, and it also contains changes to third_party/fbgemm which should be removed. We can work on those two items tomorrow if you need any help from me :)

…r_norm

yf225 · 2019-10-18T16:05:39Z

@anjali411 The code conflict and submodule issue should be fixed :D

yf225 · 2019-10-18T17:54:32Z

torch/csrc/api/include/torch/nn/functional/normalization.h

+inline Tensor layer_norm(const Tensor& input,
+    const LayerNormOptions& options,
+    const Tensor& weight,
+    const Tensor& bias) {


In the Python version, weight and bias can take None as default value, and in the C++ version I think we should take Tensor() as default value to match the Python version :D

yf225 · 2019-10-18T18:03:24Z

test/cpp/api/functional.cpp

+  const auto input = torch::randn({2, 2});
+  const auto weight = torch::randn({2, 2});
+  const auto bias = torch::randn({2, 2});
+  auto y = F::layer_norm(input, LayerNormOptions({2, 2}).eps(2e-5), weight, bias);


In order to test the "no weights and no bias" case for F::layer_norm, let's probably change this to

Suggested change

auto y = F::layer_norm(input, LayerNormOptions({2, 2}).eps(2e-5), weight, bias);

auto y = F::layer_norm(input, LayerNormOptions({2, 2}).eps(2e-5));

and remove weight and bias for this test.

yf225 · 2019-10-18T18:03:40Z

test/cpp/api/functional.cpp

+  const auto weight = torch::randn({2, 2});
+  const auto bias = torch::randn({2, 2});
+  auto y = F::layer_norm(input, LayerNormOptions({2, 2}).eps(2e-5), weight, bias);
+  auto y_exp = torch::layer_norm(input, {2, 2}, weight, bias, 2e-5);


and we can pass Tensor() as weight and bias here.

…ion and updated the test in functional.cpp

yf225

@anjali411 Thanks a lot for the update! I think we are almost there :D

yf225 · 2019-10-21T18:14:37Z

torch/csrc/api/src/nn/modules/normalization.cpp

+namespace torch {
+namespace nn {
+
+LayerNormImpl::LayerNormImpl(const LayerNormOptions& options_) : options(options_) {


based on clang-tidy:

{ path: 'torch/csrc/api/src/nn/modules/normalization.cpp', start_line: 15, end_line: 15, start_column: 30, end_column: 30, annotation_level: 'failure', message: '[[modernize-pass-by-value]] warning: pass by value and use std::move' },

we should probably add // NOLINT(modernize-pass-by-value) at the end of this line.

yf225 · 2019-10-21T18:15:23Z

torch/csrc/api/src/nn/options/normalization.cpp

+namespace torch {
+namespace nn {
+
+LayerNormOptions::LayerNormOptions(std::vector<int64_t> normalized_shape) : normalized_shape_(normalized_shape) {}


clang-tidy complains:

{ path: 'torch/csrc/api/src/nn/options/normalization.cpp', start_line: 6, end_line: 6, start_column: 36, end_column: 36, annotation_level: 'failure', message: '[[modernize-pass-by-value]] warning: pass by value and use std::move' }

for this one I think we can do normalized_shape_(std::move(normalized_shape)).

facebook-github-bot

@anjali411 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot · 2019-10-22T22:09:29Z

@anjali411 merged this pull request in 7b59174.

Summary: Since now we have merged #28032 (thanks anjali411!) Pull Request resolved: #28484 Differential Revision: D18085844 Pulled By: yf225 fbshipit-source-id: 4be972687addea8f57f48dfe9707837196593062

Summary: Pull Request resolved: pytorch#28032 Differential Revision: D18047371 Pulled By: anjali411 fbshipit-source-id: fb61aea52d6622a67ec1d84950e17e85686461ae

Summary: Since now we have merged pytorch#28032 (thanks anjali411!) Pull Request resolved: pytorch#28484 Differential Revision: D18085844 Pulled By: yf225 fbshipit-source-id: 4be972687addea8f57f48dfe9707837196593062

anjali411 and others added 30 commits September 16, 2019 17:35

added data members, updated constructor and forward method

6a281bc

updated embedding functions

04ede2b

updated pretty print

821e383

added tests, updated variable name to make it similar to py embedding…

2366d55

… var names, updated get var calls

changed constructor and a bunch of other things based on comments

9885566

made changes based on comments and added EmbeddingBagImpl

fa7387c

Merge remote-tracking branch 'origin/master' into HEAD

e6a0858

added Embedding::pretrained(...) and EmbeddingBag::pretrained(...)

694a851

updated methods based on comments

ba9bdbe

made options optional for pre_trained

1efa921

changed tests

c56d8a0

debugging

0018b70

fixed bugs

6a5929f

Merge branch 'master' of https://github.com/pytorch/pytorch

5f281c9

fixed bugs, need to fix from_pretrained tests

9952883

remove unnecessary headers

25470ed

fixed from_pretrained bugs

4580857

minor changes

5ce4303

fixed from_pretrained test bugs

b9951e7

Merge remote-tracking branch 'origin/master' into HEAD

1b0de80

changed a few variables being passed to fns to be const

7c806b8

Merge branch 'master' of https://github.com/anjali411/pytorch

85cef78

minor changes

4d3b0c4

formatting

ea75ac8

updated modulelist and sequential embedding prettyprint test

947a74b

NOLINT bypass

276e04f

Merge remote-tracking branch 'origin/master' into HEAD

4408c84

Merge branch 'master' of https://github.com/pytorch/pytorch into embe…

80073da

…dding_branch

Merge branch 'master' into master

2cdd5dd

try to fix Windows build error C2338: You are trying to default const…

18f6a6e

…ruct a module which has no default constructor

anjali411 added 3 commits October 17, 2019 11:09

added F::layer_norm and functionality test

56567c1

minor changes

f3f989f

fixed the segfault issue for the functional test

0665dcf

yf225 reviewed Oct 18, 2019

View reviewed changes

anjali411 and others added 4 commits October 18, 2019 11:02

minor

96a6ae8

Merge branch 'master' of https://github.com/pytorch/pytorch into laye…

8316b7c

…r_norm

Merge branch 'master' into layer_norm

2d05948

fix third_party/fbgemm

0567e00

anjali411 added 3 commits October 18, 2019 13:14

added test for LayerNorm forward

c817d71

fixed merge conflict

6637464

reverted changes for Bilinear test

f233692

yf225 reviewed Oct 18, 2019

View reviewed changes

added Tensor() as default for weight and bias in the layer_norm funct…

bc4f980

…ion and updated the test in functional.cpp

yf225 reviewed Oct 21, 2019

View reviewed changes

fixing clang-tidy

c87383b

facebook-github-bot reviewed Oct 21, 2019

View reviewed changes

yf225 added the module: cpp Related to C++ API label Oct 21, 2019

yf225 approved these changes Oct 22, 2019

View reviewed changes

facebook-github-bot closed this in 7b59174 Oct 22, 2019

facebook-github-bot added the merged label Oct 22, 2019

yf225 mentioned this pull request Oct 23, 2019

Update C++ torch::nn parity table for LayerNorm #28484

Closed

thiagocrepaldi pushed a commit to thiagocrepaldi/pytorch that referenced this pull request Feb 4, 2020

torch::nn::LayerNorm

38ada7c

Summary: Pull Request resolved: pytorch#28032 Differential Revision: D18047371 Pulled By: anjali411 fbshipit-source-id: fb61aea52d6622a67ec1d84950e17e85686461ae

mruberry added the Merged label Oct 28, 2020

	auto y = F::layer_norm(input, LayerNormOptions({2, 2}).eps(2e-5), weight, bias);
	auto y = F::layer_norm(input, LayerNormOptions({2, 2}).eps(2e-5));

torch::nn::LayerNorm #28032

torch::nn::LayerNorm #28032

Uh oh!

Conversation

anjali411 commented Oct 15, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yf225 commented Oct 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yf225 commented Oct 18, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yf225 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Oct 22, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

yf225 commented Oct 18, 2019 •

edited

Loading