Skip to content
This repository has been archived by the owner on Jul 7, 2023. It is now read-only.

WIP: Added new initializers #1666

Merged
merged 18 commits into from
Aug 28, 2019
Merged

WIP: Added new initializers #1666

merged 18 commits into from
Aug 28, 2019

Conversation

joaogui1
Copy link
Contributor

Adding kaiming uniform and normal and lecun uniform and normal initializers, besides adding the more general variance scaling initializer

@googlebot googlebot added the cla: yes PR author has signed CLA label Aug 20, 2019
@joaogui1
Copy link
Contributor Author

I believe I'm done here

Copy link
Contributor

@lukaszkaiser lukaszkaiser left a comment

Choose a reason for hiding this comment

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

In addition to the changes below, please make sure each initializer is at least ran once in initializers_test -- that'd catch the syntax errors that were left here (running pylint is also enoucraged).

tensor2tensor/trax/layers/initializers.py Outdated Show resolved Hide resolved
tensor2tensor/trax/layers/initializers.py Outdated Show resolved Hide resolved
tensor2tensor/trax/layers/initializers.py Show resolved Hide resolved
tensor2tensor/trax/layers/initializers.py Outdated Show resolved Hide resolved
@joaogui1
Copy link
Contributor Author

Implemented the requested changes

Copy link
Contributor

@lukaszkaiser lukaszkaiser left a comment

Choose a reason for hiding this comment

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

There were unnamed variables, so the tests should have failed. It's strange if the test passed with this code. Please correct the code and run the tests manually to make sure they pass.

tensor2tensor/trax/layers/initializers.py Outdated Show resolved Hide resolved
tensor2tensor/trax/layers/initializers.py Show resolved Hide resolved
tensor2tensor/trax/layers/initializers.py Outdated Show resolved Hide resolved
@joaogui1
Copy link
Contributor Author

Fixed those parts and testes on my machine, should work now

Copy link
Contributor

@lukaszkaiser lukaszkaiser left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@lukaszkaiser lukaszkaiser merged commit 76803de into tensorflow:master Aug 28, 2019
tensorflow-copybara pushed a commit that referenced this pull request Aug 28, 2019
PiperOrigin-RevId: 265800010
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes PR author has signed CLA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants