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
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

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 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.

❌ 4 New Failures, 1 Unrelated Failure

As of commit c1accdd with merge base d87161c (image):

NEW FAILURES - The following jobs have failed:

UNSTABLE - The following job is marked as unstable, possibly due to flakiness on trunk:

  • pull / linux-jammy-py3_9-clang9-xla / test (xla, 1, 1, linux.12xlarge, unstable) (gh) (#158876)
    /var/lib/jenkins/workspace/xla/torch_xla/csrc/runtime/BUILD:476:14: Compiling torch_xla/csrc/runtime/xla_util_test.cpp failed: (Exit 1): gcc failed: error executing CppCompile command (from target //torch_xla/csrc/runtime:xla_util_test) /usr/bin/gcc -U_FORTIFY_SOURCE -fstack-protector -Wall -Wunused-but-set-parameter -Wno-free-nonheap-object -fno-omit-frame-pointer -g0 -O2 '-D_FORTIFY_SOURCE=1' -DNDEBUG -ffunction-sections ... (remaining 229 arguments skipped)

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

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