Add: Rectified Adam optimizer #564

vballoli · 2019-11-22T08:24:55Z

No description provided.

googlebot · 2019-11-22T08:24:59Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

vballoli · 2019-11-22T08:26:19Z

@googlebot I signed it!

googlebot · 2019-11-22T08:26:22Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

Shashi456

Also please add a test, in the case of optimizers you can do that by adding a line here.

Shashi456 · 2019-11-22T09:07:24Z

Adding this here for other reviewers, is firstMoments_h really needed? and if the _h variables are just filler variables then you should not add the blank space between variables IMO.

vballoli · 2019-11-24T16:04:46Z

Added the test. Hoping for comments on this addition.

dan-zheng

Thank you for adding a test!

If you have time, could you please create a new file Tests/TensorFlow/OptimizerTests.swift and add an optimizer correctness test as well? That would be a huge help!

Here's a simple reference test for rectified Adam that we can copy.

dan-zheng · 2019-11-24T17:07:06Z

Tests/TensorFlowTests/SequentialTests.swift

@@ -58,6 +59,7 @@ final class SequentialTests: XCTestCase {
                amsgrad.update(&model, along: 𝛁model)
                adagrad.update(&model, along: 𝛁model)
                adadelta.update(&model, along: 𝛁model)
+                radam.update(&model, along: 𝛁model)
            }
        }
        XCTAssertEqual(model.inferring(from: [[0, 0], [0, 1], [1, 0], [1, 1]]),


I wonder if you ran this test locally, after adding RAdam? The expected results of model.inferring should be updated.

+1, do check if build and test pass locally.

The test did pass locally. Will run it again and get back to you.

Test is failing now - result's going to NaN. Anyway I can debug this ?

Test is failing now - result's going to NaN. Anyway I can debug this ?

I'd recommend writing a standalone unit test for rectified Adam. This will help isolate the issue.

Here's a simple reference test for rectified Adam - would you like to port it to a new file Tests/TensorFlow/OptimizerTests.swift?

Yeah, I will be porting it to a new file like you've suggested. I've found the issue, will make the changes and submit OptimizerTests as a separate PR.

Looks like the test is still failing:

XCTAssertEqual failed: ("[[0.5567076], [0.5567076], [0.5567076], [0.5567076]]") is not equal to ("[[0.5053005], [0.5053005], [0.5053005], [0.5053005]]")

sgugger

There a few issues if we want to match the implementation with the official Pytorch one (those are small differences between the math in the paper and the actual code the authors provided).

Sources/TensorFlow/Optimizers/MomentumBased.swift

Removed redundant variables, updated RAdam test and added epsilon to secondMoments_h

vballoli · 2019-12-06T16:49:45Z

I've made the changes as pointed by @sgugger. I'll be sending a separate PR for optimizer correctness for Rectified Adam and other optimizers as suggested by @dan-zheng

dan-zheng

Looks good! I applied some minor style changes in 0794ac4.

sgugger · 2019-12-06T19:39:13Z

Thanks a lot!

Add: Rectified Adam optimizer

5bde6f8

Shashi456 reviewed Nov 22, 2019

View reviewed changes

Add RAdam test

3c22d78

vballoli mentioned this pull request Nov 24, 2019

Add more optimizers and losses #127

Open

27 tasks

dan-zheng reviewed Nov 24, 2019

View reviewed changes

dan-zheng added the tests needed Tests are needed for this pull request label Nov 26, 2019

sgugger suggested changes Dec 4, 2019

View reviewed changes

Sources/TensorFlow/Optimizers/MomentumBased.swift Outdated Show resolved Hide resolved

Sources/TensorFlow/Optimizers/MomentumBased.swift Outdated Show resolved Hide resolved

burmako assigned sgugger Dec 4, 2019

vballoli added 2 commits December 6, 2019 22:14

Changes to RAdam based on original implementation

da43ba3

Removed redundant variables, updated RAdam test and added epsilon to secondMoments_h

Merge branch 'master' into master

cb07efa

vballoli requested review from sgugger and dan-zheng December 6, 2019 16:50

sgugger approved these changes Dec 6, 2019

View reviewed changes

sgugger added the kokoro:force-run label Dec 6, 2019

kokoro-team removed the kokoro:force-run label Dec 6, 2019

vballoli and others added 2 commits December 6, 2019 23:58

Fix RAdam test values

6a74464

NFC: style changes.

0794ac4

dan-zheng approved these changes Dec 6, 2019

View reviewed changes

sgugger added the kokoro:force-run label Dec 6, 2019

kokoro-team removed the kokoro:force-run label Dec 6, 2019

sgugger merged commit efb4596 into tensorflow:master Dec 6, 2019

vballoli mentioned this pull request Feb 27, 2020

Add optimizer tests #696

Closed

Add: Rectified Adam optimizer #564

Add: Rectified Adam optimizer #564

Uh oh!

Conversation

vballoli commented Nov 22, 2019

Uh oh!

googlebot commented Nov 22, 2019

What to do if you already signed the CLA

Individual signers

Corporate signers

Uh oh!

vballoli commented Nov 22, 2019

Uh oh!

googlebot commented Nov 22, 2019

Uh oh!

Shashi456 left a comment

Choose a reason for hiding this comment

Uh oh!

Shashi456 commented Nov 22, 2019

Uh oh!

vballoli commented Nov 24, 2019

Uh oh!

dan-zheng left a comment

Choose a reason for hiding this comment

Uh oh!

dan-zheng Nov 24, 2019

Choose a reason for hiding this comment

Uh oh!

Shashi456 Nov 24, 2019

Choose a reason for hiding this comment

Uh oh!

vballoli Nov 25, 2019

Choose a reason for hiding this comment

Uh oh!

vballoli Nov 26, 2019

Choose a reason for hiding this comment

Uh oh!

dan-zheng Nov 26, 2019

Choose a reason for hiding this comment

Uh oh!

vballoli Nov 26, 2019

Choose a reason for hiding this comment

Uh oh!

sgugger Dec 6, 2019

Choose a reason for hiding this comment

Uh oh!

sgugger left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vballoli commented Dec 6, 2019

Uh oh!

dan-zheng left a comment

Choose a reason for hiding this comment

Uh oh!

sgugger commented Dec 6, 2019

Uh oh!

Uh oh!