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

【Hackathon 6th No.26】为 paddle.view 进行功能增强 -part #64205

Merged
merged 17 commits into from May 17, 2024
Merged

【Hackathon 6th No.26】为 paddle.view 进行功能增强 -part #64205

merged 17 commits into from May 17, 2024

Conversation

yinfan98
Copy link
Contributor

PR Category

Others

PR Types

Improvements

Description

增强paddle.view,使其能支持动态推理shape

@paddle-bot Paddle-bot
Copy link

paddle-bot bot commented May 10, 2024

你的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 the contributor External developers label May 10, 2024
@yinfan98
Copy link
Contributor Author

#63134 之前的PR已实现 XavierNormal gain

@luotao1 luotao1 mentioned this pull request May 11, 2024
Open
@zhwesky2010 zhwesky2010 changed the title 【Hackathon 6th No.26】为 paddle.view/paddle.nn.initializer.XavierNormal/XavierUniform /KaimingNormal/KaimingUniform 进行功能增强 为 paddle.view 进行功能增强 May 11, 2024
@zhwesky2010 zhwesky2010 changed the title 为 paddle.view 进行功能增强 【Hackathon 6th No.26】为 paddle.view 进行功能增强 May 11, 2024
@yinfan98
Copy link
Contributor Author

yinfan98 commented May 11, 2024 •
edited

b）view 的 shape_or_dtype 参数 不支持 paddle 原生数据类型，如 padlde.int32 作为输入。

试了一下 感觉是支持paddle.xx这种作为输入的。另外我全局搜了一下test。似乎没搜到view 的 test。请问还需要在哪里补充一下测试用例么

@yinfan98
Copy link
Contributor Author

yinfan98 commented May 13, 2024 •
edited

@luotao1 @zhwesky2010 hi, 我似乎没在test里找到关于view的test。我需要为-1的shape推导单独写一个test么。如果需要我该写在什么位置呢。谢谢～(我已经测试过，并确定shape推导没问题了)

@zhwesky2010
Copy link
Contributor

zhwesky2010 commented May 14, 2024 •
edited

@luotao1 @zhwesky2010 hi, 我似乎没在test里找到关于view的test。我需要为-1的shape推导单独写一个test么。如果需要我该写在什么位置呢。谢谢～(我已经测试过，并确定shape推导没问题了)

在test_stride.py里面，在这个里面加一个case吧：https://github.com/PaddlePaddle/Paddle/blob/develop/test/deprecated/legacy_test/test_stride.py#L519-L557

@zhwesky2010
Copy link
Contributor

@yinfan98 看一下PR-CI-APPROVAL 的失败原因，需要使用 np.tesing.assert 来测试

