Skip to content

refactor namespaces in lr scheduler #1730

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.

Sign up for GitHub

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

Jump to bottom
Closed
erip wants to merge 14 commits into from
Closed

refactor namespaces in lr scheduler #1730

erip wants to merge 14 commits into from

Conversation

@erip
Copy link
Contributor

@erip erip commented Feb 21, 2020 •
edited
Loading

Before submitting

  • Was this discussed/approved via a Github issue? (no need for typos, doc improvements)
  • Did you read the contributor guideline?
  • Did you make sure to update the docs?
  • Did you write any new necessary tests?

What does this PR do?

Fixes #1672 in part (part 2: context)

PR review

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

Did you have fun?

Make sure you had fun coding 🙃

erip added 12 commits February 20, 2020 20:19
@erip
update base lr scheduler to require from_args classmethod and remove …
077ad87 
…args as an attribute.
@erip
update cosine lr scheduler to incorporate new lr scheduler interface.
853a9c1
@erip
update fixed schedule to incorporate new lr scheduler interface.
ec55cae
@erip
update noam schedule to incorporate new lr scheduler interface.
f64a331
@erip
update reduce_lr_on_plateau schedule to incorporate new lr scheduler …
7096268 
…interface.
@erip
update polynomial decay schedule to incorporate new lr scheduler inte…
5ce3f96 
…rface.
@erip
update tristage schedule to incorporate new lr scheduler interface.
a06db19
@erip
update triangular schedule to incorporate new lr scheduler interface.
654bf94
@erip
fix typo in lr on plateau from_args.
8d12775
@erip
add builder method to conform to registry magic lookup.
9e6d795
@erip
add lrs attribute since the lr list was being pummeled by updating se…
f76da9c 
…lf.lr with a double.
@erip
fix issue where warmup_updates was not set.
482a015
facebook-github-bot
facebook-github-bot reviewed Feb 22, 2020
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@myleott has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@erip
Copy link
Contributor Author

erip commented Feb 25, 2020

This will need to be re-imported because of a merge conflict.

facebook-github-bot
facebook-github-bot reviewed Feb 25, 2020
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@myleott has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@erip
Copy link
Contributor Author

erip commented Mar 11, 2020

I think this one is ready too, @myleott. Wish there was some way to make the FB-side stuff a bit easier for you. ☹️

@erip erip mentioned this pull request Mar 11, 2020
Closed
4 tasks
facebook-github-bot
facebook-github-bot reviewed Mar 13, 2020
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@myleott has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@myleott
Copy link

myleott commented Mar 13, 2020

Wish there was some way to make the FB-side stuff a bit easier for you. ☹️

It's mostly that the full suite of tests take over 6 hours right now, which makes it very slow to iterate. We're working with some of the internal teams on making these tests faster, but may take a while until that's done.

@erip erip closed this Oct 9, 2020
@erip erip deleted the feature/refactor-namespaces-lr-scheduler branch October 9, 2020 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@facebook-github-bot facebook-github-bot facebook-github-bot left review comments

Assignees

No one assigned

Labels

CLA Signed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Replace use of argparse.Namespace with named args and kwargs where possible.

3 participants

@erip @myleott @facebook-github-bot