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

【complex op】 No.10 add complex support for exp/expm1 #56398

Closed
wants to merge 14 commits into from
Closed

【complex op】 No.10 add complex support for exp/expm1 #56398

wants to merge 14 commits into from

Conversation

Wanglongzhi2001
Copy link
Contributor

@Wanglongzhi2001 Wanglongzhi2001 commented Aug 17, 2023

PR types

New features

PR changes

OPs

Description

运行测试会出现问题,报如下的错误:
2023-08-17 14-35-55屏幕截图

可以麻烦帮忙看看吗, @ScottWong98
#56145

@paddle-bot
Copy link

paddle-bot bot commented Aug 17, 2023

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

@CLAassistant
Copy link

CLAassistant commented Aug 17, 2023

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@paddle-bot paddle-bot bot added contributor External developers status: proposed labels Aug 17, 2023
@paddle-bot
Copy link

paddle-bot bot commented Aug 17, 2023

✅ This PR's description meets the template requirements!
Please wait for other CI results.

@@ -183,28 +263,72 @@ def test_api_int(self):
paddle.enable_static()


class TestParameter:
Copy link
Contributor

Choose a reason for hiding this comment

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

把这个类放到原来位置吧,不然diff太大了

class TestExpm1(TestActivation):
def setUp(self):
self.op_type = "expm1"
self.prim_op_type = "prim"
Copy link
Contributor

Choose a reason for hiding this comment

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

应该不需要添加这行

self.python_api = paddle.expm1
self.public_python_api = paddle.exp
Copy link
Contributor

Choose a reason for hiding this comment

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

这行应该也不需要添加,并且如果添加应该是 paddle.expm1

out = np.expm1(x)

self.inputs = {'X': OpTest.np_dtype_to_fluid_dtype(x)}
self.outputs = {'Out': out}
self.convert_input_output()

def test_check_grad(self):
self.check_grad(['X'], 'Out')
if self.dtype == np.complex64 or self.dtype == np.complex128:
Copy link
Contributor

Choose a reason for hiding this comment

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

这一块应该也不需要更改,check_gradcheck_prim 默认为 False。之前的逻辑里非 complex 类型的数据也没有 check prim,保持逻辑一致吧

pass


class TestExp_Complex128(OpTest):
Copy link
Contributor

Choose a reason for hiding this comment

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

这个应该可以继承 TestExp_Complex64

@@ -96,6 +96,25 @@ struct Cosine<dtype::bfloat16> {
}
};

