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 4 No.14】Add Polar to paddle #50901

Merged
merged 15 commits into from
Mar 17, 2023

Conversation

PommesPeter
Copy link
Contributor

@PommesPeter PommesPeter commented Feb 25, 2023

PR types

New features

PR changes

APIs

Describe

added polar API to paddle

rfc doc here: PaddlePaddle/community#364
Chinese doc here: PaddlePaddle/docs#5639

@paddle-bot
Copy link

paddle-bot bot commented Feb 25, 2023

你的PR提交成功,感谢你对开源项目的贡献!
请关注后续CI自动化测试结果,详情请参考Paddle-CI手册
Your PR has been submitted. Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

@paddle-bot paddle-bot bot added contributor External developers status: proposed labels Feb 25, 2023
@PommesPeter
Copy link
Contributor Author

文档链接 PaddlePaddle/docs#5639

@CLAassistant
Copy link

CLAassistant commented Mar 2, 2023

CLA assistant check
All committers have signed the CLA.

@PommesPeter
Copy link
Contributor Author

PommesPeter commented Mar 3, 2023

我观察 CI 发现 paddle.multiply API 在计算 float64 和 complex128 做乘法的时候报错了,CI 中是编译的 develop 版本,但是我用 release 的 windows 版本的 paddlepaddle 2.3.2 在本地是可以执行这个操作的,这个报错是正常的么? @iclementine
image

@iclementine
Copy link

我观察 CI 发现 paddle.multiply API 在计算 float64 和 complex128 做乘法的时候报错了,CI 中是编译的 develop 版本,但是我用 release 的 windows 版本的 paddlepaddle 2.3.2 在本地是可以执行这个操作的,这个报错是正常的么? @iclementine
image

在静态图运行模式下,有这样严格的类型限制,应该是设计如此的。所以还是加一个 cast 吧。

@PommesPeter
Copy link
Contributor Author

PommesPeter commented Mar 7, 2023

我观察 CI 发现 paddle.multiply API 在计算 float64 和 complex128 做乘法的时候报错了,CI 中是编译的 develop 版本,但是我用 release 的 windows 版本的 paddlepaddle 2.3.2 在本地是可以执行这个操作的,这个报错是正常的么? @iclementine
image

在静态图运行模式下,有这样严格的类型限制,应该是设计如此的。所以还是加一个 cast 吧。

cast 只能在 ['bool', 'float16', 'float32', 'float64', 'int8', 'int16', 'int32', 'int64', 'uint8', 'uint16'] 之间转换,是否可将代码优化成如下方式:

def polar(abs, angle, name=None):
    """
    .....
    """
    check_variable_and_dtype(abs, 'abs', ['float32', 'float64'], 'paddle.polar')
    check_variable_and_dtype(
        angle, 'angle', ['float32', 'float64'], 'paddle.polar'
    )
    return paddle.complex(abs * paddle.cos(angle), abs * paddle.sin(angle))

按照如此可避免 dtype 不相同的问题,并且保证类型严格一致,float to complex 的类型变换只会在 paddle.complex 上变换。

@PommesPeter
Copy link
Contributor Author

代码已修改,辛苦 review。 @iclementine

iclementine
iclementine previously approved these changes Mar 9, 2023
@jeff41404
Copy link
Contributor

rfc document link and Chinese document link need to be given in description.

@PommesPeter
Copy link
Contributor Author

rfc document link and Chinese document link need to be given in description.

okay, I have updated doc links in the description.

@PommesPeter
Copy link
Contributor Author

rfc doc here: PaddlePaddle/community#364
Chinese doc here: PaddlePaddle/docs#5639

jeff41404
jeff41404 previously approved these changes Mar 14, 2023
Copy link
Contributor

@jeff41404 jeff41404 left a comment

Choose a reason for hiding this comment

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

LGTM

sunzhongkai588
sunzhongkai588 previously approved these changes Mar 14, 2023
Copy link
Contributor

@sunzhongkai588 sunzhongkai588 left a comment

Choose a reason for hiding this comment

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

LGTM

@PommesPeter PommesPeter changed the title 【Hackthon 4 No.14】Add Polar to paddle 【Hackathon 4 No.14】Add Polar to paddle Mar 16, 2023
@luotao1 luotao1 merged commit eefe601 into PaddlePaddle:develop Mar 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API contributor External developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants