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 3 No.9】为 Paddle 新增 Laplace API #44913

Merged
merged 27 commits into from
Sep 23, 2022

Conversation

TUDelftHao
Copy link
Contributor

@TUDelftHao TUDelftHao commented Aug 5, 2022

PR types

New features

PR changes

APIs

Describe

新增laplace分布api;历史commit: #44579 (comment)
设计文档:PaddlePaddle/community#190 (comment)
中文api文档:PaddlePaddle/docs#5214

@paddle-bot
Copy link

paddle-bot bot commented Aug 5, 2022

你的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 Aug 5, 2022
@TUDelftHao TUDelftHao marked this pull request as ready for review August 5, 2022 02:42
@Ligoml
Copy link
Contributor

Ligoml commented Aug 8, 2022

  1. 需要按照活动要求的格式修改下pr标题,方便统计~
  2. 需要补充中文文档到 docs repo

@cxxly
Copy link
Contributor

cxxly commented Aug 8, 2022

解决CI失败中问题

@cxxly
Copy link
Contributor

cxxly commented Aug 8, 2022

修改PR标题,一句话描述清楚你的工作

@cxxly
Copy link
Contributor

cxxly commented Aug 8, 2022

参考 test_distribution_multinomial_static 增加静态图测试(测试用例保持和动态图一致)

@TUDelftHao TUDelftHao changed the title 增加ks-test;注册kl_divergence 【PaddlePaddle Hackathon 3 No.9】为 Paddle 新增 Laplace API Aug 8, 2022
loc, scale, uniform = paddle.broadcast_tensors(
[self.loc, self.scale, uniform])
else:
loc, scale = self.loc, self.scale
Copy link
Contributor

Choose a reason for hiding this comment

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

建议不用显示broadcast,加减乘除等Paddle一些基本运算支持Broadcast语义

@cxxly
Copy link
Contributor

cxxly commented Aug 29, 2022

image

@cxxly 单测的时候发现sample函数的采样有时会出现inf的结果,公式和torch以及tensorflow的框架都是一样的,请教一下这是什么原因?已经把采样上下限缩小了eps / 2,还是不能避免这个问题。

这个不太好直接判断,不排除Paddle OP本身可能存在缺陷,需要Debug。 建议
1)在 rsample方法中每一步操作,加上日志,打印每个OP的输入、输出数据
2)因为是随机性问题,重复执行 rsample,直到复现上述问题
3)根据日志定位是写法问题还是OP存在BUG,定位到具体OP
4)如果是写法错误,修改即可;如果是OP存在BUG,提交issue

另外,我在linux机器上没有复现,是只有Windows存在上述问题吗?自己先尝试定位问题,如果无法定位或问题解决不了,再进一步交流

@TUDelftHao
Copy link
Contributor Author

TUDelftHao commented Sep 3, 2022

image
@cxxly 单测的时候发现sample函数的采样有时会出现inf的结果,公式和torch以及tensorflow的框架都是一样的,请教一下这是什么原因?已经把采样上下限缩小了eps / 2,还是不能避免这个问题。

这个不太好直接判断,不排除Paddle OP本身可能存在缺陷,需要Debug。 建议 1)在 rsample方法中每一步操作,加上日志,打印每个OP的输入、输出数据 2)因为是随机性问题,重复执行 rsample,直到复现上述问题 3)根据日志定位是写法问题还是OP存在BUG,定位到具体OP 4)如果是写法错误,修改即可;如果是OP存在BUG,提交issue

另外,我在linux机器上没有复现,是只有Windows存在上述问题吗?自己先尝试定位问题,如果无法定位或问题解决不了,再进一步交流

感谢@cxxly。我这边debug了一下,paddle.uniform()的实现可能存在问题,具体在这个issue #45713 (comment) 提出了,辛苦有空看下。Laplace我强制paddle.uniform生成的数据类型和self.loc一致,并缩小上下界,暂时回避了这个问题,目前没有再出现inf的结果。

@TUDelftHao TUDelftHao requested a review from cxxly September 4, 2022 09:05
cxxly
cxxly previously approved these changes Sep 6, 2022
Copy link
Contributor

@cxxly cxxly 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 requested a review from Ligoml September 6, 2022 11:39
@Ligoml
Copy link
Contributor

Ligoml commented Sep 8, 2022

英文API文档的部分需要和中文部分对齐哈,现在缺少了公式和name参数的介绍

@TUDelftHao
Copy link
Contributor Author

英文API文档的部分需要和中文部分对齐哈,现在缺少了公式和name参数的介绍

Done.


.. math::

log\_prob(value) = \frac{-log(2 * \sigma) - |value - \mu|}{\sigma}
Copy link
Contributor

@Ligoml Ligoml Sep 9, 2022

Choose a reason for hiding this comment

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

image

这里解析报错了,再检查一下,.. math::和公式之间应该不用空一行

Tensor: The log probability, whose data type is same with value.

Examples:
.. code-block:: python
Copy link
Contributor

@Ligoml Ligoml Sep 9, 2022

Choose a reason for hiding this comment

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

image

.. code-block:: python 和代码块之间需要空一行


.. math::

cdf(value) = 0.5 - 0.5 * sign(value - \mu) * e^\frac{-|(\mu - \sigma)|}{\sigma}
Copy link
Contributor

Choose a reason for hiding this comment

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

image

公式似乎没有正常显示

@Ligoml
Copy link
Contributor

Ligoml commented Sep 9, 2022

Copy link
Contributor

@Ligoml Ligoml 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

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

@Ligoml Ligoml merged commit 09e4875 into PaddlePaddle:develop Sep 23, 2022
umiswing pushed a commit to umiswing/Paddle that referenced this pull request Sep 28, 2022
…44913)

* 增加ks-test;注册kl_divergence

* bug fix

* bug fix data shape

* 按要求修改

* sample code fix

* 按要求修改

* bugfix

* bugfix

* bugfix

* bugfix

* bugfix

* fix sample func

* fix sample func

* 按要求修改;sample函数bugfix

* 按要求修改;sample函数bugfix

* bugfix

* 补充英文文档公式

* 英文文档bugfix

* bugfix

* bugfix

* 英文文档修复

* 去除name参数
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants