【Hackathon No.16】add PoissonNLLLoss API #51117

LyndonKong · 2023-03-02T09:50:45Z

PR types

New features

PR changes

APIs

Describe

Add paddle.nn.PoissonNLLLoss and paddle.nn.functional.poisson_nll_loss
document pr: PaddlePaddle/docs#5675
rfc pr1: PaddlePaddle/community#395
rfc pr2: PaddlePaddle/community#463

paddle-bot · 2023-03-02T09:50:50Z

你的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 · 2023-03-02T09:50:54Z

❌ The PR is not created using PR's template. You can refer to this Demo.
Please use PR's template, it helps save our maintainers' time so that more developers get helped.

LyndonKong · 2023-03-08T02:56:15Z

@Ligoml 辛苦review，#51117

GGBond8488 · 2023-03-06T09:32:05Z

python/paddle/fluid/tests/unittests/test_poisson_nll_loss.py

+        for r in res:
+            np.allclose(out_ref, r, rtol=1e-5)
+
+    def test_api(self):


不同case的测试分成多个test class吧，方便后续定位具体是哪个case异常

已经对单元测试进行了拆分

这个单测有点分的太细了，动态图和静态图可以整合到一个class下面的两个方法，然后再不同的case分成不同的class, test error的也可以整合到一个class，不同的方法测试不同的error

GGBond8488 · 2023-03-06T09:32:43Z

python/paddle/nn/functional/loss.py

+    See more detail in :ref:`NLLLoss <api_paddle_nn_PoissonNLLLoss>` .
+    Parameters::
+
+         input (Tensor):


tensor 类型的输入都说明一下支持的数据类型

已经添加

GGBond8488 · 2023-03-08T08:22:49Z

python/paddle/fluid/tests/unittests/test_poisson_nll_loss.py

+        self.test_dynamic_case('float64', full=True)
+        self.test_dynamic_case(log_input=False, full=True)
+        self.test_static_case(full=True, reduction='none')
+        self.test_dynamic_case('float64', full=True)


这里为什么不测 float16,int16,in32

poisson_nll_loss当前只支持float32和float64。paddle.poisson_nll_loss依赖exp算子和log算子，exp算子当前不支持float16和int 16，log算子不支持int32，在所依赖的算子支持对应数据类型后我们将添加相应支持。

LyndonKong · 2023-03-13T12:58:02Z

@GGBond8488 对问题进行了修改，辛苦review

GGBond8488 · 2023-03-15T06:38:45Z

python/paddle/nn/functional/loss.py

+    label,
+    log_input=True,
+    full=False,
+    eps=1e-8,


eps改为全称epsilon 具体见 https://www.paddlepaddle.org.cn/documentation/docs/zh/dev_guides/api_contributing_guides/api_design_guidelines_standard_cn.html

已根据意见进行修改

GGBond8488 · 2023-03-15T06:39:44Z

python/paddle/nn/functional/loss.py

+            If ``True``, the Stirling approximation term is added.
+            If ``False``, the Stirling approximation is dropped.
+            Default: ``False``.
+         eps (float, optioonal):


文档里面的说明需要同步修改（eps）

同步更新了API文档和doc string。

以及rfc设计文档也提一个pr修改吧

已经提交pr。

GGBond8488 · 2023-03-15T06:41:43Z

python/paddle/nn/functional/loss.py

