Skip to content

Conversation

NKNaN
Copy link
Contributor

@NKNaN NKNaN commented Mar 26, 2024

Add rfcs for sinc / sinc_ API.

Copy link

paddle-bot bot commented Mar 26, 2024

你的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.


## 底层OP设计

c++接口为 `_C_ops.sinc` 和 `_C_ops.sinc_`
Copy link
Contributor

Choose a reason for hiding this comment

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

该OP能否通过组合算子的方式实现?

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的做法用where来对等于0的元素做替换的话,应该需要临时创建一个和输入一样大小的tensor,这样对于sinc_这个inplace operator而言这一部分又需要占用额外的空间,我想用底层kernel的话可能会省一些空间?

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 的结果对齐;
- 不同 shape;
- 前向计算;
- 计算dtype类型:验证 `float64`,`float32`等;
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.

数据类型支持 float16,float32,float64。(paddle.sin 支持 float16,float32,float64)


不涉及底层OP

## API实现方案
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.

已修改

Copy link
Contributor

@zxcd zxcd left a comment

Choose a reason for hiding this comment

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

LGTM


# 一、概述
## 1、相关背景
[NO.7 为 Paddle 新增 PolynomialLR / sinc / sinc_ API](https://github.com/PaddlePaddle/community/blob/master/hackathon/hackathon_6th/【Hackathon%206th】开源贡献个人挑战赛框架开发任务合集.md#no7-为-paddle-新增-polynomiallr--sinc--sinc_-api)
Copy link
Collaborator

Choose a reason for hiding this comment

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

这处需要修改,没有 PolynomialLR 了

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已修改

x = np.asanyarray(x)
y = pi * where(x == 0, 1.0e-20, x)
return sin(y)/y
```
Copy link
Collaborator

Choose a reason for hiding this comment

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

image 这里格式需要修改

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已修改

@luotao1 luotao1 merged commit b8bd96c into PaddlePaddle:master Apr 16, 2024
@NKNaN NKNaN deleted the lrsinc branch August 27, 2024 11:01
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.

3 participants