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

【Hackathon No.18】 新增 heaviside API 设计文档 #57

Merged
merged 2 commits into from
Mar 25, 2022

Conversation

BrilliantYuKaimin
Copy link
Contributor

增加了paddle.heaviside设计文档。

@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.

Copy link
Contributor

@zoooo0820 zoooo0820 left a comment

Choose a reason for hiding this comment

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

整体调研情况全面细致,仅个别细节需要细化一下


# 五、设计思路与实现方案

- 向前计算设计为`x == 0 ? y : static_cast<T>(x > 0) `;
Copy link
Contributor

Choose a reason for hiding this comment

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

由于该API的输入涉及较多与广播机制相关的内容,这里建议补充一下我们期望的API行为:例如x, y为标量/张量时,shape一致/不一致时等情况。

此外,这里有个小的语病,向前->前向

Copy link
Contributor Author

Choose a reason for hiding this comment

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

我在后面补充说了这个API是逐元素计算的,那么广播时的行为就不言而喻了。

语病:修改完成。


## 命名与参数设计

API设计为`heaviside(x, y, name=None)`,它支持广播,参数为
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. API设计上给出调用全路径吧,如paddle.heaviside(x, y)这样;另外可以补充下Paddle.Tensor.heaviside这个调用路径。

  2. 这里参数的命名想再讨论下。从Pytorch的设计来看,使用了inputvalues的命名。在Paddle中,输入用x是没问题的,但使用y会显得和x是对等关系(例如paddle.lerp, paddle.matmul等双输入的API)。 从这个函数的功能上来看(类似paddle.Tensor.fill_, paddle.full等API),Pytorch的命名为values似乎更贴切。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. 完成

  2. 我选择使用xy是因为我倾向于在这里把heaviside视为一个二元函数,这样就有理由为xy都设计偏导数。另外,从功能上来看,我认为它和paddle.maximum更类似,都是逐元素计算的算子。

API设计为`heaviside(x, y, name=None)`,它支持广播,参数为

- x (Tensor)- 输入的Tensor。数据类型为 float32、 float64、int32或 int64;
- y (Tensor)- 输入的Tensor。数据类型为 float32、 float64 、int32或int64;
Copy link
Contributor

Choose a reason for hiding this comment

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

numpy是支持xyscalar形式输入的,从下面的用例看该设计上也是可以支持的。这里参数类型需要更正下。

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确实支持标量输入,pytorch是不支持的。在飞桨中,其他逐元素计算的算子(比如paddle.maximum)均不支持标量输入。后文的用例设计确实是我原先写错了。

- 与Numpy对比计算结果的一致性:
- x和y的形状一样时,
- x和y中有一个是标量时,
- 广播机制测试;
Copy link
Contributor

Choose a reason for hiding this comment

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

同前面关于预期行为的说明,测试用例能否细化下各种情况(标量/张量,shape一致/不一致等)。以及异常检查具体会包括哪些用例?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

完成

Copy link
Contributor

@zoooo0820 zoooo0820 left a comment

Choose a reason for hiding this comment

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

LGTM

@zoooo0820 zoooo0820 merged commit b01bdaa into PaddlePaddle:master Mar 25, 2022
@paddle-bot-old
Copy link

你的PR已合入community库,请进行后续代码开发,并将代码提交至Paddle仓库。
Your PR has been merged into community repository. Please move on coding part and submit your code to corresponding repo.

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