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.20】为 Paddle 新增 cummax API #39

Merged
merged 9 commits into from
Apr 1, 2022
Merged

【Hackathon No.20】为 Paddle 新增 cummax API #39

merged 9 commits into from
Apr 1, 2022

Conversation

jinyouzhi
Copy link
Contributor

No description provided.

@jinyouzhi jinyouzhi mentioned this pull request Mar 16, 2022
Closed
@paddle-bot-old
Copy link

Thanks for your contribution!

@CLAassistant
Copy link

CLAassistant commented Mar 16, 2022 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@dingjiaweiww
Copy link
Contributor

你的 PR 提交成功，感谢你对于开源项目的贡献，请检查 PR 提交格式和内容是否完备，具体请参考示例和模版。

@dingjiaweiww
Copy link
Contributor

PR 格式检查通过，你的PR 将接受Paddle 专家以及开源社区的review，请及时关注PR 动态

DDDivano
DDDivano reviewed Mar 18, 2022
View reviewed changes
rfcs/APIs/20220316_api_design_for_cummax.md Outdated
测试考虑的case如下：

- 动态图，静态图，与 PyTorch 的结果保持一致；
- NaN 处理：关注异常值，边界情况的处理；
Copy link

Choose a reason for hiding this comment

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

请详细说明测试用例

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已增补

@paddle-bot-old
Copy link

你的PR有最新反馈，请及时修改。
There’s the latest feedback about your PR. Please check.

yingyibiao
yingyibiao reviewed Mar 28, 2022
View reviewed changes
rfcs/APIs/20220316_api_design_for_cummax.md
Comment on lines +33 to +35
# 三、业内方案调研

## PyTorch
Copy link
Contributor

@yingyibiao yingyibiao Mar 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

可以提供更多框架（如 numpy / tf 等）方案的调研，以及目前 paddle 内是否可以通过组合方式实现。

Copy link
Contributor Author

@jinyouzhi jinyouzhi Mar 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

谢谢，很有启发性

Copy link
Contributor Author

Choose a reason for hiding this comment

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

调研了NumPy，以组合方式实现（np.maximum.accumulate()）。Paddle内没有提供accumulate方法，并且考虑对齐 cumsum、cumprod，提供原生实现为好。

yingyibiao
yingyibiao reviewed Mar 28, 2022
View reviewed changes
rfcs/APIs/20220316_api_design_for_cummax.md Outdated
测试考虑的case如下：

- 动态图，静态图，与 PyTorch 的结果保持一致；
- NaN 处理：关注异常值，边界情况的处理；
Copy link
Contributor

Choose a reason for hiding this comment

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

这里的异常值处理设计方案具体是什么呢？

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已增补在测试用例说明，采取同NumPy一致

@yingyibiao
Copy link
Contributor

LGTM

@yingyibiao yingyibiao merged commit df17275 into PaddlePaddle:master Apr 1, 2022
@Ligoml Ligoml mentioned this pull request Jul 27, 2022
Closed
@Ligoml Ligoml changed the title 【Hackathon No.20】 【Hackathon No.20】为 Paddle 新增 cummax API Jul 27, 2022
@Ligoml Ligoml mentioned this pull request Nov 15, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yingyibiao yingyibiao yingyibiao approved these changes

@DDDivano DDDivano DDDivano approved these changes

Assignees

@DDDivano DDDivano

@yingyibiao yingyibiao

Labels
contributor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jinyouzhi @CLAassistant @dingjiaweiww @yingyibiao @DDDivano @yang131313