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

RMSprop implementation based on G. Hinton Lecture 6 #1890

Closed
wants to merge 9 commits into from
Closed

RMSprop implementation based on G. Hinton Lecture 6 #1890

wants to merge 9 commits into from

Conversation

erogol
Copy link
Contributor

@erogol erogol commented Feb 17, 2015

I implemented RMSprop as Hinton suggests and alternatively you can apply momentum as well with RMSprop suggested by http://www.cs.toronto.edu/~tijmen/csc321/slides/lecture_slides_lec6.pdf

From my experiments I observed much better convergence rate compared to other implemented solver methods. Here's my another toy experiment if you like to see more empirical results. http://www.erogol.com/

Any comments are very welcome.

Here is the mnist example output, https://gist.github.com/erogol/5c5b7beae8b088bd00e9

@shelhamer
Copy link
Member

Thanks for the solver -- RMSprop has been on our list so we'll take a look when we can.

Please drop the logs from versioning and instead post a gist in your PR description for us to read.

@erogol
Copy link
Contributor Author

erogol commented Feb 17, 2015

Sorry I forgot to remove these logs but now they are gone.

@erogol
Copy link
Contributor Author

erogol commented Feb 17, 2015

I add rmsprop example to mnist example folder also I ll give gist link as you liked.

@erogol erogol changed the title RMSprop+Momentum implementation based on G. Hinton Lecture 6 RMSprop implementation based on G. Hinton Lecture 6 Feb 19, 2015
@shelhamer shelhamer added the JD label Mar 10, 2015
@ducha-aiki
Copy link
Contributor

@erogol, I have tested it and it works quite fine :)

@erogol
Copy link
Contributor Author

erogol commented Apr 2, 2015

@ducha-aiki yeah :) thank you... However there are some compilation issues I guess pointed by Travis.

@seanbell
Copy link

Are there any plans on porting this to the newest master branch?

@@ -628,7 +628,7 @@ void NesterovSolver<Dtype>::ComputeUpdateValue() {
net_params[param_id]->cpu_diff(), momentum,
this->history_[param_id]->mutable_cpu_data());

// compute udpate: step back then over step
// compute uppate: step back then over step
Copy link
Contributor

Choose a reason for hiding this comment

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

Well, you almost fixed the typo :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah :)

@cbalint13
Copy link
Contributor

@erogol ,

  • Could update for latest upstream changes ?
  • Also can please update to not use 'tabs' but use spaces instead? (this is very bad)
  • Would be also nice to squash all you commits into one.

@erogol
Copy link
Contributor Author

erogol commented Jul 27, 2015

I will take care of it in one of the available times but in a very buzzy period right now.

@beniz
Copy link

beniz commented Jul 29, 2015

For those interested, I've merged this PR against my own slightly modified Caffe tree which is up to date with current master, and it appears to work fine on a few applications I've tested. However, two points:

  • the unit tests for the gradient do fail for RMSProp and I've been unable to fix them
  • I can't tell whether the results do match those of a reference implementation

To reproduce the merge against master, you should be able to use the last two commits from https://github.com/beniz/caffe/tree/master_dd_integ_rmsprop

@shelhamer
Copy link
Member

Closing in favor of follow-up #2867. Thanks for the first version @erogol.

@shelhamer shelhamer closed this Aug 6, 2015
@shelhamer shelhamer removed the focus label Aug 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants