-
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
【Hackathon 4th No.24】为 Paddle 新增 paddle.sparse.is_nan 稀疏 API #51513
Conversation
你的PR提交成功,感谢你对开源项目的贡献! |
@@ -170,6 +171,20 @@ SparseCooTensor ReluCsr(const Context& dev_ctx, const SparseCooTensor& x) { | |||
return csr; | |||
} | |||
|
|||
template <typename T, typename Context> | |||
SparseCooTensor IsnanCoo(const Context& dev_ctx, const SparseCooTensor& x) { |
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.
这个额外地C++ API可以删掉,一般用kernel表示C++ api就可以
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.
你这个删掉没
没有支持静态图,可能需要 @zkh2016 帮忙看下静态图的应该怎么写 |
coo格式的静态图单测还是建议加下。test_sparse_norm_op.py和test_sparse_conv_op.py有测试例子,可以先用这种方式测下,验下正确性。 |
@zhouwei25 @zkh2016 这个算子有点特殊, 计算结果是bool型, 在to_dense或return_numpy=True的时候,都会报错。辛苦两位老师给个建议。 InvalidArgumentError: The type of data we are trying to retrieve (float32) does not match the type of data (bool) currently contained in the container. |
当前to_dense还没注册bool类型,你可以先注册一个,测测看。 |
@zhouwei25 @zkh2016 有个不明白的地方,虽然我使用的是IsfiniteInferMeta, 返回的稀疏张量的dtype应该是bool才对,但是打印出来仍旧是float类型,所以定位在下面的内存或显存分配会发生报错,因为x的dtype是bool类型,但是模板T是float:
|
可能需要辛苦你进一步定位下,我看了下当前是通过DEFINE_SPARSE_UNARY_KERNEL这个宏定义is_nan的kernel的,这里面重新创建了out,所以类型可能变了。 |
@zhouwei25 @zkh2016 辛苦两位老师再review一下 |
args : (Tensor x) | ||
output : Tensor(out) | ||
infer_meta : | ||
func : IsfiniteInferMeta |
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.
IsfiniteInferMeta吗?最新提交已修改成unchanged
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.
@zhouwei25 如果改成unchanged,还是会报错,最新提交又修改回IsfiniteInferMeta
InvalidArgumentError: The type of data we are trying to retrieve (float32) does not match the type of data (bool) currently contained in the container.
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.
@zhouwei25 如果改成unchanged,还是会报错,最新提交又修改回IsfiniteInferMeta InvalidArgumentError: The type of data we are trying to retrieve (float32) does not match the type of data (bool) currently contained in the container.
就是命名风格有点问题,这个不是IsNanInferMeta吗
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.
@zhouwei25 IsfiniteInferMeta是参考dense tensor那里设计的IsfiniteInferMeta, 考虑到代码可能会冗余就这么直接复用了,老师建议这个地方是需要单独写一个IsNanInferMeta吗
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.
PD_REGISTER_INFER_META_FN(isnan, phi::IsfiniteInferMeta);
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.
好复用也可以
@@ -170,6 +171,20 @@ SparseCooTensor ReluCsr(const Context& dev_ctx, const SparseCooTensor& x) { | |||
return csr; | |||
} | |||
|
|||
template <typename T, typename Context> | |||
SparseCooTensor IsnanCoo(const Context& dev_ctx, const SparseCooTensor& x) { |
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.
你这个删掉没
} | ||
|
||
template <typename T, typename Context> | ||
SparseCooTensor IsnanCsr(const Context& dev_ctx, const SparseCooTensor& x) { |
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就可以
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.
已删掉
@@ -219,5 +220,44 @@ void CastCsrKernel(const Context& dev_ctx, | |||
} | |||
} | |||
|
|||
template <typename T, typename Context> | |||
void IsnanCooKernel(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.
这个用目前的公共组件,宏函数来注册kernel。可以复用减少代码
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.
@zhouwei25 公共组件不能满足这个算子,因为emptylike会创建一个相同类型的输出tensor,而这个算子输出是bool型的,所以这里单独写了个kernel。
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.
@zhouwei25 公共组件不能满足这个算子,因为emptylike会创建一个相同类型的输出tensor,而这个算子输出是bool型的,所以这里单独写了个kernel。
OK
…nto is_nan_sparse
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
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
python/paddle/sparse/unary.py
Outdated
Return whether every element of input tensor is `NaN` or not, requiring x to be a SparseCooTensor or SparseCsrTensor. | ||
|
||
Args: | ||
x (Tensor): The input tensor, it's data type should be float16, float32, float64, int32, int64. |
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.
中文文档中 '可以为 Coo 或 Csr 格式' 可以表达出来
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.
@sunzhongkai588 已修改
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
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 for CI-OP-Benchmark
PR types
New features
PR changes
APIs
Describe
完成第四期第24项目开发任务: https://github.com/PaddlePaddle/community/blob/master/hackthon_4th/%E3%80%90PaddlePaddle%20Hackathon%204%E3%80%91%20%E6%A0%B8%E5%BF%83%E6%A1%86%E6%9E%B6%E5%BC%80%E6%BA%90%E8%B4%A1%E7%8C%AE%20API%20%E5%BC%80%E5%8F%91%E4%BB%BB%E5%8A%A1%E5%90%88%E9%9B%86.md#task24
isnan 检查输入Tensor 的每一个值是否为 +/-NaN, 并返回布尔型结果。目前在 PaddlePaddle 中,对于稀疏Tensor还没有支持isnan的API。
RFC设计文档: PaddlePaddle/community#415
中文api文档:PaddlePaddle/docs#5705