zhwesky2010
zhwesky2010 previously approved these changes May 15, 2024
View reviewed changes
zhwesky2010
zhwesky2010 reviewed May 15, 2024
View reviewed changes
paddle/phi/kernels/stride/view_kernel.cc Outdated
auto new_size = 1;
auto numel = input.numel();
std::vector<int64_t> dims_copy = dims;
for (int dim = 0, ndim = dims_copy.size(); dim != ndim; ++dim) {
Copy link
Contributor

Choose a reason for hiding this comment

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

这个不是dim<ndim吗

paddle/phi/kernels/stride/view_kernel.cc Show resolved Hide resolved
paddle/phi/kernels/stride/view_kernel.cc Outdated
}
}
if (infer_dim >= 0 && new_size > 0 && numel % new_size == 0) {
if (new_size == 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

这个异常在前面的 numel % new_size 不会被触发吗，应该在前面就PADDLE_ENFOCE_NEQ

@zhwesky2010 zhwesky2010 self-requested a review May 15, 2024 11:48
zhwesky2010
zhwesky2010 previously approved these changes May 16, 2024
View reviewed changes
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

wanghuancoder
wanghuancoder previously approved these changes May 16, 2024
View reviewed changes
Copy link
Contributor

@wanghuancoder wanghuancoder left a comment

Choose a reason for hiding this comment

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

LGTM

@yinfan98 yinfan98 dismissed stale reviews from wanghuancoder and zhwesky2010 via f29c41a May 16, 2024 05:54
@yinfan98
Copy link
Contributor Author

在python api接口添加了代码示例，以便docs能同步更新~

zhwesky2010
zhwesky2010 reviewed May 16, 2024
View reviewed changes
python/paddle/tensor/manipulation.py Outdated

>>> x = paddle.rand([2, 4, 6], dtype="float32")

>>> out = paddle.view(x, paddle.uint8")
Copy link
Contributor

Choose a reason for hiding this comment

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

这一行代码无法运行

xuxinyi389
xuxinyi389 reviewed May 16, 2024
View reviewed changes
test/deprecated/legacy_test/test_stride.py
np.testing.assert_allclose(x.numpy(), x_np)

# shape inference
out = paddle.view(x, [10, 100, -1])
Copy link
Contributor

Choose a reason for hiding this comment

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

可以适当增加下测试case，当前测试形状推导是4维推导3维。可以扩充到2，1维情况，或是低维到高维。

zhwesky2010
zhwesky2010 previously approved these changes May 16, 2024
View reviewed changes
@yinfan98
add view test
d2dd00d 
add dim4 -> dim2
add dim4 -> dim1
add dim4 -> dim5
@luotao1 luotao1 changed the title 【Hackathon 6th No.26】为 paddle.view 进行功能增强 【Hackathon 6th No.26】为 paddle.view 进行功能增强 -part May 17, 2024
@yinfan98
Copy link
Contributor Author

yinfan98 commented May 17, 2024 •
edited

静态检查没过，请问是因为我在view 的 api接口的注释里使用print了吗？这种情况下我需要修改哪里或者如何通过一下嘞

@luotao1
Copy link
Contributor

luotao1 commented May 17, 2024

静态检查没过，请问是因为我在view 的 api接口的注释里使用print了吗

这个没有关系，最后review通过后，可以豁免

@yinfan98
Copy link
Contributor Author

这个没有关系，最后review通过后，可以豁免

get～谢谢。
FYI：在昨天的基础上，添加了1. 四维到二维 2. 四维到一维 3. 四维到五维（低维到高维）的形状推导test。
请再review下我的pr吧 @zhwesky2010 @xuxinyi389 @wanghuancoder ，谢谢

@xuxinyi389
Copy link
Contributor

LGTM

zhwesky2010
zhwesky2010 approved these changes May 17, 2024
View reviewed changes
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 2219256 into PaddlePaddle:develop May 17, 2024
31 checks passed
@yinfan98 yinfan98 deleted the view_enhance branch May 17, 2024 11:00
co63oc pushed a commit to co63oc/Paddle that referenced this pull request May 18, 2024
@yinfan98 @co63oc
【Hackathon 6th No.26】为 paddle.view 进行功能增强 -part (PaddlePaddle#64205)
3fff1ec 
* support infer dims

* fix error

* fix error

* Update view_kernel.cc

* fix code format

* Update test_stride.py

* using np.testing.assert_allclose

* fix lint

* fix div0 error

* using PADDLE_ENFORCE_NE

* fix lint

* update python api

* update python api

* fix view test typo

* add view test

add dim4 -> dim2
add dim4 -> dim1
add dim4 -> dim5

* fix lint
co63oc pushed a commit to co63oc/Paddle that referenced this pull request May 19, 2024
@yinfan98 @co63oc
【Hackathon 6th No.26】为 paddle.view 进行功能增强 -part (PaddlePaddle#64205)
de53e6f 
* support infer dims

* fix error

* fix error

* Update view_kernel.cc

* fix code format

* Update test_stride.py

* using np.testing.assert_allclose

* fix lint

* fix div0 error

* using PADDLE_ENFORCE_NE

* fix lint

* update python api

* update python api

* fix view test typo

* add view test

add dim4 -> dim2
add dim4 -> dim1
add dim4 -> dim5

* fix lint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xuxinyi389 xuxinyi389 xuxinyi389 left review comments

@zhwesky2010 zhwesky2010 zhwesky2010 approved these changes

@wanghuancoder wanghuancoder wanghuancoder left review comments

Assignees

@luotao1 luotao1

@zhwesky2010 zhwesky2010

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

Successfully merging this pull request may close these issues.

None yet
5 participants
@yinfan98 @zhwesky2010 @luotao1 @xuxinyi389 @wanghuancoder