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.35、40】 Migrate paddle.nn.ChannelShuffle/ClipGradByNorm into pir #60192

Closed
wants to merge 0 commits into from

Conversation

fsczz
Copy link
Contributor

@fsczz fsczz commented Dec 20, 2023

PR types

Others

PR changes

APIs

Description

PIR API 推全升级
paddle.nn.ClipGradByNorm 引用了clip_by_norm,对clip_by_norm迁移升级至 pir,并更新单测,test_clip_by_norm_op单测覆盖率:4/4 test_gradient_clip 单测覆盖率:2/2
paddle.nn.ChannelShuffle 引用了paddle.nn.functional.channel_shuffle,对channel_shuffle迁移升级至 pir,并更新单测, 单测覆盖率:6/6(TestChannelShuffleError 通过)

Copy link

paddle-bot bot commented Dec 20, 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 the contributor External developers label Dec 20, 2023
@luotao1 luotao1 added the HappyOpenSource 快乐开源活动issue与PR label Dec 20, 2023
@fsczz fsczz changed the title Migrate paddle.nn.ChannelShuffle/ClipGradByNorm into pir 【PIR API adaptor No.35、40】 Migrate paddle.nn.ChannelShuffle/ClipGradByNorm into pir Dec 20, 2023
@MarioLulab
Copy link
Contributor

#58067

问题已收到,正在排查

@MarioLulab
Copy link
Contributor

MarioLulab commented Dec 22, 2023

test_gradient_clip.py 文件下的 TestGradientClipByNorm 单测也需要迁移。具体做法为:对 TestGradientClipByNorm 新增添一个 TestPirGradientClipByNorm 单测。可以参考该文件下 TestPirGradientClipByGlobalNormTestGradientClipByGlobalNorm 的做法。
paddle.static.nn.fc 为静态图专用 api,需要改写成 paddle.nn.Linear 和 paddle.nn.functional.relu 或 paddle.nn.functional.softmax 的形式

@fsczz
Copy link
Contributor Author

fsczz commented Dec 24, 2023

已经添加了TestPirGradientClipByNorm 并测试通过

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~
但 ClipGradByNorm 的 pir 单测还需要适配 test_none_grad 这个单测,否则 ci 覆盖率不达标
image

@0x45f 帮忙看一下这个问题:pir.Value 没有原来的 "need_clip" 属性,导致 pir 的单测与旧ir单测无法对齐,ci 覆盖率不达标,是否需要删除 _pir_clip 函数中对 need_clip 属性的判断?

@0x45f
Copy link
Contributor

0x45f commented Dec 25, 2023

nice work~ 但 ClipGradByNorm 的 pir 单测还需要适配 test_none_grad 这个单测,否则 ci 覆盖率不达标 image

@0x45f 帮忙看一下这个问题:pir.Value 没有原来的 "need_clip" 属性,导致 pir 的单测与旧ir单测无法对齐,ci 覆盖率不达标,是否需要删除 _pir_clip 函数中对 need_clip 属性的判断?

这里可以尝试在python/paddle/pir/core.py文件create_parameter函数最后return之前添加下面两行代码:

need_clip = kwargs.get('need_clip', True)
setattr(param, 'need_clip', need_clip)

@fsczz
Copy link
Contributor Author

fsczz commented Dec 25, 2023

nice work~ 但 ClipGradByNorm 的 pir 单测还需要适配 test_none_grad 这个单测,否则 ci 覆盖率不达标 image
@0x45f 帮忙看一下这个问题:pir.Value 没有原来的 "need_clip" 属性,导致 pir 的单测与旧ir单测无法对齐,ci 覆盖率不达标,是否需要删除 _pir_clip 函数中对 need_clip 属性的判断?

这里可以尝试在python/paddle/pir/core.py文件create_parameter函数最后return之前添加下面两行代码:

need_clip = kwargs.get('need_clip', True)
setattr(param, 'need_clip', need_clip)

添加python/paddle/pir/core.py文件代码之后,还需要在TestPirGradientClipByNorm的测试中添加test_none_grad单侧么

 def test_none_grad(self):
        clip = paddle.nn.ClipGradByNorm(self.clip_norm)
        x = (
            base.default_main_program()
            .global_block()
            .create_parameter(
                name="x", shape=[2, 3], dtype="float32", need_clip=False
            )
        )
        y = (
            base.default_main_program()
            .global_block()
            .create_parameter(
                name="y", shape=[2, 3], dtype="float32", need_clip=False
            )
        )
        # (x, None) should not be returned
        params_grads = [(x, None), (x, y)]
        params_grads = clip(params_grads)
        self.assertTrue(
            len(clip(params_grads)) == 1,
            "ClipGradByNorm: when grad is None, it shouldn't be returned by gradient clip!",
        )
        self.assertTrue(
            params_grads[0][1].name == 'y',
            "ClipGradByNorm: grad should not be clipped when filtered out!",
        )

