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

【PaddlePaddle Hackathon 第四期】No.6:为 Paddle 新增 ldexp API #51395

Merged
merged 26 commits into from
Jun 7, 2023

Conversation

longranger2
Copy link
Contributor

@longranger2 longranger2 commented Mar 9, 2023

PR types

New features

PR changes

APIs

Description

为 Paddle 新增 ldexp API

Details

Tracking Issue: #51281
RFC: PaddlePaddle/community#407
CN-Docs: PaddlePaddle/docs#5925

@paddle-bot
Copy link

paddle-bot bot commented Mar 9, 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 Mar 9, 2023
@longranger2 longranger2 changed the title 【PaddlePaddle Hackathon 第四期】No1:为 Paddle 新增 ldexp API 【PaddlePaddle Hackathon 第四期】No6:为 Paddle 新增 ldexp API Mar 9, 2023
@luotao1 luotao1 added the API label Mar 10, 2023
@longranger2 longranger2 changed the title 【PaddlePaddle Hackathon 第四期】No6:为 Paddle 新增 ldexp API 【PaddlePaddle Hackathon 第四期】No.6:为 Paddle 新增 ldexp API Mar 13, 2023
@Ligoml Ligoml requested a review from luotao1 March 14, 2023 02:40
@luotao1
Copy link
Contributor

luotao1 commented Mar 29, 2023

请解决下冲突

@paddle-ci-bot
Copy link

paddle-ci-bot bot commented Apr 24, 2023

Sorry to inform you that d182aa0's CIs have passed for more than 7 days. To prevent PR conflicts, you need to re-run all CIs manually.

@luotao1
Copy link
Contributor

luotao1 commented Jun 2, 2023

你可以看下,torch是不是不管x输入是int还是float,y都是float?#51395 (comment) 如果是这样的话,需要改RFC文档
是的,不管输入的是float还是int,输出都是float32,我去修改下RFC文档

>>> a = torch.ones([2], dtype=torch.float64)
>>> torch.ldexp(a, torch.tensor([-2])).dtype
torch.float64

输出是float32或float64,不是只有float32,RFC也需要对应修改,放上例子

@luotao1
Copy link
Contributor

luotao1 commented Jun 2, 2023

单测中需要加一下输出类型的检查

@longranger2
Copy link
Contributor Author

你可以看下,torch是不是不管x输入是int还是float,y都是float?#51395 (comment) 如果是这样的话,需要改RFC文档
是的,不管输入的是float还是int,输出都是float32,我去修改下RFC文档

>>> a = torch.ones([2], dtype=torch.float64)
>>> torch.ldexp(a, torch.tensor([-2])).dtype
torch.float64

输出是float32或float64,不是只有float32,RFC也需要对应修改,放上例子

好的👌

单测中需要加一下输出类型的检查

好的👌

@longranger2
Copy link
Contributor Author

longranger2 commented Jun 2, 2023

还有一个问题:paddle.ldexp是否需要与torch.ldexp进行对齐呢?

  1. torch.ldexp(input, other)要求输入的两个参数都是要求tensor,这样的话,就需要修改下paddle.ldexp(x,y)中y的输入类型,也设置为tensor
image

@longranger2 longranger2 requested a review from luotao1 June 2, 2023 07:57
@luotao1
Copy link
Contributor

luotao1 commented Jun 2, 2023

paddle.ldexp(x,y)中y的输入类型,也设置为tensor

好的,建议对齐

@longranger2
Copy link
Contributor Author

同时提一个paddle.pow 在负数输入时和torch不一致的issue吧 #51395 (comment)

已提交Issue:#54314

luotao1
luotao1 previously approved these changes Jun 3, 2023
Copy link
Contributor

@luotao1 luotao1 left a comment

Choose a reason for hiding this comment

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

LGTM

x = paddle.to_tensor([1, 2, 3], dtype='float32')
y = paddle.to_tensor([2, 3, 4], dtype='int32')
res = paddle.ldexp(x, y)
print(res)
Copy link
Contributor

Choose a reason for hiding this comment

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

示例可以多举一个。比如 y = paddle.to_tensor([2],这样有broadcast的例子了

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已经添加好了~

if x.dtype == paddle.float64 or y.dtype == paddle.float64:
out_dtype = paddle.float64
else:
out_dtype = paddle.float32
Copy link
Contributor

Choose a reason for hiding this comment

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

it is better to use out_dtype = paddle.get_default_dtype() ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@luotao1
Copy link
Contributor

luotao1 commented Jun 5, 2023

请解决下冲突

@longranger2
Copy link
Contributor Author

@luotao1 @jeff41404 已经修改好了,辛苦review下~

jeff41404
jeff41404 previously approved these changes Jun 6, 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

add note for code description

Co-authored-by: zachary sun <70642955+sunzhongkai588@users.noreply.github.com>
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 for docs,请提供中文文档~

@luotao1 luotao1 merged commit cb2476c into PaddlePaddle:develop Jun 7, 2023
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.

None yet

5 participants