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

LearningRateLogger in multi-scheduler setting #1944

Merged
merged 5 commits into from
May 28, 2020

Conversation

ivannz
Copy link
Contributor

@ivannz ivannz commented May 25, 2020

This PR addresses two issues outlined in pr #1498:

  • garbled learning rate history in multi-scheduler setting
  • issue warning, rather than a MisconfigurationException, if no schedulers have been configured

@mergify mergify bot requested a review from a team May 25, 2020 14:07
@ivannz ivannz force-pushed the lr-callback-pr branch 2 times, most recently from 2ec5688 to 2c57d5d Compare May 25, 2020 15:12
@ivannz ivannz changed the title fixed undesired behaviour due to dict.fromkeys LearningRateLogger in multi-scheduler setting May 25, 2020
@@ -308,6 +310,24 @@ def test_lr_logger_single_lr(tmpdir):
'Names of learning rates not set correctly'


def test_lr_logger_no_lr(tmpdir):
Copy link
Member

Choose a reason for hiding this comment

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

mind move all LR related test move to tests/callbacks/test_lr.py

@mergify mergify bot requested a review from a team May 25, 2020 19:28
Copy link
Member

@SkafteNicki SkafteNicki left a comment

Choose a reason for hiding this comment

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

Remember to update CHANGELOG

@mergify mergify bot requested a review from a team May 25, 2020 19:41
@mergify mergify bot requested a review from a team May 25, 2020 19:53
@Borda Borda added the bug Something isn't working label May 25, 2020
@Borda Borda added this to the 0.7.7 milestone May 25, 2020
@Borda Borda added the ready PRs ready to be merged label May 25, 2020
@mergify
Copy link
Contributor

mergify bot commented May 25, 2020

This pull request is now in conflict... :(

@ivannz
Copy link
Contributor Author

ivannz commented May 25, 2020

I rebased atop the current master.
@Borda Thank you for adding to the changelog and moving tests.

@Borda Borda modified the milestones: 0.7.7, 0.8.0 May 26, 2020
@williamFalcon williamFalcon merged commit 7c19c37 into Lightning-AI:master May 28, 2020
justusschock pushed a commit that referenced this pull request Jun 29, 2020
* fixed undesired behaviour due to dict.fromkeys

* a test for log length consistency

* runtime-warn if no schedulers are configured

* chlog

* move

Co-authored-by: Jirka <jirka@pytorchlightning.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ready PRs ready to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants