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

[phi] Move clip op to phi #40602

Merged
merged 11 commits into from
Apr 2, 2022
Merged

[phi] Move clip op to phi #40602

merged 11 commits into from
Apr 2, 2022

Conversation

wuyefeilin
Copy link
Contributor

@wuyefeilin wuyefeilin commented Mar 15, 2022

PR types

Others

PR changes

OPs

Describe

Move clip op to phi

@paddle-bot-old
Copy link

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

paddle/phi/kernels/clip_kernel.h Outdated Show resolved Hide resolved
paddle/phi/kernels/cpu/clip_grad_kernel.cc Outdated Show resolved Hide resolved
paddle/phi/kernels/cpu/clip_kernel.cc Outdated Show resolved Hide resolved
paddle/phi/kernels/gpu/clip_grad_kernel.cu Outdated Show resolved Hide resolved
paddle/phi/kernels/gpu/clip_kernel.cu Outdated Show resolved Hide resolved

#include "paddle/phi/backends/gpu/gpu_context.h"
#include "paddle/phi/core/kernel_registry.h"
#include "paddle/phi/kernels/clip_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头文件在前,空行隔开

paddle/phi/kernels/selected_rows/clip_kernel.cc Outdated Show resolved Hide resolved
paddle/phi/kernels/selected_rows/clip_kernel.cu Outdated Show resolved Hide resolved
paddle/phi/ops/compat/clip_sig.cc Show resolved Hide resolved
paddle/phi/kernels/impl/clip_grad_kernel_impl.h Outdated Show resolved Hide resolved
paddle/phi/kernels/impl/clip_grad_kernel_impl.h Outdated Show resolved Hide resolved
paddle/phi/kernels/impl/clip_kernel_impl.h Outdated Show resolved Hide resolved
@wuyefeilin
Copy link
Contributor Author

Fix as review

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.

CE-framework还在挂,可能需要拉下PaddleTest repo,找到失败的脚本本地调试下


#include "paddle/phi/backends/all_context.h"
#include "paddle/phi/core/kernel_registry.h"
#include "paddle/phi/kernels/clip_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头文件在前面

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


#include "paddle/phi/backends/all_context.h"
#include "paddle/phi/core/kernel_registry.h"
#include "paddle/phi/kernels/clip_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头文件在前

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,66 @@
// Copyright (c) 2022 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.

这个也是cpu gpu共用的吗,放在selected_rows子目录的impl文件下?selected_rows下和上一层目录的组织方式一样,看起来这个在impl,然后cc在cpu,cu在gpu

@@ -0,0 +1,30 @@
// Copyright (c) 2022 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.

同上

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 c065804 into PaddlePaddle:develop Apr 2, 2022
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