【Hackathon 5th No.26】为 Paddle 新增 diagonal_scatter API -part #57879

DanGuge · 2023-10-05T08:30:44Z

PR types

Others

PR changes

Others

Description

为 Paddle 新增 diagonal_scatter API
RFC：PaddlePaddle/community#669
#57262

paddle-bot · 2023-10-05T08:30:48Z

你的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-ci-bot · 2023-10-13T03:07:43Z

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

DanGuge · 2023-11-05T05:40:25Z

@zxcd 辛苦review一下，diagonal_scatter的实现

python/paddle/tensor/manipulation.py

test/legacy_test/test_diagonal_scatter.py

DanGuge · 2023-11-08T15:11:53Z

@zxcd 修改完了，辛苦再review一下

zxcd

LGTM

python/paddle/__init__.py

jeff41404 · 2023-11-09T04:40:52Z

The logic of the diagonal_scatter API and the existing function fill_diagonal_tensor in python/paddle/tensor/manipulation.py are most of the same, whether it can be directly exposed as an alias of fill_diagonal_tensor function, so no need to add additional Python logic? Meanwhile, its underlying operator fill_ Diagonal_ tensor needs to add support for int16 data types

DanGuge · 2023-11-09T11:40:08Z

The logic of the diagonal_scatter API and the existing function fill_diagonal_tensor in python/paddle/tensor/manipulation.py are most of the same, whether it can be directly exposed as an alias of fill_diagonal_tensor function, so no need to add additional Python logic? Meanwhile, its underlying operator fill_ Diagonal_ tensor needs to add support for int16 data types

@jeff41404

There is still difference between fill_diagonal_tensor and diagonal_scatter, the former one is only available in dynamic mode while the latter one supports both dynamic & static mode. And I think maybe we should not change the origin usage of fill_diagonal_tensor, so it may be better to add additional API for diagonal_scatter.
I can submit another PR for int16 data types of fill_diagonal_tensor & add relative changes, e.g. tests and so on.

jeff41404 · 2023-11-13T10:24:36Z

There is still difference between fill_diagonal_tensor and diagonal_scatter, the former one is only available in dynamic mode while the latter one supports both dynamic & static mode. And I think maybe we should not change the origin usage of fill_diagonal_tensor, so it may be better to add additional API for diagonal_scatter.

Adding branch of static mode to the current API fill_diagonal_tensor is the compatibility upgrade and correct approach, which can reduce future maintenance costs, as long as it is clearly described in the rfc document and pass existing test case.

jeff41404 · 2023-11-13T10:27:33Z

2. I can submit another PR for int16 data types of fill_diagonal_tensor & add relative changes, e.g. tests and so on.

OK, you can add link of PR to the description above and add these tasks to rfc

DanGuge · 2023-11-14T17:12:14Z

@jeff41404 cc @zxcd
I added the logic of static mode to fill_diagonal_tensor and simplified diagonal_scatter.
And I added int16 to fill_diagonal_tensor in this PR, please review it.

jeff41404

LGTM

python/paddle/tensor/manipulation.py

…tensor for diagonal_scatter

Co-authored-by: ooo oo <106524776+ooooo-create@users.noreply.github.com>

ooooo-create

其余LGTM @sunzhongkai588

python/paddle/tensor/manipulation.py

Co-authored-by: ooo oo <106524776+ooooo-create@users.noreply.github.com>

ooooo-create

LGTM @sunzhongkai588

sunzhongkai588

LGTM for docs

luotao1 · 2023-11-17T10:04:52Z

hi, @DanGuge

非常感谢你对飞桨框架的贡献，我们正在运营一个PFCC组织，会通过定期分享技术知识与发布开发者主导任务的形式持续为飞桨框架做贡献，详情可见 https://github.com/luotao1 主页说明。
如果你对PFCC有兴趣，请发送邮件至 ext_paddle_oss@baidu.com，我们会邀请你加入~

…ddle#57879) * add diagonal_scatter * add diagonal_scatter_test * add annotations * modify int32/int64 test range * update diagonal_scatter doc * fix: bfloat16 type & test name & LayerHelper position * add diagonal scatter in __all__ list * add int16 in fill_diagonal_tensor & reuse the logic of fill_diagonal_tensor for diagonal_scatter * remove xpu settings * op test get_numeric_gradient does not support int16 * fix * Update python/paddle/tensor/manipulation.py Co-authored-by: ooo oo <106524776+ooooo-create@users.noreply.github.com> * Update python/paddle/tensor/manipulation.py Co-authored-by: ooo oo <106524776+ooooo-create@users.noreply.github.com> * fix review suggestions * Update python/paddle/tensor/manipulation.py Co-authored-by: ooo oo <106524776+ooooo-create@users.noreply.github.com> * fix --------- Co-authored-by: ooo oo <106524776+ooooo-create@users.noreply.github.com>

paddle-bot bot added the contributor External developers label Oct 5, 2023

Ligoml mentioned this pull request Oct 7, 2023

【PaddlePaddle Hackathon 5th】开源贡献个人挑战赛 #57262

Open

luotao1 added the PaddlePaddle Hackathon label Oct 9, 2023

luotao1 assigned luotao1 and zxcd Oct 9, 2023

DanGuge mentioned this pull request Oct 12, 2023

【Hackathon 5th No.26】为 Paddle 新增 diagonal_scatter API PaddlePaddle/community#669

Merged

DanGuge force-pushed the diagonal_scatter branch from bab7de6 to 209bdda Compare November 4, 2023 14:24

DanGuge closed this Nov 4, 2023

DanGuge reopened this Nov 4, 2023

DanGuge mentioned this pull request Nov 5, 2023

【Hackathon 5th No.26】为 Paddle 新增 diagonal_scatter API PaddlePaddle/docs#6289

Merged

zxcd reviewed Nov 7, 2023

View reviewed changes

python/paddle/tensor/manipulation.py Outdated Show resolved Hide resolved

python/paddle/tensor/manipulation.py Outdated Show resolved Hide resolved

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

DanGuge requested a review from zxcd November 8, 2023 02:07

zxcd approved these changes Nov 9, 2023

View reviewed changes

jeff41404 reviewed Nov 9, 2023

View reviewed changes

python/paddle/__init__.py Show resolved Hide resolved

DanGuge requested review from jeff41404 and zxcd November 11, 2023 06:33

jeff41404 previously approved these changes Nov 15, 2023

View reviewed changes

luotao1 assigned sunzhongkai588 Nov 15, 2023

ooooo-create reviewed Nov 16, 2023

View reviewed changes

python/paddle/tensor/manipulation.py Outdated Show resolved Hide resolved

python/paddle/tensor/manipulation.py Outdated Show resolved Hide resolved

python/paddle/tensor/manipulation.py Outdated Show resolved Hide resolved

python/paddle/tensor/manipulation.py Outdated Show resolved Hide resolved

DanGuge dismissed jeff41404’s stale review via 716c294 November 16, 2023 12:50

add diagonal_scatter

4635a73

DanGuge and others added 13 commits November 16, 2023 20:51

add diagonal_scatter_test

707f6e2

add annotations

4252709

modify int32/int64 test range

517a32f

update diagonal_scatter doc

e030023

fix: bfloat16 type & test name & LayerHelper position

ebc3c6f

add diagonal scatter in __all__ list

3629b40

add int16 in fill_diagonal_tensor & reuse the logic of fill_diagonal_…

501f699

…tensor for diagonal_scatter

remove xpu settings

bd48100

op test get_numeric_gradient does not support int16

c19ec30

fix

802e2a8

Update python/paddle/tensor/manipulation.py

8f22f20

Co-authored-by: ooo oo <106524776+ooooo-create@users.noreply.github.com>

Update python/paddle/tensor/manipulation.py

4e7926a

Co-authored-by: ooo oo <106524776+ooooo-create@users.noreply.github.com>

fix review suggestions

bcda589

DanGuge force-pushed the diagonal_scatter branch from 6930362 to bcda589 Compare November 16, 2023 13:03

DanGuge requested review from ooooo-create and jeff41404 November 17, 2023 01:08

ooooo-create reviewed Nov 17, 2023

View reviewed changes

python/paddle/tensor/manipulation.py Show resolved Hide resolved

python/paddle/tensor/manipulation.py Outdated Show resolved Hide resolved

DanGuge and others added 2 commits November 16, 2023 20:07

Update python/paddle/tensor/manipulation.py

58e5966

Co-authored-by: ooo oo <106524776+ooooo-create@users.noreply.github.com>

fix

ae6fba1

DanGuge requested a review from ooooo-create November 17, 2023 02:11

ooooo-create approved these changes Nov 17, 2023

View reviewed changes

sunzhongkai588 approved these changes Nov 17, 2023

View reviewed changes

luotao1 changed the title ~~【Hackathon 5th No.26】为 Paddle 新增 diagonal_scatter API~~ 【Hackathon 5th No.26】为 Paddle 新增 diagonal_scatter API -part Nov 17, 2023

luotao1 merged commit 98bb0b8 into PaddlePaddle:develop Nov 17, 2023
28 checks passed

luotao1 mentioned this pull request Dec 11, 2023

【Hackathon 5th No.26】Add diagonal_scatter API to Paddle #57854

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

【Hackathon 5th No.26】为 Paddle 新增 diagonal_scatter API -part #57879

【Hackathon 5th No.26】为 Paddle 新增 diagonal_scatter API -part #57879

DanGuge commented Oct 5, 2023

paddle-bot bot commented Oct 5, 2023

paddle-ci-bot bot commented Oct 13, 2023

DanGuge commented Nov 5, 2023

DanGuge commented Nov 8, 2023

zxcd left a comment

jeff41404 commented Nov 9, 2023 •

edited

Loading

DanGuge commented Nov 9, 2023 •

edited

Loading

jeff41404 commented Nov 13, 2023 •

edited

Loading

jeff41404 commented Nov 13, 2023

DanGuge commented Nov 14, 2023

jeff41404 left a comment

ooooo-create left a comment

ooooo-create left a comment

sunzhongkai588 left a comment

luotao1 commented Nov 17, 2023

【Hackathon 5th No.26】为 Paddle 新增 diagonal_scatter API -part #57879

【Hackathon 5th No.26】为 Paddle 新增 diagonal_scatter API -part #57879

Conversation

DanGuge commented Oct 5, 2023

PR types

PR changes

Description

paddle-bot bot commented Oct 5, 2023

paddle-ci-bot bot commented Oct 13, 2023

DanGuge commented Nov 5, 2023

DanGuge commented Nov 8, 2023

zxcd left a comment

Choose a reason for hiding this comment

jeff41404 commented Nov 9, 2023 • edited Loading

DanGuge commented Nov 9, 2023 • edited Loading

jeff41404 commented Nov 13, 2023 • edited Loading

jeff41404 commented Nov 13, 2023

DanGuge commented Nov 14, 2023

jeff41404 left a comment

Choose a reason for hiding this comment

ooooo-create left a comment

Choose a reason for hiding this comment

ooooo-create left a comment

Choose a reason for hiding this comment

sunzhongkai588 left a comment

Choose a reason for hiding this comment

luotao1 commented Nov 17, 2023

jeff41404 commented Nov 9, 2023 •

edited

Loading

DanGuge commented Nov 9, 2023 •

edited

Loading

jeff41404 commented Nov 13, 2023 •

edited

Loading