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

[0 Tensor support] support the 0d tensor for the cumsum #49518

Merged
merged 4 commits into from
Jan 18, 2023
Merged

[0 Tensor support] support the 0d tensor for the cumsum #49518

merged 4 commits into from
Jan 18, 2023

Conversation

wawltor
Copy link
Contributor

@wawltor wawltor commented Jan 3, 2023 •
edited
Loading

PR types

Function optimization

PR changes

OPs

Describe

support the 0d tensor for the cumsum and add_n

@paddle-bot
Copy link

paddle-bot bot commented Jan 3, 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.

@paddle-bot
Copy link

paddle-bot bot commented Jan 3, 2023

❌ The PR is not created using PR's template. You can refer to this Demo.
Please use PR's template, it helps save our maintainers' time so that more developers get helped.

@wawltor wawltor changed the title support the 0d tensor for the cumsum and add_n [0 Tensor support] support the 0d tensor for the cumsum and add_n Jan 5, 2023
zhwesky2010
zhwesky2010 reviewed Jan 5, 2023
View reviewed changes
python/paddle/fluid/tests/unittests/test_zero_dim_tensor.py Outdated
@@ -86,6 +86,7 @@
paddle.lgamma,
paddle.poisson,
paddle.bernoulli,
paddle.cumsum,
Copy link
Contributor

Choose a reason for hiding this comment

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

这个单独写吧，因为None的时候相当于先做一个展平1D，较特殊
然后axis要加检查，0D时axis只能None，0，-1（None时会做展平）

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这个单独写吧，因为None的时候相当于先做一个展平1D，较特殊 然后axis要加检查，0D时axis只能None，0，-1（None时会做展平）

已经单独展开

python/paddle/fluid/tests/unittests/test_zero_dim_tensor.py Outdated
out1.backward()
out2.backward()

self.assertEqual(out1.shape, [])
Copy link
Contributor

Choose a reason for hiding this comment

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

可以再测下广播的，加个x是ND的
反向的shape测试可以再加个x.grad.shape

Copy link
Contributor Author

Choose a reason for hiding this comment

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

可以再测下广播的，加个x是ND的 反向的shape测试可以再加个x.grad.shape

add_n 本身api不支持广播

Copy link
Contributor

Choose a reason for hiding this comment

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

可以再测下广播的，加个x是ND的 反向的shape测试可以再加个x.grad.shape

add_n 本身api不支持广播

OK

paddle/phi/kernels/xpu/cum_kernel.cc Outdated
@@ -30,6 +30,11 @@ void CumsumKernel(const Context& dev_ctx,
using XPUType = typename XPUTypeTrait<T>::Type;
dev_ctx.template Alloc<T>(out);

if (x.dims().size() == 0) {
phi::Copy(dev_ctx, x, dev_ctx.GetPlace(), false, out);
Copy link
Contributor

Choose a reason for hiding this comment

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

XPU的应该得这么写：

r = xpu::copy<XPUTypeT>(xpu_ctx,
                                d_qk_ptr,
                                d_src_mask_out_ptr,
                                batch_size * seq_len * seq_len * num_heads);
PADDLE_ENFORCE_XDNN_SUCCESS(r, "copy");

Copy link
Contributor Author

Choose a reason for hiding this comment

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

XPU的应该得这么写：

r = xpu::copy<XPUTypeT>(xpu_ctx,
                                d_qk_ptr,
                                d_src_mask_out_ptr,
                                batch_size * seq_len * seq_len * num_heads);
PADDLE_ENFORCE_XDNN_SUCCESS(r, "copy");

Done

paddle/phi/infermeta/multiary.cc Outdated
@@ -360,7 +362,11 @@ void AddNInferMeta(const std::vector<const MetaTensor*>& x,
}
}
}
out->set_dims(in_dim);
if (is_0d_tensor) {
out->set_dims(phi::make_ddim({}));
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 Author

Choose a reason for hiding this comment

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

add_n 本身api不支持广播

Copy link
Contributor

Choose a reason for hiding this comment

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

add_n 本身api不支持广播

OK

python/paddle/fluid/tests/unittests/xpu/test_zero_dim_tensor_xpu.py Outdated
@@ -87,6 +87,7 @@
paddle.lgamma,
paddle.poisson,
paddle.bernoulli,
paddle.cumsum,
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 Author

Choose a reason for hiding this comment

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

同上

已经单独展开单测

paddle/phi/infermeta/unary.cc Outdated
@@ -412,14 +412,18 @@ void CumInferMeta(const MetaTensor& x,
bool reverse,
MetaTensor* out) {
auto x_dims = x.dims();
auto x_dims_all = phi::product(x_dims);
Copy link
Contributor

@zhwesky2010 zhwesky2010 Jan 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

目前这个逻辑是对的，如果flatten后应该都为1D
非flatten就和原来shape一致，但是要检查axis，0D较特殊，axis必须[-1, 0]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

目前这个逻辑是对的，如果flatten后应该都为1D 非flatten就和原来shape一致，但是要检查axis，0D较特殊，axis必须[-1, 0]

已经做了axis的check和单测

zhwesky2010
zhwesky2010 reviewed Jan 5, 2023
View reviewed changes
paddle/phi/infermeta/multiary.cc Outdated
@@ -311,6 +312,7 @@ void AddNInferMeta(const std::vector<const MetaTensor*>& x,
}
// for 0D tensor
if (x_dim.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.

写法上可以去掉这里试试，不过可能会触发历史问题

Copy link
Contributor Author

Choose a reason for hiding this comment

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

写法上可以去掉这里试试，不过可能会触发历史问题

会触发历史问题

paddle/phi/infermeta/multiary.cc Outdated
@@ -360,7 +362,11 @@ void AddNInferMeta(const std::vector<const MetaTensor*>& x,
}
}
}
out->set_dims(in_dim);
if (is_0d_tensor) {
out->set_dims(phi::make_ddim({}));
Copy link
Contributor

Choose a reason for hiding this comment

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

add_n 本身api不支持广播

OK

@wawltor wawltor closed this Jan 11, 2023
@wawltor wawltor reopened this Jan 11, 2023
@wawltor wawltor changed the title [0 Tensor support] support the 0d tensor for the cumsum and add_n [0 Tensor support] support the 0d tensor for the cumsum Jan 17, 2023
zhwesky2010
zhwesky2010 approved these changes Jan 18, 2023
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

@wawltor wawltor merged commit 5fca45e into PaddlePaddle:develop Jan 18, 2023
@GGBond8488 GGBond8488 mentioned this pull request Feb 17, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zhwesky2010 zhwesky2010 zhwesky2010 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@wawltor @zhwesky2010