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.

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

Add strict option to lr_scheduler dict #3586

Merged
merged 5 commits into from
Oct 21, 2020
Merged

Add strict option to lr_scheduler dict #3586

merged 5 commits into from
Oct 21, 2020
+49 −6

Conversation

carmocca
Copy link
Contributor

@carmocca carmocca commented Sep 21, 2020 •
edited
Loading

Fixes #3526

Before submitting

  • Was this discussed/approved via a Github issue? (no need for typos and docs improvements)
  • Did you read the contributor guideline, Pull Request section?
  • Did you make sure your PR does only one thing, instead of bundling different changes together? Otherwise, we ask you to create a separate PR for every change.
  • Did you make sure to update the documentation with your changes?
  • Did you write any new necessary tests?
  • Did you verify new and existing tests pass locally with your changes?
  • If you made a notable change (that affects users), did you update the CHANGELOG?

@mergify mergify bot requested a review from a team September 21, 2020 16:43
@carmocca carmocca mentioned this pull request Sep 21, 2020
Merged
7 tasks
@Borda Borda added the bug Something isn't working label Sep 21, 2020
SkafteNicki
SkafteNicki approved these changes Sep 23, 2020
View reviewed changes
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.

LGTM, test are solid and the failing checks seems to be unrelated to this PR

@mergify mergify bot requested a review from a team September 23, 2020 09:47
@codecov
Copy link

codecov bot commented Sep 24, 2020 •
edited
Loading

Codecov Report

Merging #3586 into master will increase coverage by 3%.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master   #3586    +/-   ##
=======================================
+ Coverage      90%     93%    +3%     
=======================================
  Files         103     103            
  Lines        7842    7844     +2     
=======================================
+ Hits         7053    7279   +226     
+ Misses        789     565   -224

@mergify mergify bot requested a review from a team September 25, 2020 13:41
@mergify
Copy link
Contributor

mergify bot commented Sep 25, 2020

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

@awaelchli
Copy link
Contributor

awaelchli commented Sep 25, 2020 •
edited
Loading

@Borda Isn't this more like a feature/enhancement than bugfix? Is it allowed pre v1.0?

@Borda Borda added feature Is an improvement or enhancement and removed bug Something isn't working labels Sep 25, 2020
@Borda Borda added this to the 1.0 milestone Sep 25, 2020
@mergify
Copy link
Contributor

mergify bot commented Sep 28, 2020

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

@awaelchli awaelchli modified the milestones: 1.0, 1.1 Sep 29, 2020
@mergify
Copy link
Contributor

mergify bot commented Oct 3, 2020

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

@Borda Borda removed the v1.0 post label Oct 13, 2020
@carmocca
Copy link
Contributor Author

Rebased master, can you review? @Borda @awaelchli @rohitgr7

@mergify
Copy link
Contributor

mergify bot commented Oct 20, 2020

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

@SkafteNicki SkafteNicki added the ready PRs ready to be merged label Oct 21, 2020
@mergify mergify bot requested a review from a team October 21, 2020 09:22
@SkafteNicki SkafteNicki merged commit e0f9799 into Lightning-AI:master Oct 21, 2020
@carmocca carmocca deleted the feature/3526_scheduler-strict branch October 21, 2020 12:38
@carmocca carmocca self-assigned this Nov 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SkafteNicki SkafteNicki SkafteNicki approved these changes

@justusschock justusschock justusschock approved these changes

@tchaton tchaton tchaton approved these changes

@awaelchli awaelchli Awaiting requested review from awaelchli

@nateraw nateraw Awaiting requested review from nateraw

@rohitgr7 rohitgr7 Awaiting requested review from rohitgr7

@yukw777 yukw777 Awaiting requested review from yukw777

Assignees

@carmocca carmocca

Labels
feature Is an improvement or enhancement ready PRs ready to be merged
Projects
None yet
Milestone
pl:1.1
Development

Successfully merging this pull request may close these issues.

Add strict option to scheduler
6 participants
@carmocca @awaelchli @tchaton @justusschock @SkafteNicki @Borda