-
Notifications
You must be signed in to change notification settings - Fork 25.7k
Allow SequentialLR to include ChainedScheduler #133450
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
Allow SequentialLR to include ChainedScheduler #133450
Conversation
a SequentialLR - see pytorch#132745
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/133450
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 3236c2b with merge base 487873f ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for tackling this! Could you add a test case in test/optim/test_lrscheduler.py?
|
@janeyx99 Sure, will do. |
|
I've added a test case, so hopefully it passes the CI. |
|
Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as |
|
Ahhh I kicked off the CI on this and forgot to check back in. Could you fix the lint and rebase onto latest main? We can land the PR once CI is green. |
|
I've hopefully fixed the linting issue. |
|
You can install lintrunner locally and verify it all passes: pip install lintrunner |
|
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
This fixes #132745 and allows a
SequentialLRto include schedulers that are compound scheduler types (i.e., aChainedScheduler), which contain a list of schedulers in a_schedulersattribute.