【Hackathon No.15】为 Paddle 新增 count_nonzero #167
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
luotao1
reviewed
Jul 7, 2022
| # 二、飞桨现状 | ||
| 目前paddle缺少相关功能实现。 | ||
|
|
||
| API方面，已有相关功能的API，[paddle.nansum](https://github.com/PaddlePaddle/Paddle/blob/develop/python/paddle/tensor/math.py#L910), 由于容易实现，所以在Paddle中是一个由其他API组合成的API，没有实现自己的OP，其主要实现逻辑为： |
There was a problem hiding this comment.
21行和23行不是很通顺，需要再组织下：
- “目前paddle缺少相关功能实现”，和“所以在Paddle中是一个由其他API组合成的API，没有实现自己的OP”，不能对应。是否可以明确为：可以由其他API组合成的API，不需要实现自己的OP？
- 这里引入
nansum是为什么呢？是因为实现方式类似？
| # 六、测试和验收的考量 | ||
| 测试考虑的case如下： | ||
|
|
||
| - 和numpy结果的数值的一致性, `paddle.nanmean`,和`np.nanmean`结果是否一致； |
There was a problem hiding this comment.
``paddle.nanmean,和np.nanmean`结果是否一致`这句是忘记删了？
luotao1
reviewed
Jul 7, 2022
| - 参数`axis`为int,tuple和list时输出的正确性； | ||
| - `keepdim`参数的正确性； | ||
| - 未输入维度时的输出正确性； | ||
| - 测试在进行反向梯度计算时结果的正确性(包含nan值和非nan值位置的梯度)； |
There was a problem hiding this comment.
(包含nan值和非nan值位置的梯度)的原因是什么呢？
luotao1
approved these changes
Jul 7, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
为 Paddle 新增 count_nonzero