-
Notifications
You must be signed in to change notification settings - Fork 9
Add adam optimizer #36
Conversation
src/function.h
Outdated
| float beta2_; | ||
| // store the following items in order: first moment, second moment, beta1_pow, | ||
| // beta2_pow | ||
| std::unordered_map< |
There was a problem hiding this comment.
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.
-
Hyperparameter should be strongly associated with parameters.
- When saving the parameters, we also need to save the parameters' hyperparameter.
- 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
adam1first, then we want to switch to anotheradam2with a different set up(differentbeta1_,beta2_for example), the current implementation would require copying states fromadam1toadam2
-
In the future, I am planning to remove variable's name. The map from
stringtoVariableHandlehas to be changed by then.
There was a problem hiding this comment.
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()?
There was a problem hiding this comment.
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.
tonyyang-svail
left a comment
There was a problem hiding this 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!
|
|
||
| #include <memory> | ||
| #include <string> | ||
| #include <vector> |
There was a problem hiding this comment.
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>
No description provided.