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.

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

【PIR API adaptor No.124】 optimizer.Lamb #58881

Merged
merged 19 commits into from
Dec 25, 2023
Merged

Conversation

Liyulingyue
Copy link
Contributor

PR types

Others

PR changes

APIs

Description

#58067 124

@paddle-bot paddle-bot bot added the contributor External developers label Nov 9, 2023
@luotao1 luotao1 added the HappyOpenSource 快乐开源活动issue与PR label Nov 10, 2023
@@ -783,6 +784,7 @@ def get_optimizer_dygraph(self, parameter_list):
)
return optimizer

@test_with_pir_api

This comment was marked as resolved.

@@ -200,6 +202,7 @@ def test_lamb_op(self):


class TestLambOpMultiPrecision(unittest.TestCase):
@test_with_pir_api

This comment was marked as resolved.

@MarioLulab
Copy link
Contributor

还貌似遗漏了 test/legacy_test/test_lamb_op.py 的单测适配?

Copy link
Contributor

@YuanRisheng YuanRisheng left a comment

Choose a reason for hiding this comment

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

image
这里新ir并没有适配这个未来会废弃掉的接口,可以尝试修改使用paddle.nn.Linear接口

test/legacy_test/test_lambv2_op.py Outdated Show resolved Hide resolved

This comment was marked as resolved.

Liyulingyue and others added 2 commits November 16, 2023 12:37
Co-authored-by: Lu Qi <61354321+MarioLulab@users.noreply.github.com>
Co-authored-by: Lu Qi <61354321+MarioLulab@users.noreply.github.com>
@MarioLulab
Copy link
Contributor

需要 pre-commit 一下

Copy link

paddle-ci-bot bot commented Nov 29, 2023

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

This comment was marked as resolved.

This comment was marked as resolved.

Copy link
Contributor

Choose a reason for hiding this comment

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

image

fetch_list = [avg_loss]

image

fetch_list = [loss]

Copy link
Contributor

Choose a reason for hiding this comment

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

test_lamb_op_with_multi_steps 单测下的所有 base.Program() 麻烦改成 paddle.static.Program()

test/legacy_test/test_imperative_base.py Outdated Show resolved Hide resolved
MarioLulab

This comment was marked as resolved.

Co-authored-by: Lu Qi <61354321+MarioLulab@users.noreply.github.com>
@MarioLulab

This comment was marked as resolved.

Copy link
Contributor

@MarioLulab MarioLulab left a comment

Choose a reason for hiding this comment

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

nice work~ 但是遇到了不兼容的问题,需要补充到 pr 描述里

@@ -266,6 +270,7 @@ def get_parameter(var):
np.testing.assert_array_equal(bias_np, get_parameter(bias))
return weight_np, bias_np

@test_with_pir_api
Copy link
Contributor

Choose a reason for hiding this comment

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

先取消此单测,因为混合精度单测涉及 class OptimizerWithMixedPrecision,该 class 暂不支持 pir 模式。请在 pr 描述里补充说明一下这个情况,并更新单测覆盖率~

@MarioLulab
Copy link
Contributor

需要 pre-commit 一下

Copy link
Contributor

@MarioLulab MarioLulab left a comment

Choose a reason for hiding this comment

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

LGTM
lamb 的混合精度单测涉及 class OptimizerWithMixedPrecision,该 class 暂不支持 pir 模式。请在 pr 描述里补充说明一下这个情况,并更新单测覆盖率~

Copy link
Contributor

@0x45f 0x45f left a comment

Choose a reason for hiding this comment

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

辛苦更新下单测覆盖率~

@0x45f 0x45f merged commit 3969332 into PaddlePaddle:develop Dec 25, 2023
29 checks passed
Wanglongzhi2001 pushed a commit to Wanglongzhi2001/Paddle that referenced this pull request Jan 7, 2024
@Liyulingyue Liyulingyue deleted the 124 branch January 15, 2024 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor External developers HappyOpenSource 快乐开源活动issue与PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants