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.26】为 Paddle 新增 paddle.sparse.nn.Softmax 稀疏 API 的 coo 格式计算逻辑 #53613

Merged
merged 46 commits into from
May 25, 2023

Conversation

thunder95
Copy link
Contributor

@thunder95 thunder95 commented May 8, 2023

PR types

New features

PR changes

APIs

Description

针对 Paddle 的稀疏 Tensor 格式 COO,需要新增 softmax 的计算逻辑.

RFC设计文档: PaddlePaddle/community#514
中文api文档:

@paddle-bot
Copy link

paddle-bot bot commented May 8, 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.

@thunder95 thunder95 requested a review from zkh2016 May 18, 2023 02:56
@paddle-ci-bot
Copy link

paddle-ci-bot bot commented May 18, 2023

Sorry to inform you that 8a7b606's CIs have passed for more than 7 days. To prevent PR conflicts, you need to re-run all CIs manually.

@thunder95 thunder95 requested a review from zkh2016 May 21, 2023 08:12
@zkh2016
Copy link
Contributor

zkh2016 commented May 23, 2023

CI-Coverage没过,还需要补充些单测

@thunder95 thunder95 requested a review from zkh2016 May 24, 2023 00:15
@thunder95
Copy link
Contributor Author

CI-Coverage没过,还需要补充些单测

@zkh2016 在反向时, 参考torch实现,有一种情况out_offsets != grad_offsets没有覆盖到,但是现在测试时,始终out_offsets==grad_offsets, 因为out和dout的indices是一样的。麻烦老师给个建议,是否可以删掉这个if分支,或者有没有其他办法可以单独测试这个反向并可以制定反向输入的参数。

@zkh2016
Copy link
Contributor

zkh2016 commented May 24, 2023

CI-Coverage没过,还需要补充些单测

@zkh2016 在反向时, 参考torch实现,有一种情况out_offsets != grad_offsets没有覆盖到,但是现在测试时,始终out_offsets==grad_offsets, 因为out和dout的indices是一样的。麻烦老师给个建议,是否可以删掉这个if分支,或者有没有其他办法可以单独测试这个反向并可以制定反向输入的参数。

行,这个可以先豁免下。

不过看了下反向的kernel好像没优化,看上去应该和前向差不多思路就行,要不也试试优化看?

@thunder95
Copy link
Contributor Author

CI-Coverage没过,还需要补充些单测

@zkh2016 在反向时, 参考torch实现,有一种情况out_offsets != grad_offsets没有覆盖到,但是现在测试时,始终out_offsets==grad_offsets, 因为out和dout的indices是一样的。麻烦老师给个建议,是否可以删掉这个if分支,或者有没有其他办法可以单独测试这个反向并可以制定反向输入的参数。

行,这个可以先豁免下。

不过看了下反向的kernel好像没优化,看上去应该和前向差不多思路就行,要不也试试优化看?

谢谢老师, 已修改 @zkh2016

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 for docs

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

@luotao1 luotao1 merged commit 4ea1d04 into PaddlePaddle:develop May 25, 2023
25 checks passed
@luotao1
Copy link
Contributor

luotao1 commented May 25, 2023

@thunder95 请补充下中文文档

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

7 participants