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

【Hackathon 4th No.24】为 Paddle 新增 paddle.sparse.is_nan 稀疏 API #415

Merged
merged 11 commits into from
Mar 13, 2023

Conversation

thunder95
Copy link
Contributor

为 Paddle 新增 paddle.sparse.is_nan 稀疏 API

@paddle-bot
Copy link

paddle-bot bot commented Mar 5, 2023

你的PR提交成功,感谢你对开源项目的贡献!
请检查PR提交格式和内容是否完备,具体请参考示例模版
Your PR has been submitted. Thanks for your contribution!
Please check its format and content. For this, you can refer to Template and Demo.

## 1、相关背景

is_nan 是一个检查输入稀疏 Tensor 的每一个值是否为 +/-NaN 。目前在 PaddlePaddle 种,没有稀疏涨量的is_nan计算逻辑。
针对 Paddle 的两种稀疏 Tensor 格式 COO 与 CSR ,都需新增 reshape 的计算逻辑,一共需要新增 2个 kernel 的前向与反向。
Copy link
Contributor

Choose a reason for hiding this comment

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

reshape?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已修改


## 1、相关背景

is_nan 是一个检查输入稀疏 Tensor 的每一个值是否为 +/-NaN 。目前在 PaddlePaddle 种,没有稀疏涨量的is_nan计算逻辑。
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 Author

Choose a reason for hiding this comment

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

已修改


## 1、相关背景

is_nan 是一个检查输入稀疏 Tensor 的每一个值是否为 +/-NaN 。目前在 PaddlePaddle 种,没有稀疏涨量的is_nan计算逻辑。
Copy link
Contributor

Choose a reason for hiding this comment

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

PaddlePaddle中

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已修改


# 四、对比分析

Pytorch, Tensorflow, Scipy虽然对于稠密张量支持is_nan操作,但都没有直接实现对稀疏张量的 is_nan 的一元操作。
Copy link
Contributor

Choose a reason for hiding this comment

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

torch支持稀疏的is_nan判断

Copy link
Contributor Author

Choose a reason for hiding this comment

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

谢谢指导,已修改


## 命名与参数设计

sparse is_nan 这个稀疏张量上的方法的命名和参数不需要额外设计,在 paddle/phi/api/yaml 下新增注册该算子的前向和反向。
Copy link
Contributor

Choose a reason for hiding this comment

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

需要写下API命名和参数设计

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已修改

## 命名与参数设计

sparse is_nan 这个稀疏张量上的方法的命名和参数不需要额外设计,在 paddle/phi/api/yaml 下新增注册该算子的前向和反向。

Copy link
Contributor

Choose a reason for hiding this comment

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

paddle/phi/api/yaml/sparse_ops.yaml中注册,需要描述一下yaml形式

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已修改


## 底层OP设计

新增两个 Kernel:
Copy link
Contributor

Choose a reason for hiding this comment

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

kernel形式写IsnanCooKernel就可以,参数不对

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已修改

}
```

在前向推理中,一元操作的实现较简单,取出 DenseTensor 类型的 non_zero_elements() 后,逐元素进行 is_nan 操作,并创建新的稀疏张量即可。
Copy link
Contributor

Choose a reason for hiding this comment

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

这个API建议参考下unary_kernel.cc,看能否批量注册复用dense 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.

可以复用dense kernel

在 Paddle repo 的 python/paddle/sparse/unary.py 文件中新增api:

```cpp
@dygraph_only
Copy link
Contributor

Choose a reason for hiding this comment

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

新增的sparse API需要同时实现动态图和静态图

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已修改


# 六、测试和验收的考量

测试考虑的case如下:
Copy link
Contributor

Choose a reason for hiding this comment

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

具体有哪些case设计呢

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已修改

func : isnan_coo{sparse_coo -> sparse_coo},
isnan_csr{sparse_csr -> sparse_csr}
layout : x
backward : null
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 Author

Choose a reason for hiding this comment

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

已修改

@@ -0,0 +1,124 @@
# paddle.sparse.is_nan 设计文档
Copy link
Contributor

Choose a reason for hiding this comment

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

这个API的名字调整成 paddle.sparse.isnan 吧,和dense的风格对齐,后面涉及到名字的都需要改下

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已修改

args : (Tensor x)
output : Tensor(out)
infer_meta :
func : IsfiniteInferMeta
Copy link
Contributor

Choose a reason for hiding this comment

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

UnchangedInferMeta,后面直接复用sparse/unary.cc中的实现吧,参考relu的实现

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已修改

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zhouwei25 这个跟其他unary算子还不太一样,因为返回的是bool类型数据,是否UnchangedInferMeta就不太合适了,譬如这种错是不是是因为infer_meta造成的:
return _C_ops.sparse_to_dense(self)
ValueError: (InvalidArgument) The type of data we are trying to retrieve (float32) does not match the type of data (bool) currently contained in the container. 

Copy link
Contributor

Choose a reason for hiding this comment

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

@zhouwei25 这个跟其他unary算子还不太一样,因为返回的是bool类型数据,是否UnchangedInferMeta就不太合适了,譬如这种错是不是是因为infer_meta造成的: return _C_ops.sparse_to_dense(self) ValueError: (InvalidArgument) The type of data we are trying to retrieve (float32) does not match the type of data (bool) currently contained in the container.

嗯这个确实特殊些

Copy link
Contributor

@zhwesky2010 zhwesky2010 left a comment

Choose a reason for hiding this comment

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

LGTM

@zhwesky2010 zhwesky2010 merged commit 81e6bb6 into PaddlePaddle:master Mar 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants