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

【Hackathon No.39】为 Paddle 新增 LPPool1D / LPPool2D API #58433

Closed
wants to merge 37 commits into from
Closed

【Hackathon No.39】为 Paddle 新增 LPPool1D / LPPool2D API #58433

wants to merge 37 commits into from

Conversation

WintersMontagne10335
Copy link
Contributor

@WintersMontagne10335 WintersMontagne10335 commented Oct 26, 2023 •
edited
Loading

PR types

Others

PR changes

APIs

Description

为 Paddle 新增 LPPool1D / LPPool2D API

Related links

Current progress

  • LPPool2D forward
  • LPPool2D backward
  • LPPool1D forward
  • LPPool1D backward

@paddle-bot
Copy link

paddle-bot bot commented Oct 26, 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 Oct 26, 2023
@luotao1 luotao1 mentioned this pull request Oct 29, 2023
Closed
@WintersMontagne10335
Copy link
Contributor Author

@cyber-pioneer 老师您好，现在CI中没通过的部分，主要是因为改变文件超过20个、API改变和单测测试用例较少。请问如果您有时间的话，可以先看一下代码吗？

目前基本情况是：

  1. 2d 的算子能跑通，可以通过单测；
  2. 因为借鉴的AvgPool2d、MaxPool2d实现得LPPool2d，但是相比于它们多了norm_type入参，不能完全兼容，所以存在有冗余入参的问题，后续会修正；

想求教的问题：

  1. 1d实现要不要分出来，另外提一个PR？1d依赖2d底层；
  2. 目前代码基本结构需要做调整？LPPool2d有一部分代码，是复制了一份的AvgPool2d相关的代码，然后做了修改，这样可以吗？
  3. 现在反向计算是不是不对？

cyber-pioneer
cyber-pioneer reviewed Nov 7, 2023
View reviewed changes
paddle/phi/kernels/impl/pool_grad_kernel_impl.h Outdated

template <typename T, typename Context>
void PoolGradRawKernel(const Context& ctx,
const DenseTensor& x,
Copy link
Contributor

Choose a reason for hiding this comment

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

这个kernel是哪里用的？

Copy link
Contributor Author

Choose a reason for hiding this comment

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

没有用，已删除~~

@cyber-pioneer
Copy link
Contributor

工作做得很棒，由于涉及到kernel新增，我这边需要将pr拉下来验证。

@paddle-ci-bot Paddle-ci-bot
Copy link

paddle-ci-bot bot commented Nov 11, 2023

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

@cyber-pioneer
Copy link
Contributor

新增的算子注册建议写在ops.yaml 和backward.yaml内，legacy_ops.yaml和 static_ops.yaml是动态和静态模式算子不统一时才涉及到。

@WintersMontagne10335
Copy link
Contributor Author

新增的算子注册建议写在ops.yaml 和backward.yaml内，legacy_ops.yaml和 static_ops.yaml是动态和静态模式算子不统一时才涉及到。

收到！合入最新代码之后出问题了，我这两天再调一下。

@WintersMontagne10335
Copy link
Contributor Author

新增的算子注册建议写在ops.yaml 和backward.yaml内，legacy_ops.yaml和 static_ops.yaml是动态和静态模式算子不统一时才涉及到。

Done。
我感觉反向算子的计算可能有问题，下周我验证一下，并补充完整单测用例。如果反向计算没问题的话，我12.04前做完LPPool2D，并给您回复。

@WintersMontagne10335 WintersMontagne10335 mentioned this pull request Dec 3, 2023
Closed
@WintersMontagne10335
Copy link
Contributor Author

@cyber-pioneer 静态图单测存在严重问题，推进很不顺利，估计12.04前做不完了。我会先把动态图单测、文档和LPPool1d补充完。静态图的问题估计需要一段较长的时间来解决。

@cyber-pioneer
Copy link
Contributor

nice job

@cyber-pioneer 静态图单测存在严重问题，推进很不顺利，估计12.04前做不完了。我会先把动态图单测、文档和LPPool1d补充完。静态图的问题估计需要一段较长的时间来解决。

没关系，这个算子难度比较大，工作量挺多，辛苦啦

@cyber-pioneer
Copy link
Contributor

image 这个分支似乎没覆盖到

@WintersMontagne10335
Copy link
Contributor Author

image 这个分支似乎没覆盖到

因为本地测试时，这个分支有问题，所以没有push上来~~

@WintersMontagne10335
Copy link
Contributor Author

image 这个分支似乎没覆盖到

最新代码push上来啦

@paddle-ci-bot Paddle-ci-bot
Copy link

paddle-ci-bot bot commented Dec 29, 2023

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

@luotao1 luotao1 closed this Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cyber-pioneer cyber-pioneer cyber-pioneer left review comments

Assignees

@luotao1 luotao1

@cyber-pioneer cyber-pioneer

Labels
contributor External developers PaddlePaddle Hackathon
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@WintersMontagne10335 @cyber-pioneer @luotao1