Skip to content
This repository has been archived by the owner on Jan 24, 2024. It is now read-only.

Add adam optimizer #36

Merged
merged 7 commits into from
Jun 27, 2018
Merged

Conversation

kexinzhao
Copy link
Collaborator

No description provided.

src/function.h Outdated
float beta2_;
// store the following items in order: first moment, second moment, beta1_pow,
// beta2_pow
std::unordered_map<
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel the following four hyperparameters should be owned by Variable.

  1. Hyperparameter should be strongly associated with parameters.

    1. When saving the parameters, we also need to save the parameters' hyperparameter.
    2. The lifetime of a hyperparameter should be the same as the parameter, not the optimizer. For example, when we try to optimize the network with adam1 first, then we want to switch to another adam2 with a different set up(different beta1_, beta2_ for example), the current implementation would require copying states from adam1 to adam2
  2. In the future, I am planning to remove variable's name. The map from string to VariableHandle has to be changed by then.

Copy link
Collaborator

Choose a reason for hiding this comment

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

How about adding something like Variable::FirstMoment()?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Totally agree. I choose to use a map to store different optimizer's hyperparams so that we can conveniently add other optimizer that use hypers. Code has been modified.

Copy link
Collaborator

@tonyyang-svail tonyyang-svail left a comment

Choose a reason for hiding this comment

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

Look good to me!

@@ -15,6 +15,7 @@

#include <memory>
#include <string>
#include <vector>
Copy link
Collaborator

Choose a reason for hiding this comment

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

mnior change: add #include <unordered_map>

@kexinzhao kexinzhao merged commit e5b5539 into PaddlePaddle:develop Jun 27, 2018
@kexinzhao kexinzhao deleted the adam_optmizer branch June 27, 2018 17:43
@tonyyang-svail tonyyang-svail mentioned this pull request Jun 27, 2018
36 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants