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

Move real and imag op to phi #39777

Merged
merged 6 commits into from
Feb 22, 2022

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里修改

@@ -50,4 +50,14 @@ DenseTensor Conj(const Context& dev_ctx, const DenseTensor& x) {
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants