Skip to content

【Hackathon 6th No.5】Add chi2/LKJCholesky API to Paddle -part #63883

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 57 commits into from
Jun 12, 2024
Merged

【Hackathon 6th No.5】Add chi2/LKJCholesky API to Paddle -part #63883

merged 57 commits into from
Jun 12, 2024

Conversation

@cmcamdy
Copy link
Contributor

@cmcamdy cmcamdy commented Apr 25, 2024 •
edited
Loading

PR Category

Others

PR Types

Others

Description

需要为飞桨扩充 API paddle.distribution.chi2 和 paddle.distribution.LKJCholesky
1.实现分布chi2
2.实现分布LKJCholesky,支持sample_method参数,onion/cvine可选
3.两个单测的静态图模式同等规模下可能超时，因此手动改了个cmake超时设定，如下所示：

  • set_tests_properties(test_distribution_lkj_cholesky_static PROPERTIES TIMEOUT 120)
  • 该测试超时主要是因为TestLKJCholeskyLogProb这个测试单项在验证计算的时候耗时太大，目前修改超时时间为120s

4.RFC-link:【Hackathon 6th No.5】为 Paddle 新增 chi2/LKJCholesky API #872
@paddle-bot
Copy link

paddle-bot bot commented Apr 25, 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 Apr 25, 2024
@luotao1 luotao1 mentioned this pull request Apr 28, 2024
Closed
@paddle-ci-bot
Copy link

paddle-ci-bot bot commented May 4, 2024

Sorry to inform you that 1269701's CIs have passed for more than 7 days. To prevent PR conflicts, you need to re-run all CIs manually.
@xuxinyi389
Copy link
Contributor

xuxinyi389 commented May 29, 2024

LGTM
@cmcamdy
Copy link
Contributor Author

cmcamdy commented May 31, 2024

已经为chi2和lkj两个分布新增负样本测试

需要通过流水线

嗯嗯目前还剩下俩，看起来需要同学手动通过
@jeff41404
Copy link
Contributor

jeff41404 commented May 31, 2024

please add link of rfc in description above.
jeff41404
jeff41404 reviewed May 31, 2024
View reviewed changes
python/paddle/distribution/chi2.py Outdated
if not paddle.all(self.df > 0):
raise ValueError("The arg of `df` must be positive.")

super().__init__(self.df, self.rate)
Copy link
Contributor

@jeff41404 jeff41404 May 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

except __init__, shall we need method of expand?
jeff41404
jeff41404 reviewed May 31, 2024
View reviewed changes
python/paddle/distribution/lkj_cholesky.py

# 5.Compute the normalization term and return the final log probability density:
normalize_term = pi_constant + numerator - denominator
return unnormalized_log_pdf - normalize_term
Copy link
Contributor

@jeff41404 jeff41404 May 31, 2024

Choose a reason for hiding this comment

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

shall we need method of expand?
@cmcamdy
Copy link
Contributor Author

cmcamdy commented Jun 1, 2024

except __init__, shall we need method of expand?

sry,
I'm not quite sure about the role/function of the expand method.
Can you describe it to me? I haven't found a specifically implemented expand method among the distribution.
@cmcamdy
Copy link
Contributor Author

cmcamdy commented Jun 1, 2024

please add link of rfc in description above.

done.
@cmcamdy cmcamdy requested a review from jeff41404 June 3, 2024 05:44
@luotao1
Copy link
Contributor

luotao1 commented Jun 3, 2024

@cmcamdy 关于expand的问题，comment by @jeff41404

pytorch 中有expand方法，我们是否需要，设计文档中也没有说明，如果不需要，代码转换时如何替代
@cmcamdy
Copy link
Contributor Author

cmcamdy commented Jun 3, 2024 •
edited
Loading

@cmcamdy 关于expand的问题，comment by @jeff41404

pytorch 中有expand方法，我们是否需要，设计文档中也没有说明，如果不需要，代码转换时如何替代

我明白了，实际上pytorch中的extend是对batch维度的扩展，如gamma中的实现：https://github.com/pytorch/pytorch/blob/e3ac61587aa368c613ef01df1f328a396b64cd5d/torch/distributions/gamma.py#L60
这部分逻辑paddle中并没有专门实现（因此在设计的时候并没有计划实现expand类的方法），但在如果想要等价实现，只需要在初始化的时候传入带batch的参数即可，我们在sample中会考虑多维shape的情况

I see, in PyTorch, the expand functionality is actually used to extend the batch dimension, as exemplified in the Gamma distribution implementation: https://github.com/pytorch/pytorch/blob/e3ac61587aa368c613ef01df1f328a396b64cd5d/torch/distributions/gamma.py#L60
PaddlePaddle doesn't have a dedicated implementation for this specific logic (indicating that it wasn't planned as part of the original design to include an expand-like method). However, to achieve an equivalent functionality in PaddlePaddle, one can simply pass batch-aware parameters during initialization. When sampling, we will take into account multi-dimensional shapes.
jeff41404
jeff41404 previously approved these changes Jun 5, 2024
View reviewed changes
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
@jeff41404
Copy link
Contributor

jeff41404 commented Jun 5, 2024

@cmcamdy 关于expand的问题，comment by @jeff41404
pytorch 中有expand方法，我们是否需要，设计文档中也没有说明，如果不需要，代码转换时如何替代

我明白了，实际上pytorch中的extend是对batch维度的扩展，如gamma中的实现：https://github.com/pytorch/pytorch/blob/e3ac61587aa368c613ef01df1f328a396b64cd5d/torch/distributions/gamma.py#L60 这部分逻辑paddle中并没有专门实现（因此在设计的时候并没有计划实现expand类的方法），但在如果想要等价实现，只需要在初始化的时候传入带batch的参数即可，我们在sample中会考虑多维shape的情况

I see, in PyTorch, the expand functionality is actually used to extend the batch dimension, as exemplified in the Gamma distribution implementation: https://github.com/pytorch/pytorch/blob/e3ac61587aa368c613ef01df1f328a396b64cd5d/torch/distributions/gamma.py#L60 PaddlePaddle doesn't have a dedicated implementation for this specific logic (indicating that it wasn't planned as part of the original design to include an expand-like method). However, to achieve an equivalent functionality in PaddlePaddle, one can simply pass batch-aware parameters during initialization. When sampling, we will take into account multi-dimensional shapes.

ok, thanks
@cmcamdy
Copy link
Contributor Author

cmcamdy commented Jun 5, 2024 •
edited
Loading

@luotao1 , 我重启了下APPROVAL的CI，似乎还需要下面几位同学的approve，这个CI里面的报错提示如下：
2024-06-05 11:28:16 0. You must have raindrops2sea or XiaoguangHu01 approval for change 20+ files or add than 1000+ lines of content.
2024-06-05 11:28:16 1. You must have one QA (XieYunshen(Recommend) or chalsliu) approval for setting parameter RUN_TYPE as EXCLUSIVE, DIST, HYBRID, NIGHTLY, EXCLUSIVE:NIGHTLY or DISTNIGHTLY, or setting parameter SERIAL, or setting TIMEOUT properties.
@luotao1
Copy link
Contributor

luotao1 commented Jun 5, 2024

Approval 流水线我们会来操作，请耐心等待 review 即可
sunzhongkai588
sunzhongkai588 reviewed Jun 7, 2024
View reviewed changes
@paddle-ci-bot
Copy link

paddle-ci-bot bot commented Jun 9, 2024

Sorry to inform you that fe3edf2's CIs have passed for more than 7 days. To prevent PR conflicts, you need to re-run all CIs manually.
@cmcamdy @sunzhongkai588
Update python/paddle/distribution/chi2.py
386519d 
Co-authored-by: zachary sun <70642955+sunzhongkai588@users.noreply.github.com>
cmcamdy and others added 2 commits June 11, 2024 20:58
@cmcamdy @sunzhongkai588
Update python/paddle/distribution/chi2.py
b7cdcc6 
Co-authored-by: zachary sun <70642955+sunzhongkai588@users.noreply.github.com>
@cmcamdy
merge develop
23c87e7
@cmcamdy
Copy link
Contributor Author

cmcamdy commented Jun 12, 2024

已修改
sunzhongkai588
sunzhongkai588 approved these changes Jun 12, 2024
View reviewed changes
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
@luotao1 luotao1 changed the title 【Hackathon 6th No.5】Add chi2/LKJCholesky API to Paddle 【Hackathon 6th No.5】Add chi2/LKJCholesky API to Paddle -part Jun 12, 2024
@luotao1 luotao1 merged commit fb2bd26 into PaddlePaddle:develop Jun 12, 2024
This was referenced Jun 15, 2024
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

API contributor External developers PaddlePaddle Hackathon

6 participants

@cmcamdy @luotao1 @xuxinyi389 @jeff41404 @sunzhongkai588 @lxd-cumt