Skip to content
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

replace PiecewiseDecay, StepDecay, MultiStepDecay, LambdaDecay with 2.0 version #53992

Merged
merged 6 commits into from
Jun 28, 2023
Merged

replace PiecewiseDecay, StepDecay, MultiStepDecay, LambdaDecay with 2.0 version #53992

merged 6 commits into from
Jun 28, 2023

Conversation

longranger2
Copy link
Contributor

@longranger2 longranger2 commented May 19, 2023 •
edited
Loading

PR types

Others

PR changes

APIs

Description

  • remove the PiecewiseDecay in python/paddle/fluid/dygraph/learning_rate_scheduler.py and use PiecewiseDecay in python/paddle/optimizer/lr.py to replace it
  • remove the StepDecay in python/paddle/fluid/dygraph/learning_rate_scheduler.py and use StepDecay in python/paddle/optimizer/lr.py to replace it
  • remove the MultiStepDecay in python/paddle/fluid/dygraph/learning_rate_scheduler.py and use MultiStepDecay in python/paddle/optimizer/lr.py to replace it
  • remove the LambdaDecay in python/paddle/fluid/dygraph/learning_rate_scheduler.py and use LambdaDecay in python/paddle/optimizer/lr.py to replace it

@paddle-bot
Copy link

paddle-bot bot commented May 19, 2023

你的PR提交成功，感谢你对开源项目的贡献!
请关注后续CI自动化测试结果，详情请参考Paddle-CI手册。
Your PR has been submitted. Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

@paddle-bot paddle-bot bot added contributor External developers status: proposed labels May 19, 2023
@paddle-bot
Copy link

paddle-bot bot commented May 19, 2023

❌ The PR is not created using PR's template. You can refer to this Demo.
Please use PR's template, it helps save our maintainers' time so that more developers get helped.

@longranger2
Copy link
Contributor Author

补充说明：
旧版Optimizer基类的current_step_lr函数针对PiecewiseDecay学习率的时候会进行step操作

image

而2.0版本的Optimizer基类的get_lr()是没有这个操作的

image

注意到get_lr函数中的注释对于这种情况需要对学习率进行step()操作，这样两者的行为就可以保持一致了

image

zoooo0820
zoooo0820 previously approved these changes May 29, 2023
View reviewed changes
Copy link
Contributor

@zoooo0820 zoooo0820 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@longranger2 longranger2 changed the title replace PiecewiseDecay(LearningRateDecay) with PiecewiseDecay(LRScheduler) replace PiecewiseDecay, StepDecay, MultiStepDecay, LambdaDecay with 2.0 version May 31, 2023
zoooo0820
zoooo0820 reviewed Jun 1, 2023
View reviewed changes
python/paddle/optimizer/lr.py
@@ -98,6 +98,8 @@ def __init__(self, learning_rate=0.1, last_epoch=-1, verbose=False):
type(learning_rate)
)
)
if learning_rate < 0:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里为什么要补充这个限制呢，和某个API的单测有关吗

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

是的，python/paddle/fluid/tests/unittests/test_imperative_optimizer.py文件中针对MultiStepDecay的学习率会进行是否为负数的判断

image

旧的版本是有对学习率是否为负数的判断，而2.0版本是没有该判断的，因此才考虑加进去

image

python/paddle/fluid/tests/unittests/test_learning_rate_scheduler.py Outdated
)
for epoch in range(10):
right_result = multi_step_decay(
epoch, learning_rate, milestones, decay_rate
)
fluid_result = adam.current_step_lr()
scheduler.epoch()
fluid_result = adam.get_lr()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

paddle.optimizer.lr.LRScheduler本身有test_imperative_optimizer_v2.py这个文件做测试。

目前看，test_learning_rate_scheduler.py这个文件本身是用来测试fluid下LearningRateDecay相关的内容。这个PR中验证通过替换成paddle.optimizer.lr.LRScheduler后仍然能正确运转即可，后续可以考虑直接移除这个单测文件

@paddle-ci-bot
Copy link

paddle-ci-bot bot commented Jun 9, 2023

Sorry to inform you that f9634c5's CIs have passed for more than 7 days. To prevent PR conflicts, you need to re-run all CIs manually.

jeff41404
jeff41404 previously approved these changes Jun 26, 2023
View reviewed changes
Copy link
Contributor

@jeff41404 jeff41404 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

sunzhongkai588
sunzhongkai588 previously approved these changes Jun 27, 2023
View reviewed changes
Copy link
Contributor

@sunzhongkai588 sunzhongkai588 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no doc'c changes. LGTM

sunzhongkai588
sunzhongkai588 approved these changes Jun 28, 2023
View reviewed changes
Copy link
Contributor

@sunzhongkai588 sunzhongkai588 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no doc'c changes. LGTM

@jeff41404 jeff41404 merged commit 63f242b into PaddlePaddle:develop Jun 28, 2023
@longranger2 longranger2 deleted the PiecewiseDecay branch July 8, 2023 01:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeff41404 jeff41404 jeff41404 approved these changes

@sunzhongkai588 sunzhongkai588 sunzhongkai588 approved these changes

@zoooo0820 zoooo0820 Awaiting requested review from zoooo0820

Assignees
No one assigned
Labels
contributor External developers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@longranger2 @jeff41404 @zoooo0820 @sunzhongkai588