-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Stepwise LR scheduler #20211
base: master
Are you sure you want to change the base?
Stepwise LR scheduler #20211
Conversation
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
…ytorch-lightning into ddp-strategy-alias
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #20211 +/- ##
=========================================
- Coverage 89% 81% -8%
=========================================
Files 267 264 -3
Lines 23084 23029 -55
=========================================
- Hits 20585 18618 -1967
- Misses 2499 4411 +1912 |
Hii @Borda. Do I need to make any kind of changes in the PR ? |
This looks good, thank you for the contribution @01AbhiSingh Ideally we could add a test to verify the behavior described in #17544. The current test suite can't detect the current change and this is usually a sign of insufficient coverage. Would you be willing to contribute such test? |
Yes, sure let me look into it. |
Hi @lantiga , Do you want a new test written from scratch or need me to make the necessary changes in a preexisting file? All the tests have been passed. If the changes need to be made in a preexisting file, it would be very helpful if you could point out the test in which I need to make the changes, as all the tests have been passed, and due to that, I can't find the test. |
hey @01AbhiSingh sorry for the wait You can take inspiration from:
and add a new test where scheduling goes across epoch boundaries. Maybe @falckt can help too? |
Hey @01AbhiSingh will you have time to add a test? |
Yes, Today's my last exam after that I will add the test asap. Sorry for the delay. |
@01AbhiSingh just checking, no pressure :-) |
What does this PR do?
Fixes #<17544>
Hii @awaelchli. Can you please verify the changes I made. If they are correct then i will take up and correct any failing tests also.
Before submitting
PR review
Anyone in the community is welcome to review the PR.
Before you start reviewing, make sure you have read the review guidelines. In short, see the following bullet-list:
Reviewer checklist
- [ ] Is this pull request ready for review? (if not, please submit in draft mode) - [ ] Check that all items from **Before submitting** are resolved - [ ] Make sure the title is self-explanatory and the description concisely explains the PR - [ ] Add labels and milestones (and optionally projects) to the PR so it can be classified📚 Documentation preview 📚: https://pytorch-lightning--20211.org.readthedocs.build/en/20211/