Skip to content

Conversation

@zyfncg
Copy link
Contributor

@zyfncg zyfncg commented Jun 2, 2021

PR types

New features

PR changes

OPs

Describe

Add digamma_op and unittest

API:

paddle.digamma() / paddle.tensor.digamma()

Code position:

python/paddle/tensor/math.py

Example:

import paddle

data = paddle.to_tensor([[1, 1.5], [0, -2.2]], dtype='float32')
res = paddle.digamma(data)
print(res)
# Tensor(shape=[2, 2], dtype=float32, place=CUDAPlace(0), stop_gradient=True,
#       [[-0.57721591,  0.03648996],
#        [ nan       ,  5.32286835]])

Doc:

Cn Doc PR: PaddlePaddle/docs#3574

image

@paddle-bot-old
Copy link

paddle-bot-old bot commented Jun 2, 2021

Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

@CLAassistant
Copy link

CLAassistant commented Jun 2, 2021

CLA assistant check
All committers have signed the CLA.

@@ -0,0 +1,100 @@
/* Copyright (c) 2020 PaddlePaddle Authors. All Rights Reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

2020 -> 2021

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

@@ -0,0 +1,64 @@
/* Copyright (c) 2020 PaddlePaddle Authors. All Rights Reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

same above

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!

def init_dtype_type(self):
self.dtype = np.float32

def test_check_grad_normal(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

use default numeric_grad_delta is ok

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!

self.check_output()

def test_check_grad_normal(self):
self.check_grad(['X'], 'Out', numeric_grad_delta=1e-7)
Copy link
Contributor

Choose a reason for hiding this comment

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

use default value

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!

out_value = exe.run(feed=input_dict, fetch_list=[out.name])
self.assertEqual(
np.allclose(
out_value[0], sc_res, rtol=1e-04), True)
Copy link
Contributor

Choose a reason for hiding this comment

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

can use more samll rtol?

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, update the rtol to 1e-05

with fluid.dygraph.guard(place):
input_t = paddle.to_tensor(input)
res = paddle.digamma(input_t).numpy()
self.assertEqual(np.allclose(res, sc_res, rtol=1e-04), True)
Copy link
Contributor

Choose a reason for hiding this comment

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

same above

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, update the rtol to 1e-05

name(str, optional): The default value is None. Normally there is no need for
user to set this property. For more information, please refer to :ref:`api_guide_Name`
Returns:
Tensor, the digamma of the input Tensor computed element-wise.
Copy link
Contributor

Choose a reason for hiding this comment

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

element-wise?

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, has modified this sentence.


REGISTER_OP_CUDA_KERNEL(
digamma_grad,
ops::DigammaGradKernel<paddle::platform::CUDADeviceContext, float>,
Copy link
Contributor

Choose a reason for hiding this comment

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

need special DigammaGradKernel here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

register a CudaKernel for digamma grad is necessary

};

template <typename T>
class DigammaKernel<platform::CUDADeviceContext, T>
Copy link
Contributor

@chenwhql chenwhql Jun 4, 2021

Choose a reason for hiding this comment

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

here is faster? maybe can test it and add some comments

Copy link
Contributor Author

@zyfncg zyfncg Jun 4, 2021

Choose a reason for hiding this comment

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

Done, it isn't faster in the test, so removed it.

limitations under the License. */

#include <unsupported/Eigen/SpecialFunctions>
#include "paddle/fluid/operators/digamma_op.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

here we only need the header digamma_op.h, remove other headers

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!

chenwhql
chenwhql previously approved these changes Jun 8, 2021
Copy link
Contributor

@chenwhql chenwhql left a comment

Choose a reason for hiding this comment

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

LGTM

chenwhql
chenwhql previously approved these changes Jun 10, 2021
Copy link
Contributor

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

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

@TCChenlong TCChenlong left a comment

Choose a reason for hiding this comment

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

LGTM

@chenwhql chenwhql merged commit 02a6d49 into PaddlePaddle:develop Jun 15, 2021
@zyfncg zyfncg deleted the zyf-addop branch July 28, 2021 04:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants