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.

Sign up for GitHub

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

Jump to bottom

【PaddlePaddle Hackathon 3 No.20】为 Paddle 新增 vsplit API #44853

Merged
merged 7 commits into from
Sep 9, 2022
Merged

【PaddlePaddle Hackathon 3 No.20】为 Paddle 新增 vsplit API #44853

merged 7 commits into from
Sep 9, 2022

Conversation

Asthestarsfalll
Copy link
Contributor

@Asthestarsfalll Asthestarsfalll commented Aug 3, 2022 •
edited by Ligoml
Loading

PR types

New features

PR changes

APIs

Describe

rfc: PaddlePaddle/community#166
中文API文档：PaddlePaddle/docs#5213

vsplit将输入按照给定的sections或num数量在垂直轴上（行上）划分，等效于将splitAPI的axis固定为0.

@paddle-bot
Copy link

paddle-bot bot commented Aug 3, 2022

你的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.

@paddle-bot paddle-bot bot added contributor External developers status: proposed labels Aug 3, 2022
@paddle-bot
Copy link

paddle-bot bot commented Aug 3, 2022 •
edited
Loading

✅ This PR's description meets the template requirements!
Please wait for other CI results.

@Asthestarsfalll Asthestarsfalll mentioned this pull request Aug 3, 2022
Closed
@Asthestarsfalll
Copy link
Contributor Author

@tizhou86
你好，CI已全部通过，可以开始review了吗？

@tizhou86
Copy link
Member

tizhou86 commented Aug 5, 2022

@tizhou86 你好，CI已全部通过，可以开始review了吗？

OK，我这儿尽快review

tizhou86
tizhou86 reviewed Aug 12, 2022
View reviewed changes
Copy link
Member

@tizhou86 tizhou86 left a comment

Choose a reason for hiding this comment

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

整体OK，劳烦添加注释明确测试对应case。

python/paddle/fluid/tests/unittests/test_splits_api.py Show resolved Hide resolved
python/paddle/fluid/tests/unittests/test_splits_api.py Show resolved Hide resolved
python/paddle/fluid/tests/unittests/test_splits_api.py Show resolved Hide resolved
python/paddle/fluid/tests/unittests/test_splits_api.py Show resolved Hide resolved
tizhou86
tizhou86 reviewed Aug 19, 2022
View reviewed changes
Copy link
Member

@tizhou86 tizhou86 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

需要在init.py中添加新增函数，可以参考该实现：https://github.com/PaddlePaddle/Paddle/pull/44568/files ，添加完成后，需要重新跑一下ci。

@Asthestarsfalll
Copy link
Contributor Author

@tizhou86 已更新

tizhou86
tizhou86 previously approved these changes Aug 22, 2022
View reviewed changes
Copy link
Member

@tizhou86 tizhou86 left a comment

Choose a reason for hiding this comment

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

LGTM

@Ligoml
Copy link
Contributor

Ligoml commented Aug 22, 2022

请补充中文API文档哈

@Asthestarsfalll
Copy link
Contributor Author

已添加

SigureMo
SigureMo requested changes Aug 29, 2022
View reviewed changes
python/paddle/tensor/manipulation.py Outdated Show resolved Hide resolved
python/paddle/fluid/tests/unittests/test_splits_api.py Outdated Show resolved Hide resolved
@SigureMo SigureMo mentioned this pull request Sep 2, 2022
Merged
SigureMo
SigureMo approved these changes Sep 2, 2022
View reviewed changes
Copy link
Member

@SigureMo SigureMo 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~~~

@Ligoml
Copy link
Contributor

Ligoml commented Sep 2, 2022

预览工具无法正常构建出文档，目前还在排查原因，需要等一段时间

SigureMo
SigureMo requested changes Sep 2, 2022
View reviewed changes
python/paddle/tensor/manipulation.py Outdated
print(out0.shape) # [1, 6, 7]
print(out1.shape) # [3, 6, 7]
print(out2.shape) # [4, 6, 7]
out0, out1, out2 = paddle.vplit(x, num_or_sections=[2, 3, -1])
Copy link
Member

Choose a reason for hiding this comment

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

预览工具无法正常构建出文档，目前还在排查原因，需要等一段时间

应该是因为某种原因 docstring 没被解析出来，中文文档那面的 COPY-FROM 也是失败的，而且这里 vplit 明显是个 typo，CI 通过了的话说明这个示例并没有跑

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

Choose a reason for hiding this comment

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

重新编译后正常，我这边没有问题了

Ligoml
Ligoml approved these changes Sep 8, 2022
View reviewed changes
Copy link
Contributor

@Ligoml Ligoml 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

jeff41404
jeff41404 approved these changes Sep 9, 2022
View reviewed changes
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

@jeff41404 jeff41404 merged commit 3304e34 into PaddlePaddle:develop Sep 9, 2022
@SigureMo SigureMo mentioned this pull request Sep 22, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SigureMo SigureMo SigureMo requested changes

@Ligoml Ligoml Ligoml approved these changes

@jeff41404 jeff41404 jeff41404 approved these changes

@tizhou86 tizhou86 tizhou86 left review comments

Assignees

@tizhou86 tizhou86

@luotao1 luotao1

@Ligoml Ligoml

Labels
contributor External developers PaddlePaddle Hackathon
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Asthestarsfalll @tizhou86 @Ligoml @jeff41404 @SigureMo @luotao1