-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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 isnan_v2、isfinite_v2、isinf_v2 to phi #40076
Conversation
Thanks for your contribution! |
@@ -0,0 +1,95 @@ | |||
// Copyright (c) 2022 PaddlePaddle Authors. All Rights Reserved. |
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.
我看了下底层的TensorContainsInfV2这几个实现,还是属于cpu和gpu公用的实现,这样的话,这个还是属于应该在cpu和gpu子目录下分别创建cc和cu文件的情况,kernels根目录下是放置纯设备无关的kernel实现,比如说这个也需要在xpu和将来的其他设备上能用才行,目前的实现是不行的
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.
@chenwhql Done.已在cpu和gpu子目录下分别创建cc和cu文件
#define DEFINE_ISFINITE_SR(isfinite_sr, functor) \ | ||
template <typename T, typename Context> \ | ||
void isfinite_sr( \ | ||
const Context& ctx, const SelectedRows& x, SelectedRows* out) { \ | ||
IsfiniteSRImpl<T, Context, functor>(ctx, x, out); \ | ||
} | ||
|
||
DEFINE_ISFINITE_SR(IsinfSR, funcs::InfinityV2Functor) | ||
DEFINE_ISFINITE_SR(IsnanSR, funcs::NANV2Functor) | ||
DEFINE_ISFINITE_SR(IsfiniteSR, funcs::IsfiniteV2Functor) | ||
#undef DEFINE_ISFINITE_SR |
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.
SelectedRows相关的kernel逻辑需要放在selected_rows目录下
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.
Done.
paddle/phi/kernels/isfinite_kernel.h
Outdated
#define DEFINE_ISFINITE_SR(isfinite_sr) \ | ||
template <typename T, typename Context> \ | ||
void isfinite_sr( \ | ||
const Context& ctx, const SelectedRows& x, SelectedRows* out); | ||
|
||
DEFINE_ISFINITE_SR(IsinfSR) | ||
DEFINE_ISFINITE_SR(IsnanSR) | ||
DEFINE_ISFINITE_SR(IsfiniteSR) | ||
#undef DEFINE_ISFINITE_SR |
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.
同上
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.
Done.
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
e8daa44
@@ -0,0 +1,39 @@ | |||
// Copyright (c) 2022 PaddlePaddle Authors. All Rights Reserved. |
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 phi { | ||
|
||
template <typename T, typename Context, typename Functor> | ||
inline void IsfiniteSRImpl(const Context& dev_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.
SelectedRows在这几个kernel上,建议直接调用DenseTensor的kernel,而不是单独为它实现一个类似的,参考现有的几个SelectedRows kernel
#include "paddle/phi/core/kernel_registry.h" | ||
#include "paddle/phi/kernels/selected_rows/isfinite_kernel_impl.h" | ||
|
||
namespace phi { |
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.
selected_rows目录下命名空间加一层sr,kernel名也不需要加SR的中缀了,参考其他几个SelectedRows kernel
@@ -14,39 +14,32 @@ | |||
|
|||
#pragma once | |||
|
|||
#include <vector> | |||
|
|||
#include "paddle/fluid/framework/eigen.h" | |||
#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.
op_registry这个头文件还需要吗
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.
几处comment麻烦追加PR完善
PR types
Function optimization
PR changes
OPs
Describe
迁移phi算子:isnan_v2、isfinite_v2、isinf_v2
1、包括两个kernel的实现:densetensor以及selectedrows
2、将selected_rows kernel相关的声明头文件调整到selected_rows目录下
3、多加一层传入functor