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 6th No.11】Add log_normal and log_normal_ API to Paddle -part #64552

Merged
merged 15 commits into from
Jun 11, 2024

Conversation

NKNaN
Copy link
Contributor

@NKNaN NKNaN commented May 23, 2024

PR Category

User Experience

PR Types

New features

Description

Add paddle.log_normal and paddle.log_normal_ API
rfc: https://github.com/PaddlePaddle/community/blob/master/rfcs/APIs/20230914_api_design_for_bernoulli_%26log_normal.md

Copy link

paddle-bot bot commented May 23, 2024

你的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 the contributor External developers label May 23, 2024
@NKNaN
Copy link
Contributor Author

NKNaN commented May 23, 2024

paddle.log_normal的参数中是否需要dtype参数?如果mean或std可以是tensor的话,输出tensor的dtype和mean或std保持一致就可以吧,就和paddle.normal一样

@zhwesky2010
Copy link
Contributor

zhwesky2010 commented May 27, 2024

paddle.log_normal的参数中是否需要dtype参数?如果mean或std可以是tensor的话,输出tensor的dtype和mean或std保持一致就可以吧,就和paddle.normal一样

按paddle.noraml的形式来吧,组合时形式简单一点,尽可能复用代码

out = standard_normal(paddle.shape(std), std.dtype, name)
else:
log_output = gaussian(shape=shape, mean=mean, std=std, name=name)
return paddle.exp(log_output)
Copy link
Contributor

Choose a reason for hiding this comment

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

是不是直接在paddle.normal的基础上组合,会简单很多,不必要重写这么多内容

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好的

@NKNaN
Copy link
Contributor Author

NKNaN commented May 28, 2024

按paddle.noraml的形式来吧,组合时形式简单一点,尽可能复用代码

已修改

Tensor(shape=[3], dtype=float32, place=Place(cpu), stop_gradient=True,
[10.31321430, 8.97369766 , 35.76752090])
"""
if not in_dynamic_mode():
Copy link
Contributor

Choose a reason for hiding this comment

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

check的这些内容,是不是应该在paddle.normal中就应该有了,所以无需重复check一遍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好的

@@ -748,6 +748,7 @@ if(WITH_DISTRIBUTE)
endif()

# setting timeout value as 15S
set_tests_properties(test_log_normal PROPERTIES TIMEOUT 500)
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.

因为lognormal的参数如果稍微设置大一点,就需要非常多的重复采样次数,来得到与理论均值方差比较接近的经验均值和方差,目前一对参数设置的是采样100000次。我再调一下参数的设置范围和采样次数吧。耗时控制在多少秒以内比较合适?

Copy link
Contributor

Choose a reason for hiding this comment

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

因为lognormal的参数如果稍微设置大一点,就需要非常多的重复采样次数,来得到与理论均值方差比较接近的经验均值和方差,目前一对参数设置的是采样100000次。我再调一下参数的设置范围和采样次数吧。耗时控制在多少秒以内比较合适?

默认是15s,最好是不要超太远吧,看test_uniform_random_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.

改成30s了。这个是参照 test_normal 写的,test_normal 是设置了 120s。

zhwesky2010
zhwesky2010 previously approved these changes May 28, 2024
Copy link
Contributor

@zhwesky2010 zhwesky2010 left a comment

Choose a reason for hiding this comment

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

LGTM

@zhwesky2010
Copy link
Contributor

@NKNaN 补充一下API中文文档与API映射文档

@NKNaN
Copy link
Contributor Author

NKNaN commented May 29, 2024

@NKNaN 补充一下API中文文档与API映射文档

mean 和 std 的默认值是否需要与 torch 相同,torch 为 mean=1, std=2

另外30s还是有点超时,能否改成60s?

@luotao1
Copy link
Contributor

luotao1 commented May 29, 2024

另外30s还是有点超时,能否改成60s?

可以的

@luotao1 luotao1 added the API label May 30, 2024
self.std = np.random.uniform(0.1, 0.5, [1, 2]).astype('float32')


class TestLogNormalAlias(unittest.TestCase):
Copy link
Contributor

Choose a reason for hiding this comment

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

还可以继续再精简一下 测试case 吗,尽可能交叉 减少重复测试,目前时间还是有点长

@@ -686,6 +688,8 @@
'slice_scatter',
'normal',
'normal_',
'log_normal',
Copy link
Contributor

Choose a reason for hiding this comment

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

最终暴露的API是:

paddle.log_normal
paddle.log_normal_
paddle.Tensor.log_normal_

check一下

@zhwesky2010
Copy link
Contributor

@NKNaN coverage还是超时了,windows挂了

if isinstance(self.std, np.ndarray)
else self.std
)
for i in range(self.repeat_num):
Copy link
Contributor

Choose a reason for hiding this comment

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

这里产生1000个样本,采用的是shape=[1]调用1000次API的方式,为何不直接把shape设置为[1000],不就可以直接产生000个样本吧,耗时还会大幅缩减,甚至可以设置为shape=[10000]

Copy link
Contributor

@zhwesky2010 zhwesky2010 left a comment

Choose a reason for hiding this comment

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

LGTM

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

@jeff41404
Copy link
Contributor

jeff41404 commented Jun 7, 2024

please add link of rfc in description above.

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

@luotao1 luotao1 merged commit 5a53c68 into PaddlePaddle:develop Jun 11, 2024
32 of 33 checks passed
@luotao1 luotao1 changed the title 【Hackathon 6th No.11】Add log_normal and log_normal_ API to Paddle 【Hackathon 6th No.11】Add log_normal and log_normal_ API to Paddle -part Jun 11, 2024
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.

5 participants