| CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 5.8k
【TJU】【Paddle Tensor 第二期 API支持 0-size Tensor】paddle.broadcast_to 支持 0-size tensor #70125
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
【TJU】【Paddle Tensor 第二期 API支持 0-size Tensor】paddle.broadcast_to 支持 0-size tensor #70125
Conversation
|
你的PR提交成功，感谢你对开源项目的贡献! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个单测好像挂了，可以看一下，
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个文件的修改可能是有问题的，会导致maskselect报错
可以构造上述的输入尝试复现看看
| auto diff = expand_shape.size() - x.dims().size(); | ||
| auto out_shape = common::vectorize<int64_t>(x.dims()); | ||
| out_shape.insert(out_shape.begin(), diff, 1); | ||
| auto vec_in_dims = common::vectorize<int>(in_dims); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dims必须用int64_t，不能用int
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
好的
| out_shape[i] = expand_shape[i]; | ||
| if (expand_shape[i] == -1) { | ||
| out_shape[i] = vec_in_dims[i]; | ||
| } else if (expand_shape[i] == 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
expand_shape[i]为0的话，是不是需要检查indims是不是0，如果非0，则应该报错
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK，我检查一下，其他设备好像也改了这里
| expand_shape[i])); | ||
| out_shape[i] = expand_shape[i]; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resize完毕之后，需要再再次检查一下out_shape内是否含有0，含0则不需要执行下面的broadcast，Alloc直接返回
| expand_shape[i])); | ||
| repeat_times[i] = expand_shape[i]; | ||
| if (expand_shape[i] == 0) { | ||
| repeat_times[i] = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
有这种0的可以用一个flag记录下来
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个文件不需要改动
| @@ -102,19 +115,22 @@ def test_api_fp16_gpu(self): | |||
| out_1 = paddle.broadcast_to(x, shape=[12, 14]) | |||
| out_2 = paddle.broadcast_to(x, shape=[positive_2, 14]) | |||
| out_3 = paddle.broadcast_to(x, shape=expand_shape) | |||
| out_4 = paddle.broadcast_to(x_zero, shape=[0, 14]) | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
添加尽可能多的单测，比如对0-size/非0-size Tensor进行expand，expand shape为0-size shape/非0-size shape，以及构造错误的expand场景。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
以上都明白了
| expand_shape[i], | ||
| 0, | ||
| common::errors::InvalidArgument("The expanded size cannot be zero.")); | ||
| if (i < diff) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里为什么要把i<diff的逻辑给删掉呢？尽量不删除原有逻辑比较好？
| out_shape[i] = vec_in_dims[i]; | ||
| } else if (expand_shape[i] == 0) { | ||
| PADDLE_ENFORCE_EQ( | ||
| vec_in_dims[i] == 1 || vec_in_dims[i] == expand_shape[i], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
out_dims=0的情况下，vec_in_dims[i]能等于1吗？可以用torch或者numpy确认下，我理解也应该等于0吧？
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pytorch是支持1的
| if (expand_shape[i] == 0) { | ||
| PADDLE_ENFORCE_EQ( | ||
| vec_in_dims[i] == 1 || vec_in_dims[i] == expand_shape[i], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
同上
| @@ -66,20 +71,23 @@ def test_api(self): | |||
| out_1 = paddle.broadcast_to(x, shape=[12, 14]) | |||
| out_2 = paddle.broadcast_to(x, shape=[positive_2, 14]) | |||
| out_3 = paddle.broadcast_to(x, shape=expand_shape) | |||
| out_4 = paddle.broadcast_to(x_zero, shape=[0, 14]) | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
broadcast_to的shape和原shape相同，单测太简单了，至少要包含broadcast前、后各含有/不含有0的case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个文件不要改动
…into test_broadcast_to
|
@fxy1699 有个单测没过： |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
PR Category
User Experience
PR Types
Bug fixes
Description
报错如下
工作
对expand_kernel的cpu/gpu/xpu进行了修改支持0-size tensor
添加了cpu/gpu对0-size tensor的单测支持
测试
api-test通过
单测通过