+        )
+    # check input dtype and dimension
+    if not in_dygraph_mode():
+        check_variable_and_dtype(


check_variable_and_dtype 会自动跳过动态图，所以不用增加这个分支，让代码更加动静统一

去除了判断分支

GGBond8488 · 2023-03-15T06:42:55Z

python/paddle/nn/functional/loss.py

+        check_variable_and_dtype(
+            input,
+            'input',
+            ['int32', 'int64', 'float32', 'float64'],


docstr说仅支持float32以及float64，这里为什么还会支持int32以及int64呢

修复了数据类型支持的不一致

GGBond8488 · 2023-03-15T06:43:04Z

python/paddle/nn/functional/loss.py

+        check_variable_and_dtype(
+            label,
+            'label',
+            ['int32', 'int64', 'float32', 'float64'],


GGBond8488 · 2023-03-15T06:43:34Z

python/paddle/nn/layer/loss.py

+            If ``True``, the Stirling approximation term is added.
+            If ``False``, the Stirling approximation is dropped.
+            Default: ``False``.
+         eps (float, optioonal):


同eps使用全称

LyndonKong · 2023-03-20T04:30:56Z

@GGBond8488 辛苦review

sunzhongkai588

最后一个小问题，麻烦修改～

sunzhongkai588 · 2023-04-03T07:09:11Z

python/paddle/nn/functional/loss.py

+            It's data type should be float32, float64.
+         log_input (bool, optional):
+            Whether to the treat input tensor as log input.
+            If ``True`` the loss is computed as,:math:`\exp(\text{input}) - \text{label} * \text{input}`.


以下是在 PoissonNLLLoss API里提到过的问题，在poisson_nll_loss中没有进行同步修改

math公式左右得空格，否则无法展示

Done in our new commit, thanks for your kind review : )

sunzhongkai588

LGTM

luotao1 · 2023-04-03T09:54:17Z

#51117 (comment) 需要处理下

LyndonKong · 2023-04-06T13:44:55Z

We follow the suggesstions to support the mentioned data dtypes of float16 and blfoat16, but just adding support in check_variable_and_dtype in exp and log op did not work.

What we did?

rebase our branch on the latest develop branch
add supported data types in log and exp op
fix our code to support these data types
compile this version and test the newly supported datatypes.

What we find?

Our unittests on these newly supported data types failed both in float16 and bfloat16, the error messages are pasted as following:

733: test_poisson_nll_loss failed
733: ........E.................
733: ======================================================================
733: ERROR: test_api (test_poisson_nll_loss.TestPoissonNLLLossFloat16Case)
733: ----------------------------------------------------------------------
733: Traceback (most recent call last):
733: File "/paddle/build/python/paddle/fluid/tests/unittests/test_poisson_nll_loss.py", line 205, in test_api
733: self.test_static_case(dtype="float16")
733: File "/paddle/build/python/paddle/fluid/tests/unittests/test_poisson_nll_loss.py", line 113, in test_static_case
733: fetch_list=[out1, out2],
733: File "/paddle/build/python/paddle/fluid/executor.py", line 1458, in run
733: use_prune=use_prune,
733: File "/paddle/build/python/paddle/fluid/executor.py", line 1660, in _run_impl
733: scope, list(feed.keys()), fetch_list, return_numpy
733: File "/paddle/build/python/paddle/fluid/executor.py", line 683, in run
733: scope, feed_names, fetch_list
733: NotImplementedError: In user code:
733:
733: File "/paddle/tools/test_runner.py", line 70, in
733: main()
733: File "/paddle/tools/test_runner.py", line 52, in main
733: res = unittest.TextTestRunner(stream=buffer).run(tests)
733: File "/usr/lib/python3.7/unittest/runner.py", line 176, in run
733: test(result)
733: File "/usr/lib/python3.7/unittest/suite.py", line 84, in call
733: return self.run(*args, **kwds)
733: File "/usr/lib/python3.7/unittest/suite.py", line 122, in run
733: test(result)
733: File "/usr/lib/python3.7/unittest/suite.py", line 84, in call
733: return self.run(*args, **kwds)
733: File "/usr/lib/python3.7/unittest/suite.py", line 122, in run
733: test(result)
733: File "/usr/lib/python3.7/unittest/case.py", line 676, in call
733: return self.run(*args, **kwds)
733: File "/usr/lib/python3.7/unittest/case.py", line 628, in run
733: testMethod()
733: File "/paddle/build/python/paddle/fluid/tests/unittests/test_poisson_nll_loss.py", line 205, in test_api
733: self.test_static_case(dtype="float16")
733: File "/paddle/build/python/paddle/fluid/tests/unittests/test_poisson_nll_loss.py", line 99, in test_static_case
733: reduction=reduction,
733: File "/paddle/build/python/paddle/nn/functional/loss.py", line 1569, in poisson_nll_loss
733: loss_out = paddle.exp(input) - label * input
733: File "/paddle/build/python/paddle/tensor/ops.py", line 580, in exp
733: helper.append_op(type='exp', inputs={"X": x}, outputs={"Out": out})
733: File "/paddle/build/python/paddle/fluid/layer_helper.py", line 45, in append_op
733: return self.main_program.current_block().append_op(*args, **kwargs)
733: File "/paddle/build/python/paddle/fluid/framework.py", line 4068, in append_op
733: attrs=kwargs.get("attrs", None),
733: File "/paddle/build/python/paddle/fluid/framework.py", line 2851, in init
733: for frame in traceback.extract_stack():
733:
733: UnimplementedError: There are no kernels which are registered in the exp operator.
733: [Hint: Expected kernels_iter != all_op_kernels.end(), but received kernels_iter == all_op_kernels.end().] (at /paddle/paddle/fluid/framework/operator.cc:2213)
733: [operator < exp > error]
733:
733: ----------------------------------------------------------------------
733: Ran 26 tests in 0.335s
733:
733: FAILED (errors=1)
733:
1/1 Test #733: test_poisson_nll_loss ............***Failed 3.26 sec

Next Step

As we are unfamilar with the implementation of exp and log, we find this beyond our reach to support float16 and bfloat16 in poisson_nll_loss at current stage. If it is possible to merge this pr first and support float16 and bfloat16 when the support of these data types are fully tested in their unitests in the develop branch @luotao1 ?

luotao1 · 2023-04-07T01:59:35Z

rebase our branch on the latest develop branch
add supported data types in log and exp op
fix our code to support these data types

Please give the code

LyndonKong · 2023-04-07T07:32:45Z

rebase our branch on the latest develop branch
add supported data types in log and exp op
fix our code to support these data types

Please give the code

The code is put in another branch, you can see it here

ZzSean · 2023-04-07T07:43:46Z

check_variable_and_dtype

看报错信息，有可能是跑在CPU上了，float16只能在GPU上测试
check_variable_and_dtype里bfloat16需要改成uint16

LyndonKong · 2023-04-07T08:15:19Z

2. uint16

We have supported these data types in our new commit, but due to our local test is not running on CPU compiled version, these tests can only be checked in CI.

LyndonKong · 2023-04-10T08:03:39Z

This pr is ready to be reviewed @luotao1 .

ZzSean

LGTM

jeff41404

LGTM

luotao1 · 2023-04-10T08:48:38Z

hi, @LyndonKong

非常感谢你对飞桨框架的贡献，我们正在运营一个PFCC组织，会通过定期分享技术知识与发布开发者主导任务的形式持续为飞桨框架做贡献，详情可见 https://github.com/luotao1 主页说明。
如果你对PFCC有兴趣，请发送邮件至 ext_paddle_oss@baidu.com，我们会邀请你加入~

paddle-bot bot added contributor External developers status: proposed labels Mar 2, 2023

This was referenced Mar 2, 2023

Update PoissonNLLLoss document. ;test=docs_preview PaddlePaddle/docs#5675

Merged

【PaddlePaddle Hackathon 第四期】任务总览 #50629

Closed

LyndonKong changed the title ~~add PoissonNLLLoss API~~ 【Hackathon No.16】add PoissonNLLLoss API Mar 2, 2023

luotao1 assigned luotao1, cloud2009 and GGBond8488 Mar 2, 2023

ForthTurn force-pushed the poisson_nll_loss branch 3 times, most recently from 91ed3bc to f570567 Compare March 3, 2023 05:16

luotao1 assigned Ligoml Mar 6, 2023

LyndonKong force-pushed the poisson_nll_loss branch from f570567 to 53d9a69 Compare March 6, 2023 16:06

Ligoml mentioned this pull request Mar 7, 2023

【PaddlePaddle Hackathon 第四期】任务总览 #51281

Closed

GGBond8488 reviewed Mar 8, 2023

View reviewed changes

luotao1 added the API label Mar 10, 2023

paddle-bot bot removed the status: proposed label Mar 10, 2023

LyndonKong force-pushed the poisson_nll_loss branch from 53d9a69 to 1df68bb Compare March 13, 2023 12:56

GGBond8488 reviewed Mar 15, 2023

View reviewed changes

LyndonKong force-pushed the poisson_nll_loss branch 6 times, most recently from c7c4aa7 to a8b159d Compare March 16, 2023 12:17

This comment was marked as abuse.

Sign in to view

sunzhongkai588 reviewed Apr 3, 2023

View reviewed changes

LyndonKong requested a review from sunzhongkai588 April 3, 2023 07:22

sunzhongkai588 previously approved these changes Apr 3, 2023

View reviewed changes

LyndonKong requested a review from luotao1 April 3, 2023 08:52

LyndonKong added 7 commits April 6, 2023 18:07

add PoissonNLLLoss API

fef43dc

update unittests

2115a6a

Fix poisson_nll_loss init and update data type support

5b07236

remove type comment

390d583

Update doc string

527189c

Fix doc string erro

2068bb7

Fix doc string math equation format

c80cf60

Add float16 and bfloat16 support

c49854e

ForthTurn dismissed sunzhongkai588’s stale review via c49854e April 7, 2023 08:13

ForthTurn force-pushed the poisson_nll_loss branch from d724acb to c49854e Compare April 7, 2023 08:13

LyndonKong requested a review from ZzSean April 7, 2023 08:16

ZzSean approved these changes Apr 10, 2023

View reviewed changes

jeff41404 approved these changes Apr 10, 2023

View reviewed changes

luotao1 merged commit 349a059 into PaddlePaddle:develop Apr 10, 2023

akshatvishu mentioned this pull request Aug 28, 2023

Discrepancy in Stirling Approximation Handling in Poisson NLL Loss Compared to TensorFlow and PyTorch #56733

Closed

【Hackathon No.16】add PoissonNLLLoss API #51117

【Hackathon No.16】add PoissonNLLLoss API #51117

Conversation

LyndonKong commented Mar 2, 2023 • edited Loading

PR types

PR changes

Describe

paddle-bot bot commented Mar 2, 2023

paddle-bot bot commented Mar 2, 2023

LyndonKong commented Mar 8, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LyndonKong commented Mar 13, 2023

Choose a reason for hiding this comment

LyndonKong Mar 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LyndonKong commented Mar 20, 2023

This comment was marked as abuse.

sunzhongkai588 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sunzhongkai588 left a comment

Choose a reason for hiding this comment

luotao1 commented Apr 3, 2023

LyndonKong commented Apr 6, 2023 • edited Loading

What we did?

What we find?

Next Step

luotao1 commented Apr 7, 2023

LyndonKong commented Apr 7, 2023

ZzSean commented Apr 7, 2023

LyndonKong commented Apr 7, 2023

LyndonKong commented Apr 10, 2023 • edited Loading

ZzSean left a comment

Choose a reason for hiding this comment

jeff41404 left a comment

Choose a reason for hiding this comment

luotao1 commented Apr 10, 2023

LyndonKong commented Mar 2, 2023 •

edited

Loading

LyndonKong Mar 15, 2023 •

edited

Loading

LyndonKong commented Apr 6, 2023 •

edited

Loading

LyndonKong commented Apr 10, 2023 •

edited

Loading