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

50 constant adam #51

Merged
merged 4 commits into from
Dec 16, 2020
Merged

50 constant adam #51

merged 4 commits into from
Dec 16, 2020

Conversation

cwmeijer
Copy link
Contributor

@cwmeijer cwmeijer commented Dec 14, 2020

add constant learning rate scheduler.
constant_lr argument is only used in transformer experiment for now.

@cwmeijer cwmeijer marked this pull request as ready for review December 14, 2020 09:58
@bhigy
Copy link
Contributor

bhigy commented Dec 14, 2020

Maybe we should take this as an opportunity to use a uniform scheduler creation procedure. I see that different experiment functions use different code (e.g. asr.experiment() has very different rules than basic.experiment()). This should include:

  • a uniform set of accepted optimizers (adadelta, adam...)
  • a uniform set of scheduling options (none, cyclic, noam; where I would use none instead of constant as adaptive optimizers don't use a constant learning rate even without scheduler)
  • uniform parameter names (constant_lr -> lr)
  • a single function in scheduler.py to handle this, called from all experiment() functions

@cwmeijer
Copy link
Contributor Author

That sounds like an excellent idea. I propose we create a new issue in order to fix the things you describe given the number and impact of the proposed changes.

@cwmeijer cwmeijer merged commit ebcd61f into master Dec 16, 2020
@cwmeijer cwmeijer deleted the 50-constant-adam branch January 6, 2021 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants