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 #51406

Merged
merged 57 commits into from
May 9, 2023

Conversation

zrr1999
Copy link
Member

@zrr1999 zrr1999 commented Mar 9, 2023

PR types

New features

PR changes

APIs

Description

Add paddle.sparse.sum
document pr: PaddlePaddle/docs#5846
rfc pr: PaddlePaddle/community#411

  • 基础要求
  • coo格式keep_dim参数支持
  • csr格式keep_dim参数支持(因为csr只支持2D或者3D格式,所以keep_dim为False时只会将Tensor变为2D)
  • dtype参数支持
  • 静态图

特殊说明
对于coo格式的稀疏张量,即使keep_dim为False,得到的结果张量的sparse_dim以及dense_dims.size()也会不小于1。
即当x.shape=[5,5],x.sparse_dim=1时,keep_dim=False
y=sum(x, axis=0)
z=sum(x, axis=1)
得到
y.shape=[1,5],y.sparse_dim=1
z.shape=[5,1],z.sparse_dim=1

@paddle-bot
Copy link

paddle-bot bot commented Mar 9, 2023

你的PR提交成功,感谢你对开源项目的贡献!
请关注后续CI自动化测试结果,详情请参考Paddle-CI手册
Your PR has been submitted. Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

zkh2016
zkh2016 previously approved these changes Apr 27, 2023
umiswing
umiswing previously approved these changes Apr 27, 2023
jeff41404
jeff41404 previously approved these changes Apr 27, 2023
Copy link
Contributor

@jeff41404 jeff41404 left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -35,6 +35,7 @@
from .unary import rad2deg
from .unary import expm1
from .unary import transpose
from .unary import sum
from .unary import reshape
from .unary import isnan
Copy link
Contributor

Choose a reason for hiding this comment

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

__all__ = 55行是不是少加了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.

好的,已添加

Copy link
Contributor

Choose a reason for hiding this comment

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

__all__里面加上sum,否则文档不会暴露在官网

__all__ = [
    'sparse_coo_tensor',
    ...
]

Copy link
Contributor

Choose a reason for hiding this comment

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

并且请提供中文api文档pr链接

Copy link
Member Author

Choose a reason for hiding this comment

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

好的,已添加
PaddlePaddle/docs#5846

@zrr1999
Copy link
Member Author

zrr1999 commented Apr 28, 2023

@luotao1 麻烦帮忙看下这个ci时怎么回事,重试了好几次依然还是错误,也重新push了几次把develop最新的内容也合入了。civerage是正常的,只有build那几个一直失败

@luotao1
Copy link
Contributor

luotao1 commented Apr 29, 2023

你可以贴一下build成功/失败的diff,目前CI develop的build是正常的,所以需要看下diff是什么,修改了哪些代码,commit太多了,看不出来。

@zrr1999
Copy link
Member Author

zrr1999 commented Apr 30, 2023

你可以贴一下build成功/失败的diff,目前CI develop的build是正常的,所以需要看下diff是什么,修改了哪些代码,commit太多了,看不出来。

之前应该是rebase的问题导致commit太多了,我现在回退到了4e5b515,也就是合入最新代码之前的部分,这部分只比正常通过的27a1cca 多了一行 'sum',

@PaddlePaddle PaddlePaddle locked and limited conversation to collaborators May 4, 2023
@PaddlePaddle PaddlePaddle unlocked this conversation May 4, 2023
@luotao1
Copy link
Contributor

luotao1 commented May 5, 2023

image

挂的地方和本PR无关,待我们排查下。

Copy link
Contributor

@sunzhongkai588 sunzhongkai588 left a comment

Choose a reason for hiding this comment

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

一些文档细节需要进行修复,仅对文档处修改即可
739b5a8f7dd6c65d4d61143c93d8c9d4


Returns:
Tensor: Results of summation operation on the specified axis of input Tensor `x`,
if `x.dtype='bool'`, `x.dtype='int32'`, it's data type is `'int64'`,
Copy link
Contributor

Choose a reason for hiding this comment

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

建议改成

  • if x.dtype='bool' or x.dtype='int32' ,否则容易造成误解。
  • 上一句的末尾改成句号吧

Copy link
Member Author

@zrr1999 zrr1999 May 8, 2023

Choose a reason for hiding this comment

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

好的,已修改

sunzhongkai588
sunzhongkai588 previously approved these changes May 9, 2023
Copy link
Contributor

@sunzhongkai588 sunzhongkai588 left a comment

Choose a reason for hiding this comment

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

LGTM

@luotao1
Copy link
Contributor

luotao1 commented May 9, 2023

image

@zrr1999 辛苦更新下示例代码

@zrr1999
Copy link
Member Author

zrr1999 commented May 9, 2023

image

@zrr1999 辛苦更新下示例代码

好的,已修改

@luotao1 luotao1 merged commit 14c642c into PaddlePaddle:develop May 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API contributor External developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants