KEMBAR78
Allow SequentialLR to include ChainedScheduler by mattpitkin · Pull Request #133450 · pytorch/pytorch · GitHub
Skip to content

Conversation

@mattpitkin
Copy link
Contributor

This fixes #132745 and allows a SequentialLR to include schedulers that are compound scheduler types (i.e., a ChainedScheduler), which contain a list of schedulers in a _schedulers attribute.

@pytorch-bot
Copy link

pytorch-bot bot commented Aug 14, 2024

🔗 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 Failures

As of commit 3236c2b with merge base 487873f (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Aug 14, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

Copy link
Contributor

@janeyx99 janeyx99 left a 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?

@mattpitkin
Copy link
Contributor Author

@janeyx99 Sure, will do.

@mattpitkin
Copy link
Contributor Author

I've added a test case, so hopefully it passes the CI.

@bdhirsh bdhirsh added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Aug 16, 2024
@github-actions
Copy link
Contributor

Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as Stale.
Feel free to remove the Stale label if you feel this was a mistake.
If you are unable to remove the Stale label please contact a maintainer in order to do so.
If you want the bot to never mark this PR stale again, add the no-stale label.
Stale pull requests will automatically be closed after 30 days of inactivity.

@github-actions github-actions bot added the Stale label Oct 15, 2024
@janeyx99
Copy link
Contributor

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.

@albanD albanD removed their request for review October 16, 2024 20:16
@mattpitkin
Copy link
Contributor Author

I've hopefully fixed the linting issue.

@janeyx99
Copy link
Contributor

You can install lintrunner locally and verify it all passes:

pip install lintrunner
lintrunner init
lintrunner -a

@janeyx99
Copy link
Contributor

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Oct 17, 2024
@pytorchmergebot
Copy link
Collaborator

Merge started

Your 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

Advanced Debugging
Check the merge workflow status
here

@mattpitkin mattpitkin deleted the allow_chained_scheduler_in_sequentiallr branch October 18, 2024 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/trunk Trigger trunk jobs on your pull request Merged open source release notes: optim Stale triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow SequentialLR scheduler to be able to use ChainedSchedulers

5 participants