Skip to content

Conversation

@From00
Copy link
Contributor

@From00 From00 commented Feb 21, 2022

PR types

Function optimization

PR changes

OPs

Describe

Move real and imag op to phi

@paddle-bot-old
Copy link

paddle-bot-old bot commented Feb 21, 2022

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

@paddle-bot-old
Copy link

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

Copy link
Contributor

@zyfncg zyfncg left a comment

Choose a reason for hiding this comment

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

real和imag这两个kernel可以都放到complex_kernel文件里,能减少一些文件数量


#include "paddle/phi/common/complex.h"
#include "paddle/phi/core/kernel_registry.h"
#include "paddle/phi/kernels/imag_kernel.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

imag_kernel.h这个头文件能放在第一个吗?

#include "paddle/phi/common/complex.h"
#include "paddle/phi/core/kernel_registry.h"
#include "paddle/phi/kernels/impl/real_grad_kernel_impl.h"
#include "paddle/phi/kernels/real_grad_kernel.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

同上

#include "paddle/phi/common/complex.h"
#include "paddle/phi/core/kernel_registry.h"
#include "paddle/phi/kernels/impl/real_kernel_impl.h"
#include "paddle/phi/kernels/real_kernel.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

同上


#include "paddle/fluid/platform/for_range.h"
#include "paddle/phi/kernels/funcs/complex_functors.h"
#include "paddle/phi/kernels/imag_grad_kernel.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

这里应该不需要引imag_grad_kernel.h这个头文件了


#include "paddle/fluid/platform/for_range.h"
#include "paddle/phi/kernels/funcs/complex_functors.h"
#include "paddle/phi/kernels/imag_kernel.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

同上


namespace phi {

template <typename T, typename DeviceContext>
Copy link
Contributor

Choose a reason for hiding this comment

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

按照目前规范,Kernel模板参数DeviceContext先统一为Context

auto numel = x.numel();
auto* x_data = x.data<T>();
auto* out_data = dev_ctx.template Alloc<phi::funcs::Real<T>>(
out, static_cast<size_t>(numel * sizeof(phi::funcs::Real<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_data的内存大小吗?如果使用默认值分配会有问题吗?

@From00
Copy link
Contributor Author

From00 commented Feb 21, 2022

real和imag这两个kernel可以都放到complex_kernel文件里,能减少一些文件数量

done

@@ -0,0 +1,31 @@
/* Copyright (c) 2021 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.

2021->2022

Copy link
Contributor Author

Choose a reason for hiding this comment

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

在下个PR里修改

return x;
}

template <typename T, typename DeviceContext>
Copy link
Contributor

Choose a reason for hiding this comment

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

DeviceContext -> Context

Copy link
Contributor Author

Choose a reason for hiding this comment

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

在下个PR里修改

@From00 From00 merged commit 345cc8f into PaddlePaddle:develop Feb 22, 2022
@From00 From00 deleted the move-real-and-imag-op-to-phi branch March 11, 2022 07:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants