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 4th No.13】为 Paddle 新增 Bernoulli API #453

Merged
merged 4 commits into from
Mar 21, 2023

Conversation

megemini
Copy link
Contributor

@megemini megemini commented Mar 13, 2023

PR types

New features

PR changes

Docs

Describe

[used AI Studio]

@CLAassistant
Copy link

CLAassistant commented Mar 13, 2023

CLA assistant check
All committers have signed the CLA.

@paddle-bot
Copy link

paddle-bot bot commented Mar 13, 2023

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

## 命名与参数设计
- 类名: `Bernoulli`
- API: `class paddle.distribution.Bernoulli(probs=None, logits=None)`

Copy link

Choose a reason for hiding this comment

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

目前因为其它API没有支持logits,为了保持一致,前期仅支持probs就行;如果有兴趣,可以所有API统一支持logits

Copy link
Contributor Author

@megemini megemini Mar 17, 2023

Choose a reason for hiding this comment

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

OK! 那我这里修改为

class paddle.distribution.Bernoulli(probs)

有机会的话尝试把其他分布也写一下~ :)

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! 修改完成!
谢谢!:)

不涉及

## API实现方案
### 继承父类: [Distribution](https://github.com/PaddlePaddle/Paddle/blob/develop/python/paddle/distribution/distribution.py#L33)
Copy link

Choose a reason for hiding this comment

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

应该是继承ExponentialFamily

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这里确实犹豫过是继承 Distribution 还是 ExponentialFamily

PyTorchBernoulliNormal 都是用的 ExponentialFamily,而 paddleNormal 继承的是 Distribution,所以这里遵从了 paddle 的习惯。

那我这里改一下,

### 继承父类: [ExponentialFamily]

:)

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! 修改完成!
谢谢!:)

Copy link

Choose a reason for hiding this comment

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

Normal是一个遗留的比较旧API,还没有迁移到新的架构体系中

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK!我尽量多参考现有的方案~

谢谢!:)

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