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

Conversation

@jon-tow
Copy link
Contributor

@jon-tow jon-tow commented Jul 8, 2019

Summary

Random Initializers

  • Adds tests, testRandomUniform(), testRandomNormal(), and testTruncatedNormal().

Variance-Scaling Initializers

  • Refactor computation of fans, fanIn and fanOut.
  • Add paper references to glorot initializers.
  • Update glorotNormal to use truncatedNormal initialization.
  • Add tests, testGlorotUniform() and testGlorotNormal().

@rxwei rxwei requested a review from eaplatanios July 8, 2019 03:32
@rxwei rxwei added the enhancement New feature or request label Jul 8, 2019
jon-tow and others added 2 commits July 8, 2019 00:17
@Shashi456
Copy link
Contributor

Shashi456 commented Jul 8, 2019

The error here is

/swift-apis/Tests/TensorFlowTests/LayerTests.swift:366: error: LayerTests.testRNN : XCTAssertEqual failed:
 ("[[[ 0.20775774,  0.20080023, -0.13768704, -0.18534681]], [[ 0.22666009,  0.30019346, -0.19720285,   
-0.146838]], [[ 0.23758982,  0.32101023, -0.20359214, -0.17870961]], [[ 0.24337786,  0.33891943, 
-0.21143381, -0.16750808]]]") is not equal to ("[[[ 0.20775771,  0.20080023, -0.13768704, -0.18534681]], [[ 
0.22666009,  0.30019346, -0.19720285, -0.14683801]], [[ 0.23758979,  0.32101023, -0.20359215,  
-0.1787096]], [[ 0.24337786,   0.3389194, -0.21143384,  -0.1675081]]]") -

But i thought we commented out the RNN tests due to variation of outputs.
cc: @eaplatanios

@jon-tow jon-tow changed the title Refactor random and variance-scaling initializers [Initializers] Refactor random and variance-scaling initializers Jul 8, 2019
@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@marcrasi
Copy link
Contributor

Hi, looks like there was a CI failure on this one. Could you take a look? Once we get CI passing, this should be ready to merge!

@marcrasi marcrasi self-assigned this Nov 14, 2019
@jon-tow
Copy link
Contributor Author

jon-tow commented Nov 15, 2019

Hey @marcrasi! Everything should be good to go. Let me know if anything needs to be addressed. :)

@marcrasi
Copy link
Contributor

It looks like this has broken LayerTests.testRNN, LayerTests.testLSTM, and SequentialTests.testSequential because they depend on specific initialization values that have changed. Is it expected that the values change? If so, updating the constants in those tests should be good.

@jon-tow
Copy link
Contributor Author

jon-tow commented Nov 16, 2019

It is expected. The previous implementations of Tensor(glorotUniform:) and Tensor(glorotNormal:) scaled and shifted drawn samples prior to variance scaling. I'm not sure what the reasoning was behind this and haven't seen it mentioned in any of the initialization literature.

I've updated the hardcoded values in the tests you've mentioned. :)

@jon-tow
Copy link
Contributor Author

jon-tow commented Nov 18, 2019

This passes locally but the kokoro build results show a discrepancy between values and expected-values in only the least significant digits. This is probably just a difference in float precision round-off. I'm going to add a bit of accuracy tolerance in the equality assertions to remediate this for now.

@marcrasi
Copy link
Contributor

Yay, it passed, thanks! Merging.

@marcrasi marcrasi merged commit b5a49b7 into tensorflow:master Nov 19, 2019
@jon-tow
Copy link
Contributor Author

jon-tow commented Nov 19, 2019

No problem! Thanks for the help :)

@jon-tow jon-tow deleted the initializers/refactor branch November 19, 2019 00:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants