Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Nadam updater clone missing schedule #8243

Merged

Conversation

braun-steven
Copy link
Contributor

What changes were proposed in this pull request?

Nadam.clone() did not include the learningRateSchedule.

Quick checklist

The following checklist helps ensure your PR is complete:

  • Eclipse Contributor Agreement signed, and signed commits - see IP Requirements page for details
  • Reviewed the Contributing Guidelines and followed the steps within.
  • Created tests for any significant new code additions.
  • Relevant tests for your changes are passing. (The Nadam test does not make use of clone() so I don't see any relevant tests)

Signed-off-by: Steven Lang <steven.lang.mz@gmail.com>
braun-steven added a commit to Waikato/wekaDeeplearning4j that referenced this pull request Sep 17, 2019
Fixes around 100 test cases that were broken.

Tests that are not yet fixed:

- NeuralNetworkConfigurationTest.testUpdater
- NeuralNetworkConfigurationTest.testBiasUpdater

For these two, it is necessary, that PR
deeplearning4j/deeplearning4j#8243
is being merged. Until then, Nadam learning rate schedules will not work
properly
@raver119 raver119 merged commit d58a4b4 into deeplearning4j:master Sep 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants