KEMBAR78
Fix `SequentialLR` deprecate warning about invoke `step(epoch)` by zeshengzong · Pull Request #149392 · pytorch/pytorch · GitHub
Skip to content

Fix SequentialLR deprecate warning about invoke step(epoch) #149392

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
Closed
wants to merge 1 commit into from
Closed

Fix SequentialLR deprecate warning about invoke step(epoch) #149392

wants to merge 1 commit into from

Conversation

@zeshengzong
Copy link
Contributor

@zeshengzong zeshengzong commented Mar 18, 2025 •
edited
Loading

Fixes #116776 #76113 #113222 #67958

Changes

  • Refactor LRScheduler.step method, leave epoch check logic in public method step
  • Move update lr logic to _update_lr method
  • Make SequentialLR use _update_lr to avoid unnecessary warning message

Test Result

pytest test/optim/test_lrscheduler.py -vv

image

@pytorch-bot
Copy link

pytorch-bot bot commented Mar 18, 2025 •
edited
Loading

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/149392

Note: Links to docs will display an error until the docs builds have been completed.

✅ You can merge normally! (1 Unrelated Failure)

As of commit 0bef796 with merge base 5cb1d71 (image):

FLAKY - The following job failed but was likely due to flakiness present on trunk:

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

@zeshengzong zeshengzong marked this pull request as ready for review March 18, 2025 09:40
@janeyx99 janeyx99 added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Mar 20, 2025
@albanD albanD removed their request for review April 9, 2025 19:37
@zeshengzong
Copy link
Contributor Author

@pytorchbot rebase -b main

@pytorchmergebot
Copy link
Collaborator

@pytorchbot started a rebase job onto refs/remotes/origin/main. Check the current status here

@pytorchmergebot
Copy link
Collaborator

Successfully rebased fix/optim/sequentiallr onto refs/remotes/origin/main, please pull locally before adding more changes (for example, via git checkout fix/optim/sequentiallr && git pull --rebase)

@pytorchmergebot pytorchmergebot force-pushed the fix/optim/sequentiallr branch from 19ee79e to 342b8df Compare April 14, 2025 08:46
@zeshengzong zeshengzong mentioned this pull request May 14, 2025
Closed
@zeshengzong
Copy link
Contributor Author

@pytorchbot rebase -b main

@pytorchmergebot
Copy link
Collaborator

@pytorchbot started a rebase job onto refs/remotes/origin/main. Check the current status here

@pytorchmergebot
Copy link
Collaborator

Successfully rebased fix/optim/sequentiallr onto refs/remotes/origin/main, please pull locally before adding more changes (for example, via git checkout fix/optim/sequentiallr && git pull --rebase)

@pytorchmergebot pytorchmergebot force-pushed the fix/optim/sequentiallr branch from 342b8df to bf81f79 Compare May 14, 2025 06:56
@zeshengzong
Copy link
Contributor Author

Hi @janeyx99 , please help review this one when available, thanks!

@zeshengzong
Copy link
Contributor Author

@pytorchbot rebase -b main

@pytorchmergebot
Copy link
Collaborator

@pytorchbot started a rebase job onto refs/remotes/origin/main. Check the current status here

@pytorchmergebot
Copy link
Collaborator

Successfully rebased fix/optim/sequentiallr onto refs/remotes/origin/main, please pull locally before adding more changes (for example, via git checkout fix/optim/sequentiallr && git pull --rebase)

@pytorchmergebot pytorchmergebot force-pushed the fix/optim/sequentiallr branch from bf81f79 to 8e9ff0d Compare July 4, 2025 03:05
janeyx99
janeyx99 approved these changes Aug 6, 2025
View reviewed changes
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.

hm, not the most ideal long term solution, but does work to silence the weird warning today.

@janeyx99
Copy link
Contributor

janeyx99 commented Aug 6, 2025

@pytorchbot merge -r

@pytorch-bot
Copy link

pytorch-bot bot commented Aug 6, 2025

Pull workflow has not been scheduled for the PR yet. It could be because author doesn't have permissions to run those or skip-checks keywords were added to PR/commits, aborting merge. Please get/give approval for the workflows and/or remove skip ci decorators before next merge attempt. If you think this is a mistake, please contact PyTorch Dev Infra.

@janeyx99
Copy link
Contributor

janeyx99 commented Aug 6, 2025

@pytorchbot rebase

@pytorchmergebot
Copy link
Collaborator

@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here

@pytorchmergebot
Copy link
Collaborator

Successfully rebased fix/optim/sequentiallr onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout fix/optim/sequentiallr && git pull --rebase)

@pytorchmergebot pytorchmergebot force-pushed the fix/optim/sequentiallr branch from 8e9ff0d to c1accdd Compare August 6, 2025 20:54
@janeyx99
Copy link
Contributor

janeyx99 commented Aug 6, 2025

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Aug 6, 2025
@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

@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: 1 mandatory check(s) failed. The first few are:

Dig deeper by viewing the failures on hud

Details for Dev Infra team Raised by workflow job

Failing merge rule: Core Maintainers

@janeyx99
Copy link
Contributor

janeyx99 commented Aug 6, 2025

ci failures are related, unfortunately

@zeshengzong zeshengzong force-pushed the fix/optim/sequentiallr branch from c1accdd to 0bef796 Compare August 29, 2025 06:42
@pytorch-bot pytorch-bot bot removed the ciflow/trunk Trigger trunk jobs on your pull request label Aug 29, 2025
@zeshengzong
Copy link
Contributor Author

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Aug 29, 2025
@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

This was referenced Sep 9, 2025
Open
Closed
Open
Closed
markc-614 pushed a commit to markc-614/pytorch that referenced this pull request Sep 17, 2025
@zeshengzong @markc-614
Fix SequentialLR deprecate warning about invoke step(epoch) (pyto…
9a85340 
…rch#149392)

Fixes pytorch#116776 pytorch#76113 pytorch#113222 pytorch#67958
## Changes

- Refactor `LRScheduler.step` method, leave `epoch` check logic in public method `step`
- Move update `lr` logic to `_update_lr` method
- Make `SequentialLR` use `_update_lr` to avoid unnecessary warning message

## Test Result

```bash
pytest test/optim/test_lrscheduler.py -vv
```

![image](https://github.com/user-attachments/assets/e1c5527e-193e-4328-bf95-023139ea0416)

Pull Request resolved: pytorch#149392
Approved by: https://github.com/janeyx99
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@janeyx99 janeyx99 janeyx99 approved these changes

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Why there’s a line scheduler.step(0) in the code of class SequentialLR?

4 participants

@zeshengzong @pytorchmergebot @janeyx99 @pytorchbot