@MarioLulab
Copy link
Contributor

nice work~ 但 ClipGradByNorm 的 pir 单测还需要适配 test_none_grad 这个单测,否则 ci 覆盖率不达标 image
@0x45f 帮忙看一下这个问题:pir.Value 没有原来的 "need_clip" 属性,导致 pir 的单测与旧ir单测无法对齐,ci 覆盖率不达标,是否需要删除 _pir_clip 函数中对 need_clip 属性的判断?

这里可以尝试在python/paddle/pir/core.py文件create_parameter函数最后return之前添加下面两行代码:

need_clip = kwargs.get('need_clip', True)
setattr(param, 'need_clip', need_clip)

添加python/paddle/pir/core.py文件代码之后,还需要在TestPirGradientClipByNorm的测试中添加test_none_grad单侧么

 def test_none_grad(self):
        clip = paddle.nn.ClipGradByNorm(self.clip_norm)
        x = (
            base.default_main_program()
            .global_block()
            .create_parameter(
                name="x", shape=[2, 3], dtype="float32", need_clip=False
            )
        )
        y = (
            base.default_main_program()
            .global_block()
            .create_parameter(
                name="y", shape=[2, 3], dtype="float32", need_clip=False
            )
        )
        # (x, None) should not be returned
        params_grads = [(x, None), (x, y)]
        params_grads = clip(params_grads)
        self.assertTrue(
            len(clip(params_grads)) == 1,
            "ClipGradByNorm: when grad is None, it shouldn't be returned by gradient clip!",
        )
        self.assertTrue(
            params_grads[0][1].name == 'y',
            "ClipGradByNorm: grad should not be clipped when filtered out!",
        )

在修改了 core.py 后,还需要在 TestPirGradientClipByNorm 的测试中添加 test_none_grad 单测。
原有的:

x = (
    base.default_main_program()
    .global_block()
    .create_parameter(
        name="x", shape=[2, 3], dtype="float32", need_clip=False
    )
)

可以改成:

with paddle.pir_utils.IrGuard():
    main = paddle.static.Program()
    startup = paddle.static.Program()
    with paddle.static.program_guard(main, startup):
        x = paddle.pir.core.create_parameter(
            dtype="float32",
            shape=[2, 3],
            name="x",
            initializer=paddle.nn.initializer.Constant(value=0.5),
            need_clip=False
        )

@fsczz
Copy link
Contributor Author

fsczz commented Dec 25, 2023

在core.py中添加代码后报错
image
param 信息:
image

注释core.py代码,添加test_none_grad

    def test_none_grad(self):
        clip = paddle.nn.ClipGradByNorm(self.clip_norm)
        with paddle.pir_utils.IrGuard():
            main = paddle.static.Program()
            startup = paddle.static.Program()
            with paddle.static.program_guard(main, startup):
                x = paddle.pir.core.create_parameter(
                    dtype="float32",
                    shape=[2, 3],
                    name="x",
                    initializer=paddle.nn.initializer.Constant(value=0.5),
                    need_clip=False
                )
                y = paddle.pir.core.create_parameter(
                    dtype="float32",
                    shape=[2, 3],
                    name="y",
                    initializer=paddle.nn.initializer.Constant(value=0.3),
                    need_clip=False
                )
                # (x, None) should not be returned
                params_grads = [(x, None), (x, y)]
                params_grads = clip(params_grads)
                self.assertTrue(
                    len(clip(params_grads)) == 1,
                    "ClipGradByNorm: when grad is None, it shouldn't be returned by gradient clip!",
                )

                self.assertTrue(
                    params_grads[0][1].name == 'y',
                    "ClipGradByNorm: grad should not be clipped when filtered out!",
                )

显示没有name信息,
image

@MarioLulab
Copy link
Contributor

MarioLulab commented Dec 26, 2023

关于给 pybind 导出的 C++ 对象设置动态属性的方法,可以参考:#60345
该 pr 可以解决当前使用 ↓ 遇到的问题

setattr(param, 'need_clip', need_clip)

可以等该 pr #60345 合入后再推进此 pr

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.

辛苦 merge 一下最新分支,然后可以尝试在python/paddle/pir/core.py文件create_parameter函数参考 #60345 对 regulizer 属性的添加方法,添加下面两行代码:

need_clip = kwargs.get('need_clip', True)
param.need_clip = need_clip

for p, g in params_grads:
if g is None:
continue
if getattr(p, 'stop_gradient', True) is True:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if getattr(p, 'stop_gradient', True) is True:
if getattr(p, 'need_clip', True) is False:

这行要保持和旧ir静态图的行为一致,都是去检查 need_clip 属性

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