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.30】为 Paddle 新增 paddle.sparse.sum 稀疏 API #411

Merged
merged 8 commits into from
Mar 28, 2023

Conversation

@paddle-bot
Copy link

paddle-bot bot commented Mar 4, 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.

@zrr1999
Copy link
Member Author

zrr1999 commented Mar 9, 2023

实现进度

rfcs/APIs/20230222_api_design_for_sparse_sum.md Outdated Show resolved Hide resolved

```

相应的InferMeta函数可以复用稠密矩阵的函数。
Copy link
Contributor

Choose a reason for hiding this comment

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

这个应该没办法简单复用,因为还涉及到一个indices、values的shape

Copy link
Member Author

Choose a reason for hiding this comment

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

这里我好像有些不太理解,我参考了稀疏矩阵另外已经实现的其他几个infermeta函数,

void Conv3dInferMeta(const MetaTensor& x,
                     const MetaTensor& kernel,
                     const std::vector<int>& paddings,
                     const std::vector<int>& dilations,
                     const std::vector<int>& strides,
                     const int groups,
                     const bool subm,
                     const std::string& key,
                     MetaTensor* out,
                     MetaTensor* rulebook,
                     MetaTensor* counter) 

看到输入为metatensor,但其中只有这三个方法

  virtual void set_dims(const DDim& dims);
  virtual void set_dtype(DataType dtype);
  virtual void set_layout(DataLayout layout);

但是indices、values的shape应该是在SparseTensor里的参数,那我是否应该在这里传入SparseTensor,这样的话感觉似乎不太符合infermeta的机制了

rfcs/APIs/20230222_api_design_for_sparse_sum.md Outdated Show resolved Hide resolved
rfcs/APIs/20230222_api_design_for_sparse_sum.md Outdated Show resolved Hide resolved
rfcs/APIs/20230222_api_design_for_sparse_sum.md Outdated Show resolved Hide resolved
rtol=1e-05,
)

def test_sum_2d(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

csr的测试case还需要看下

rfcs/APIs/20230222_api_design_for_sparse_sum.md Outdated Show resolved Hide resolved
SumInferMeta函数 将优先根据dtype。

## 底层OP设计
对于COO格式和CSR格式axis=None的简单情况,只需要把value值求和,并对相应的位置参数进行修改即可。
Copy link
Contributor

Choose a reason for hiding this comment

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

axis=None时,应该就返回一个DenseTensor了,这个时候只有一个元素,也就无所谓Sparse还是Dense了

Copy link
Member Author

Choose a reason for hiding this comment

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

那这里的kernel是不是需要实现两个,一个返回稀疏矩阵一个返回dense矩阵,或者有其他的办法,因为这里的机制我还不太了解,所以在设计时就只考虑了输出只为稀疏矩阵的情况

Copy link
Contributor

@zhwesky2010 zhwesky2010 Mar 15, 2023

Choose a reason for hiding this comment

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

还是返回稀疏Tensor吧,但是只有一个元素的稀疏Tensor

Copy link
Member Author

Choose a reason for hiding this comment

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

我做了一下实验,如果对相应的位置参数修改测试可以正常通过
image
但是如果对相应的位置参数留空todense的时候会不支持float32的错误,但是实际上我的类型是int64
image
image
这里好像比较奇怪

rfcs/APIs/20230222_api_design_for_sparse_sum.md Outdated Show resolved Hide resolved
@@ -0,0 +1,430 @@
# paddlesparse.sum 设计文档
Copy link
Contributor

Choose a reason for hiding this comment

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

paddlesparse.sum 设计文档。这些没写对,尽量避免语法错误

Copy link
Member Author

Choose a reason for hiding this comment

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

好的,这里已修改,刚刚检查了一下,应该没有类似的错误了

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.

5 participants