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 eye、size、erfinv、pixel_shuffle OP to phi #39712

Merged
merged 15 commits into from
Feb 25, 2022

Conversation

0x45f
Copy link
Contributor

@0x45f 0x45f commented Feb 18, 2022

PR types

Others

PR changes

Others

Describe

调用ForRange需要include fluid

@paddle-bot-old
Copy link

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

@0x45f 0x45f changed the title move eye OP to pten move eye and size OP to pten Feb 18, 2022
@0x45f 0x45f changed the title move eye and size OP to pten move eye、size、erfinv、pixel_shuffle OP to phi Feb 21, 2022
DenseTensor* out) {
auto place = ctx.GetPlace();
auto out_data = ctx.template Alloc<int64_t>(out);
auto cpu_place = paddle::platform::CPUPlace();
Copy link
Contributor

Choose a reason for hiding this comment

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

这里需要改成phi下的命名空间

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已修改,感谢

cpu_tensor.Resize(out->dims());
auto cpu_data = ctx.template HostAlloc<int64_t>(&cpu_tensor);
cpu_data[0] = input.numel();
paddle::framework::TensorCopy(cpu_tensor, place, out);
Copy link
Contributor

Choose a reason for hiding this comment

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

可以尝试使用phi下的copy_kernel来替换fluid下的TensorCopy

double,
int64_t,
int,
paddle::platform::float16) {}
Copy link
Contributor

Choose a reason for hiding this comment

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

这里换用phi::dtype::float16

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已修改,感谢

phi::SizeKernel,
int,
int64_t,
paddle::platform::float16,
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/backends/gpu/gpu_context.h"
#include "paddle/phi/core/kernel_registry.h"
#include "paddle/phi/kernels/impl/pixel_shuffle_grad_kernel_impl.h"
#include "paddle/phi/kernels/pixel_shuffle_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.

kernel声明头文件在最前面,可以和其他的宏空行隔开

#include "paddle/phi/backends/gpu/gpu_context.h"
#include "paddle/phi/core/kernel_registry.h"
#include "paddle/phi/kernels/impl/pixel_shuffle_kernel_impl.h"
#include "paddle/phi/kernels/pixel_shuffle_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.

同上

phi::SizeKernel,
int,
int64_t,
paddle::platform::float16,
Copy link
Contributor

Choose a reason for hiding this comment

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

同上,命名空间更新下

auto dx_dims = dx->dims();

DenseTensor t;
t.ShareDataWith(*dout);
Copy link
Contributor

Choose a reason for hiding this comment

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

这里感觉原来就写的比较trick,现在ShareDataWith其实就是浅拷贝,所以可以改成直接copy一个tensor,例如:DenseTensor t(*dout)

auto o_dims = out->dims();

DenseTensor t;
t.ShareDataWith(*in);
Copy link
Contributor

Choose a reason for hiding this comment

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

同上

// See the License for the specific language governing permissions and
// limitations under the License.

// Unless required by applicable law or agreed to in writing, software
Copy link
Contributor

Choose a reason for hiding this comment

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

好像重复了一部分license

// See the License for the specific language governing permissions and
// limitations under the License.

// Unless required by applicable law or agreed to in writing, software
Copy link
Contributor

Choose a reason for hiding this comment

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

同上

// See the License for the specific language governing permissions and
// limitations under the License.

// Unless required by applicable law or agreed to in writing, software
Copy link
Contributor

Choose a reason for hiding this comment

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

同上

chenwhql
chenwhql previously approved these changes Feb 23, 2022
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

@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

@chenwhql chenwhql merged commit 639675d into PaddlePaddle:develop Feb 25, 2022
@0x45f 0x45f deleted the pten_eye branch February 25, 2022 10:38
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.

4 participants