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

【PaddlePaddle Hackathon 2】29、为 Paddle 新增 PixelUnshuffle 组网 API #40728

Merged
merged 40 commits into from Apr 26, 2022

Conversation

BrilliantYuKaimin
Copy link
Contributor

@BrilliantYuKaimin BrilliantYuKaimin commented Mar 19, 2022

PR types

New features

PR changes

APIs

Describe

完成了Issue:#40304
实现了paddle.nn.PixelUnshuffle和paddle.nn.functional.pixel_unshuffle,其行为是将形为[N,C,H,W]重塑成[N,C,H/r,r,W/r,r]的形状,再转置成[N,C,r,r,H/r,W/r]的形状,最后重塑成[N,Crr,H/r,W/r]的形状。
设计文档:PaddlePaddle/community#60
中文文档:PaddlePaddle/docs#4523

@paddle-bot-old paddle-bot-old bot added contributor External developers status: proposed labels Mar 19, 2022
@paddle-bot-old
Copy link

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

@CLAassistant
Copy link

CLAassistant commented Mar 19, 2022

CLA assistant check
All committers have signed the CLA.

@paddle-bot-old
Copy link

PR格式检查通过,你的PR将接受Paddle专家以及开源社区的review,请及时关注PR动态。
The format inspection passed. Your PR will be reviewed by experts of Paddle and developers from the open-source community. Stay tuned.

@BrilliantYuKaimin
Copy link
Contributor Author

unary.h中有一行的格式需要调整,删除不必要的空格:
image

完成

michaelowenliu
michaelowenliu previously approved these changes Apr 15, 2022
@@ -0,0 +1,130 @@
/*Copyright (c) 2022 PaddlePaddle Authors. All Rights Reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

license格式有点问题,参考下其他头文件,增加下缩进和换行

Copy link
Contributor

Choose a reason for hiding this comment

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

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

完成

public:
using framework::OperatorWithKernel::OperatorWithKernel;

void InferShape(framework::InferShapeContext* ctx) const override {
Copy link
Contributor

Choose a reason for hiding this comment

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

这个建议放到infermeta/backward.h中哈,其他的还有是因为正在迁移中,历史算子量比较大,我们还在推进中,比如可以参考softmax_op.cc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

完成

@@ -0,0 +1,74 @@
// Copyright (c) 2022 PaddlePaddle Authors. All Rights Reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

这个文件需要放到kernels/impl目录中哈,命名可以参考impl目录中的文件,然后kernel.cc和kernel.cu分别在cpu和gpu下,kernels根目录下的cc中是设备无关的kernel实现,即这个kernel的实现里,不能有设备相关的函数,而这个kernel里的Transpose是仅支持CPU和GPU的,我们还有XPU以及将来可能有其他设备

Copy link
Contributor Author

Choose a reason for hiding this comment

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

完成

namespace phi {

template <typename T, typename Context>
void PixelUnshuffleGradKernel(const Context& ctx,
Copy link
Contributor

Choose a reason for hiding this comment

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

参数名建议统一为dev_ctx

Copy link
Contributor Author

Choose a reason for hiding this comment

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

完成

@@ -0,0 +1,73 @@
// Copyright (c) 2022 PaddlePaddle Authors. All Rights Reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

同上,该Kernel属于仅支持CPU和GPU的实现,并非设备无关的实现,麻烦移到impl中

Copy link
Contributor Author

Choose a reason for hiding this comment

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

完成


namespace phi {

KernelSignature PixelUnshuffleOpArgumentMapping(
Copy link
Contributor

Choose a reason for hiding this comment

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

前向的ArgumentMapping实现和注册建议移除,会自动解析OpMaker的参数生成的

Copy link
Contributor Author

Choose a reason for hiding this comment

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

完成

@BrilliantYuKaimin
Copy link
Contributor Author

BrilliantYuKaimin commented Apr 21, 2022

研发老师可以帮忙看一下 PR-CI-Kunlun-KP-Build 不通过的原因吗?

chenwhql
chenwhql previously approved these changes Apr 24, 2022
XiaoguangHu01
XiaoguangHu01 previously approved these changes Apr 24, 2022
Copy link
Contributor

@XiaoguangHu01 XiaoguangHu01 left a comment

Choose a reason for hiding this comment

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

LGTM

dingjiaweiww
dingjiaweiww previously approved these changes Apr 24, 2022
@shiyutang
Copy link
Contributor

@BrilliantYuKaimin 需要拉取下最新分支解决下冲突,解决后就可以准备合入了~

@BrilliantYuKaimin
Copy link
Contributor Author

@BrilliantYuKaimin 需要拉取下最新分支解决下冲突,解决后就可以准备合入了~

完成

Copy link
Contributor

@XiaoguangHu01 XiaoguangHu01 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@TCChenlong TCChenlong left a comment

Choose a reason for hiding this comment

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

LGTM for API docs

@jeff41404 jeff41404 merged commit 5be9b82 into PaddlePaddle:develop Apr 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor External developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet