-
Notifications
You must be signed in to change notification settings - Fork 7
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
ENH: add adaptive optimizers for all mappings #315
ENH: add adaptive optimizers for all mappings #315
Conversation
…enh-net-forward-pass
…enh-net-forward-pass
…o enh-combine-optimizers
…tead of nan for optimize control info with net
MAINT: merge branch main into enh-adaptive-opt-for-all-mappings
…am/main' into enh-adaptive-optimizers
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.
Voila une première passe de relecture. On peut attendre la review de @pag13 pour après échanger sur Skype pour finaliser tout ca !
smash/factory/net/net.py
Outdated
@@ -532,94 +531,111 @@ def _fit_d2p( | |||
random_state=random_state, | |||
) | |||
|
|||
# % First evaluation | |||
# calculate the gradient of J wrt rr_parameters (output of the regionalization NN) |
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.
pas que rr_parameters en soit ?
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.
# calculate the gradient of J wrt rr_parameters (output of the regionalization NN)
# and get the gradient of the parameterization NN if used
init_cost_grad, nn_parameters_b = _hcost_prime(
self, x_train, model_params_states, instance, parameters, wrap_options, wrap_returns
)
J'ai fait pour les deux gradients en fait.
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.
JE voulais dire, aussi rr_initial_states non ?
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.
Travail impressionnant et très rigoureux Truyen, bravo ! Merci pour la bonne review François.
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.
Tu as fait un commit pdt la review. Je sais pas vraiment ce qui va s'afficher ^^"
Le seul point de modification important c'est sur le suffix _reg. On en rediscute
smash/factory/net/net.py
Outdated
@@ -532,94 +531,111 @@ def _fit_d2p( | |||
random_state=random_state, | |||
) | |||
|
|||
# % First evaluation | |||
# calculate the gradient of J wrt rr_parameters (output of the regionalization NN) |
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.
JE voulais dire, aussi rr_initial_states non ?
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.
Bon pour moi merci !
# calculate the gradient of J wrt rr_parameters and rr_initial_states | ||
# that are the output of the descriptors-to-parameters (d2p) NN | ||
# and get the gradient of the pmtz NN (pmtz) if used |
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.
Top
Suggested reviewer order: FC, PAG.
Adaptive optimizers (i.e. adam, adagrad, sgd, and rmsprop) are now possible for all mappings. This PR results in the following important changes:
optimize.py
filesmash/fcore/external/lbfgsb.f