template <typename T>
struct Exp {
Copy link
Contributor

Choose a reason for hiding this comment

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

这一块或许并不需要显示地定义 Exp ?

typename Out,
typename dOut,
typename dX>
void operator()(Device d, X x, Out out UNUSED, dOut dout, dX dx) const {
Copy link
Contributor

Choose a reason for hiding this comment

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

应该是 (Device d, X x UNUSED, Out out, dOut dout, dX dx)

typename dX>
void operator()(Device d, X x, Out out UNUSED, dOut dout, dX dx) const {
dx.device(d) =
dout * x.unaryExpr(Exp<ComplexType<T>>()).unaryExpr(Conj<T>());
Copy link
Contributor

Choose a reason for hiding this comment

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

只需要求 out 的共轭就行了吧

typename Out,
typename dOut,
typename dX>
void operator()(Device d, X x, Out out UNUSED, dOut dout, dX dx) const {
Copy link
Contributor

Choose a reason for hiding this comment

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

应该是 (Device d, X x UNUSED, Out out, dOut dout, dX dx)

typename dX>
void operator()(Device d, X x, Out out UNUSED, dOut dout, dX dx) const {
dx.device(d) =
dout * x.unaryExpr(Exp<ComplexType<T>>()).unaryExpr(Conj<T>());
Copy link
Contributor

Choose a reason for hiding this comment

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

这块的求导应该不对

@ScottWong98
Copy link
Contributor

@Wanglongzhi2001 可以在 build 目录下使用 PYTHONPATH=./python:./test/ pytest -v -s test/legacy_test/test_activation_op.py::TestExp_Complex64::test_check_output 针对具体的 test case 查看到底是哪个case导致了截图中出现的错误。建议先写 cpu 下的 exp,再写 cpu 下的 expm1,然后扩展到 GPU 上。保证每一步都正确后,就能知道是哪一次的修改出现了问题

@Wanglongzhi2001
Copy link
Contributor Author

@Wanglongzhi2001 可以在 build 目录下使用 PYTHONPATH=./python:./test/ pytest -v -s test/legacy_test/test_activation_op.py::TestExp_Complex64::test_check_output 针对具体的 test case 查看到底是哪个case导致了截图中出现的错误。建议先写 cpu 下的 exp,再写 cpu 下的 expm1,然后扩展到 GPU 上。保证每一步都正确后,就能知道是哪一次的修改出现了问题

谢谢您的建议,第一次给Paddle提PR,还不大熟悉~

@Wanglongzhi2001
Copy link
Contributor Author

@ScottWong98 您好,现在的情况是如图所示测试无法通过,想向您请教一下。
fail

第一个问题是Complex128类型时输出梯度的容忍错误度为1e-6,而我的达到了0.005, 查看Optest类后发现对于Complex128类型容忍错误度写死了1e-6无法通过max_relative_error进行修改,

if (
self.dtype == np.complex128
and self.op_type
not in op_threshold_white_list.NEED_FIX_FP64_CHECK_GRAD_THRESHOLD_OP_LIST
):
numeric_grad_delta = 1e-5
max_relative_error = 1e-6

第二个出现的问题是GPU的check_output问题,在CPU上输出没有问题,但是GPU上会出现虚部一直为0的情况,但是GPU上就仅是简单的使用thrust库进行计算而已,我也在本地测试过thrust的exp函数没有问题,不知道问题出在哪儿?

#if defined(PADDLE_WITH_CUDA_OR_HIP_COMPLEX) && \
    (defined(__CUDA_ARCH__) || defined(__HIPCC__))
  return complex<T>(thrust::exp(thrust::complex<T>(a)));

@ScottWong98
Copy link
Contributor

@ScottWong98 您好,现在的情况是如图所示测试无法通过,想向您请教一下。 fail

第一个问题是Complex128类型时输出梯度的容忍错误度为1e-6,而我的达到了0.005, 查看Optest类后发现对于Complex128类型容忍错误度写死了1e-6无法通过max_relative_error进行修改,

if (
self.dtype == np.complex128
and self.op_type
not in op_threshold_white_list.NEED_FIX_FP64_CHECK_GRAD_THRESHOLD_OP_LIST
):
numeric_grad_delta = 1e-5
max_relative_error = 1e-6

第二个出现的问题是GPU的check_output问题,在CPU上输出没有问题,但是GPU上会出现虚部一直为0的情况,但是GPU上就仅是简单的使用thrust库进行计算而已,我也在本地测试过thrust的exp函数没有问题,不知道问题出在哪儿?

#if defined(PADDLE_WITH_CUDA_OR_HIP_COMPLEX) && \
    (defined(__CUDA_ARCH__) || defined(__HIPCC__))
  return complex<T>(thrust::exp(thrust::complex<T>(a)));
  • 关于第一个问题,我后面再看一下
  • 关于第二个问题,建议依据 Traceback debug 一下,看到底是哪个地方导致了 虚部全为 0 的情况

@Wanglongzhi2001
Copy link
Contributor Author

@ScottWong98 您好,现在的情况是如图所示测试无法通过,想向您请教一下。 fail
第一个问题是Complex128类型时输出梯度的容忍错误度为1e-6,而我的达到了0.005, 查看Optest类后发现对于Complex128类型容忍错误度写死了1e-6无法通过max_relative_error进行修改,

if (
self.dtype == np.complex128
and self.op_type
not in op_threshold_white_list.NEED_FIX_FP64_CHECK_GRAD_THRESHOLD_OP_LIST
):
numeric_grad_delta = 1e-5
max_relative_error = 1e-6

第二个出现的问题是GPU的check_output问题,在CPU上输出没有问题,但是GPU上会出现虚部一直为0的情况,但是GPU上就仅是简单的使用thrust库进行计算而已,我也在本地测试过thrust的exp函数没有问题,不知道问题出在哪儿?

#if defined(PADDLE_WITH_CUDA_OR_HIP_COMPLEX) && \
    (defined(__CUDA_ARCH__) || defined(__HIPCC__))
  return complex<T>(thrust::exp(thrust::complex<T>(a)));
  • 关于第一个问题,我后面再看一下
  • 关于第二个问题,建议依据 Traceback debug 一下,看到底是哪个地方导致了 虚部全为 0 的情况

好的,谢谢~

@Wanglongzhi2001
Copy link
Contributor Author

@ScottWong98 @GGBond8488 麻烦 reveiw 一下

typename dOut,
typename dX>
void operator()(Device d, X x UNUSED, Out out, dOut dout, dX dx) const {
dx.device(d) = dout * out.unaryExpr(Conj<T>()) + dout;
Copy link
Contributor

Choose a reason for hiding this comment

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

请教一下,为什么这里 expm1 (exp - 1) 的 梯度是 dout*(exp + 1) :)

Copy link
Contributor Author

@Wanglongzhi2001 Wanglongzhi2001 Sep 1, 2023

Choose a reason for hiding this comment

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

事实上我也不是很理解,tensorflow 的梯度实现也是不用加上这个 dout 的:
https://github.com/tensorflow/tensorflow/blob/f82986df65bea201e5aa466e6993504372132cec/tensorflow/python/ops/math_grad.py#L688-L695

但是我看到 paddle 的其他数据类型的梯度实现是加上了这个 dout,并且我不加上这个 dout 确实梯度误差检查过不了,所以我就加上了,我也想请教下 paddle 的前辈们是不是 paddle 的算子梯度的实现这块不一样导致这样的结果。

Copy link
Contributor

Choose a reason for hiding this comment

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

exp(x)-1的grad是exp(x),exp(x)=(exp(x)-1)+1=out+1

@@ -640,7 +640,16 @@ def expm1(x, name=None):
check_variable_and_dtype(
x,
'x',
['float16', 'uint16', 'float32', 'float64', 'int32', 'int64'],
[
Copy link
Contributor

Choose a reason for hiding this comment

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

该方法的 docstring 也相应修改一下

self.check_output()

def test_check_grad(self):
self.check_grad(['X'], 'Out', max_relative_error=0.006)
Copy link
Contributor

Choose a reason for hiding this comment

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

这里的 max_relative_error 对于 complex64 和 complex128 都需要嘛

Copy link
Contributor Author

Choose a reason for hiding this comment

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

是的

@Wanglongzhi2001
Copy link
Contributor Author

@ScottWong98 docstring 已经修改过了,请问还有什么需要改正的地方吗?麻烦 review 一下 ^_^

Copy link
Contributor

@ScottWong98 ScottWong98 left a comment

Choose a reason for hiding this comment

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

LGTM, 麻烦 @GGBond8488 再 review 一下

};

template <typename T>
struct Expm1 {
Copy link
Contributor

Choose a reason for hiding this comment

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

这里为什么要单独定义Expm1呢,按理说这里本身是定义functor的, 这个也相当于是定义了expm1的运算,与下面的有点重复,如果是想定义复数expm1运算,建议放到complex.h里面

Copy link
Contributor Author

Choose a reason for hiding this comment

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

您好,因为 trust 不支持 expm1 算子,并且 C++ 中的 expm1 不支持复数类型,所以 expm1 的复数实现需要用 exp 的复数实现来复合。之前我是放在 complex.h 里的,但是之前 @ScottWong98 建议 C++ 中不支持的放在 activation_functor.h 中
image

Copy link
Contributor

Choose a reason for hiding this comment

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

好的,但是确定需要自己额外补充定义expm1吗,我看下面非复数的expm1也是调用的函数,如果确认需要的话,可以在activation_functor中定义,但是需要是复数的特化,不要代表所有的类型

Copy link
Contributor Author

Choose a reason for hiding this comment

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

确实是我的疏忽,应该只需要定义复数的特化,感谢您的建议~

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已经修改了,麻烦 review 一下 @GGBond8488

@luotao1 luotao1 added HappyOpenSource Pro 进阶版快乐开源活动,更具挑战性的任务 and removed HappyOpenSource 快乐开源活动issue与PR labels Sep 6, 2023
@@ -466,6 +466,16 @@ HOSTDEVICE inline complex<T> tanh(const complex<T>& a) {
#endif
}

template <typename T>
Copy link
Contributor

Choose a reason for hiding this comment

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

这里之前的pr应该已经加了exp, 可以同步一下最新的代码,不要加重复了

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好的

@Wanglongzhi2001 Wanglongzhi2001 closed this by deleting the head repository Sep 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor External developers HappyOpenSource Pro 进阶版快乐开源活动,更具挑战性的任务
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants