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

[rfc] modify cummax / cummin API design #531

Merged
merged 1 commit into from
Jun 13, 2023
Merged

[rfc] modify cummax / cummin API design #531

merged 1 commit into from
Jun 13, 2023

Conversation

Patrick-Star125
Copy link
Contributor

此pr用于标识算子最新的设计,可能用于算子PR review,后续应该还有更改,暂时不用合入

相关Link:

@paddle-bot
Copy link

paddle-bot bot commented May 7, 2023

你的PR提交成功,感谢你对开源项目的贡献!
请检查PR提交格式和内容是否完备,具体请参考示例模版
Your PR has been submitted. Thanks for your contribution!
Please check its format and content. For this, you can refer to Template and Demo.

@Patrick-Star125 Patrick-Star125 changed the title 【Hackathon 4 No.17】modify cummax / cummin API design [rfc] modify cummax / cummin API design May 7, 2023
@@ -300,9 +300,8 @@ gpu:
template <typename T, typename Context>
void CummaxKernel(const Context& dev_ctx,
const DenseTensor& x,
const Scalar& axis,
DataType dtype,
bool flatten,
Copy link
Contributor

Choose a reason for hiding this comment

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

这里为什么去掉了flatten呢?cummin同

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cumsum的flatten用于反向算子执行前调整形状,而cummax/cummin的反向不依赖flatten,所以就去掉了

Copy link
Contributor

Choose a reason for hiding this comment

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

嗯,我看见你把flatten操作在python层做了,解决一下代码pr的 windows ci问题吧

Copy link
Contributor

@GGBond8488 GGBond8488 left a comment

Choose a reason for hiding this comment

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

LGTM

@luotao1 luotao1 merged commit 795348b into PaddlePaddle:master Jun 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants