-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
[pten] trans diagonal kernel into pten #39575
Conversation
Thanks for your contribution! |
@@ -12,262 +12,4 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | |||
See the License for the specific language governing permissions and | |||
limitations under the License. */ | |||
|
|||
#include "paddle/fluid/framework/op_registry.h" | |||
#include "paddle/fluid/operators/diagonal_op.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个cu文件是不是可以直接删除?
paddle/fluid/operators/diagonal_op.h
Outdated
@@ -16,148 +16,3 @@ | |||
#include <algorithm> | |||
#include <vector> | |||
#include "paddle/fluid/framework/op_registry.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个.h也直接删除吧
paddle/pten/kernels/cpu/diagonal.h
Outdated
namespace pten { | ||
|
||
template <typename T> | ||
std::vector<T> ComputeDimStride(const std::vector<T> dim) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kernels/funcs下面已经有一个diagonal.h了,这个函数能否一起整合过去?减少一些重名的头文件
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个函数本身看起来和diagonal没有关系,个人觉得这是一个shape相关的计算函数,或许直接放到的funcs/common_shape.h更合适
paddle/pten/kernels/gpu/diagonal.h
Outdated
|
||
namespace pten { | ||
template <typename T, int X_DIM_SIZE, int OUT_DIM_SIZE> | ||
__global__ void Diagonal(const T* data1, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
建议整合到funcs/diagonal.h中
#include "paddle/fluid/framework/tensor_util.h" | ||
#include "paddle/fluid/platform/device/gpu/gpu_primitives.h" | ||
#include "paddle/pten/core/kernel_registry.h" | ||
#include "paddle/pten/kernels/diagonal_grad_kernel.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kernel头文件include放到最前面
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
#include "paddle/fluid/framework/tensor_util.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个头文件麻烦check一下是必要的吗
#include "paddle/fluid/framework/tensor_util.h" | ||
#include "paddle/fluid/platform/device/gpu/gpu_primitives.h" | ||
#include "paddle/pten/core/kernel_registry.h" | ||
#include "paddle/pten/kernels/diagonal_kernel.h" |
There was a problem hiding this comment.
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. | ||
|
||
#include "paddle/fluid/framework/tensor_util.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
同上
|
||
namespace pten { | ||
|
||
KernelSignature DiagonalOpArgumentMapping(const ArgumentMappingContext& ctx) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
前向的这里不需要写,因为原先的OpMaker就比较规范,会自动正确匹配
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
PR types
Others
PR changes
Others
Describe
Others