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

[Hackthon 4th No.7] add paddle.unflatten API #53055

Merged
merged 20 commits into from
May 9, 2023

Conversation

Ainavo
Copy link
Contributor

@Ainavo Ainavo commented Apr 18, 2023

@paddle-bot
Copy link

paddle-bot bot commented Apr 18, 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.

@Ainavo
Copy link
Contributor Author

Ainavo commented Apr 18, 2023

你好 @zoooo0820,对于 RFC 中提到的 shape 参数为 Tensor 数据类型,torch 中是不支持 Tensor 数据类型的,如下图,所以 paddle 中对于 shape 参数,需要支持 Tensor 类型吗?
image

不过,相对于 torch.reshape 不支持参数 shapeTensor 的类型,而 paddle.reshape 支持参数 shapeTensor 的类型~

@paddle-bot paddle-bot bot added the contributor External developers label Apr 18, 2023
@zoooo0820
Copy link
Contributor

你好 @zoooo0820,对于 RFC 中提到的 shape 参数为 Tensor 数据类型,torch 中是不支持 Tensor 数据类型的,如下图,所以 paddle 中对于 shape 参数,需要支持 Tensor 类型吗? image

不过,相对于 torch.reshape 不支持参数 shapeTensor 的类型,而 paddle.reshape 支持参数 shapeTensor 的类型~

@Ainavo 你好,
在Paddle的设计中,需要综合考虑相关竞品及Paddle本身的情况。在这个API的调研过程中,收集到了基础信息表示,即"pytorch的shape不支持Tensor,但是TF支持"。以及上面提到的,paddle.reshape支持shape为Tensor。
从这些信息来看,支持与否其实是不强制的,如果支持更多类型的输入,可提升API易用性,是锦上添花的工作。所以这里需要特别评估实现支持shape是Tensor类型在开发上的难度,如果难度不大,则最好支持,如果会额外带来很大的开发量,甚至发现框架本身从机制上不支持等问题,那么也可以指出来,放弃支持这个点。

@Ainavo
Copy link
Contributor Author

Ainavo commented Apr 19, 2023

你好@zoooo0820paddle.reshape 在静态图模式下,类型检查不包括 unit8complex64comple128bfloat16类型检查如下图:
image
请问,这里可以直接添加 unit8complex64comple128bfloat16 的类型支持吗?或者可以在 unflatten 单测中不对这些类型进行静态图模式下的测试?

@Ainavo
Copy link
Contributor Author

Ainavo commented Apr 19, 2023

你好 @zoooo0820,对于 RFC 中提到的 shape 参数为 Tensor 数据类型,torch 中是不支持 Tensor 数据类型的,如下图,所以 paddle 中对于 shape 参数,需要支持 Tensor 类型吗? image
不过,相对于 torch.reshape 不支持参数 shapeTensor 的类型,而 paddle.reshape 支持参数 shapeTensor 的类型~

@Ainavo 你好, 在Paddle的设计中,需要综合考虑相关竞品及Paddle本身的情况。在这个API的调研过程中,收集到了基础信息表示,即"pytorch的shape不支持Tensor,但是TF支持"。以及上面提到的,paddle.reshape支持shape为Tensor。 从这些信息来看,支持与否其实是不强制的,如果支持更多类型的输入,可提升API易用性,是锦上添花的工作。所以这里需要特别评估实现支持shape是Tensor类型在开发上的难度,如果难度不大,则最好支持,如果会额外带来很大的开发量,甚至发现框架本身从机制上不支持等问题,那么也可以指出来,放弃支持这个点。

收到

@zoooo0820
Copy link
Contributor

这里可以先暂时不加,如果要加的话还涉及到补单测等工作,工作重心可能偏离。数据类型支持后续由对应专项工作去补充。目前可以测试已经支持的数据类型。

@Ainavo
Copy link
Contributor Author

Ainavo commented Apr 19, 2023

这里可以先暂时不加,如果要加的话还涉及到补单测等工作,工作重心可能偏离。数据类型支持后续由对应专项工作去补充。目前可以测试已经支持的数据类型

好的

@Ainavo
Copy link
Contributor Author

Ainavo commented Apr 21, 2023

args.Name args.Type is_support Notes
x (Tensor) uint8 仅仅进行动态图下的测试,因为 paddle.reshape 静态图下不支持该类型。
int8 仅仅进行动态图下的测试,因为 paddle.reshape 静态图下不支持该类型。
int16
int32
int64
float16
float32
float64
bfloat16 Numpy 中不支持 bfloat16 数据类型,paddle.to_tensor 创建的 bfloat16 数据类型貌似有问题,跟 torch 端对不齐。
complex64 仅仅进行动态图下的测试,因为 paddle.reshape 静态图下不支持该类型。
complex128 仅仅进行动态图下的测试,因为 paddle.reshape 静态图下不支持该类型。
bool
shape (List or Tuple) int
shape (Tensor) uint8 paddle.reshape 不支持 uint8 的数据类型。
int8 paddle.reshape 不支持 uint8 的数据类型。
int16 paddle.reshape 不支持 uint8 的数据类型。
in32
in64 paddle.shape 不支持 int64,但是可以通过 paddle.castint64 输入的数据类型转换成 int32
axis(int) int test value : -1, 0, 1

@Ainavo
Copy link
Contributor Author

Ainavo commented Apr 21, 2023

另外一个 pr 先将 shape 输入为 paddle.Tensor 的数据类型使用 list 直接转换成列表,然后再进行后续处理:

@Ainavo
Copy link
Contributor Author

Ainavo commented Apr 21, 2023

@zoooo0820 你好,ci 已过,有时间可以 review 一下吗?
因为 numpy 没有对应的 API 进行实现,因此在 test_unflatten.py 文件中大致实现了一个(基本上跟实现的 paddle.unflatten 逻辑相同),在本地跟 torch.unflatten 对单测中的一些例子进行了对齐,目前没发现问题,不知道这么做是否可行?对于某些不支持的数据类型也已在上表中提到,同时还有另一个 pr ,将输入的 shape 参数先转换成 list 再进行之后的处理,希望得到 review 的修改建议~

@luotao1
Copy link
Contributor

luotao1 commented Apr 25, 2023

4、产生任务 leader:第一个通过设计文档评审的开发者将成为该任务的 leader,后续任务开发需按照设计文档进行;若其他开发者先于任务 leader 提交了作品,则他/她的作品需经过任务 leader 的确认(自@任务leader review 后,7个自然日内无回复默认通过),未按照设计文档开发的作品将不会进入评审阶段。

https://aistudio.baidu.com/aistudio/competition/detail/776/0/introduction

cos43 review 下这个PR,同时关注下上述规则(by 2023年4月25日)。

@Ainavo
Copy link
Contributor Author

Ainavo commented Apr 25, 2023

可以请 @cos43 review 一下吗~

python/paddle/fluid/tests/unittests/test_layers.py Outdated Show resolved Hide resolved
python/paddle/nn/layer/common.py Outdated Show resolved Hide resolved
python/paddle/tensor/manipulation.py Outdated Show resolved Hide resolved
python/paddle/tensor/manipulation.py Outdated Show resolved Hide resolved
python/paddle/tensor/manipulation.py Outdated Show resolved Hide resolved
python/paddle/fluid/tests/unittests/test_unflatten.py Outdated Show resolved Hide resolved
python/paddle/tensor/manipulation.py Outdated Show resolved Hide resolved
if axis < 0:
axis = axis + length
new_shape = (
tuple(x.shape[:axis]) + tuple(shape) + tuple(x.shape[axis + 1 :])
Copy link
Contributor

Choose a reason for hiding this comment

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

支持shape 为Tensor,本质上不是单纯为了增加一个支持的数据类型,而是Tensor是一个未知的数据(例如shape是需要由前面某个OP计算出来)。

在静态图下,在组网构图阶段,是不能利用Tensor内的实际数值的,所以对于静态图而言,new_shape这样先转tuple再组合起来的方案是不适用的。这个地方,尽量保证new_shape始终由tensor计算得到,并仍然作为一个tensor给到reshape

Copy link
Contributor Author

Choose a reason for hiding this comment

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

支持shape 为Tensor,本质上不是单纯为了增加一个支持的数据类型,而是Tensor是一个未知的数据(例如shape是需要由前面某个OP计算出来)。

在静态图下,在组网构图阶段,是不能利用Tensor内的实际数值的,所以对于静态图而言,new_shape这样先转tuple再组合起来的方案是不适用的。这个地方,尽量保证new_shape始终由tensor计算得到,并仍然作为一个tensor给到reshape

目前,先使用 paddle.shape 获取为 Tensor 数据的 shape,然后通过 paddle.concat 进行张量连接,由于 paddle.shape 返回的数据类型是 int32,因此在 shape 输入时,添加了 int32 的数据类型检查。同时 paddle.reshape文档中提到 shape 参数的类型为 int32 (不过 int64 也可以正常输入)。

@Ainavo Ainavo requested a review from zoooo0820 April 28, 2023 06:56
python/paddle/tensor/manipulation.py Outdated Show resolved Hide resolved
python/paddle/tensor/manipulation.py Outdated Show resolved Hide resolved
python/paddle/tensor/manipulation.py Outdated Show resolved Hide resolved
python/paddle/tensor/manipulation.py Show resolved Hide resolved
python/paddle/nn/layer/common.py Outdated Show resolved Hide resolved
python/paddle/fluid/tests/unittests/test_unflatten.py Outdated Show resolved Hide resolved
python/paddle/fluid/tests/unittests/test_unflatten.py Outdated Show resolved Hide resolved
python/paddle/fluid/tests/unittests/test_unflatten.py Outdated Show resolved Hide resolved
@Ainavo
Copy link
Contributor Author

Ainavo commented May 4, 2023

@zoooo0820 已按照建议进行修改,其中 paddle.shape 返回的数据类型指定为 int32,而 paddle.reshape 可以支持 int32int64,因此当 int64 的数据类型输入时,使用 paddle.cast 强制转换成 int32,不知道这样做是否合适?

@Ainavo Ainavo requested a review from zoooo0820 May 5, 2023 03:19
zoooo0820
zoooo0820 previously approved these changes May 5, 2023
Copy link
Contributor

@zoooo0820 zoooo0820 left a comment

Choose a reason for hiding this comment

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

LGTM

@paddle-ci-bot
Copy link

paddle-ci-bot bot commented May 7, 2023

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

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.

unflatten也同步进行修改

Parameters:
shape (list|tuple|Tensor): Unflatten :attr:`shape` on the specified :attr:`axis`. At most one dimension of the target :attr:`shape` can be -1.
If the input :attr:`shape` does not contain -1 , the product should be equal to ``x.shape[axis]`` size.
The data type is `int` . If :attr:`shape` is a list or tuple, the elements of it should be integers or Tensors with shape [].
Copy link
Contributor

Choose a reason for hiding this comment

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

@zoooo0820 麻烦判断下Tensors with shape []描述是否准确?感觉 形状为[]的tensor 很难界定

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zoooo0820 麻烦判断下Tensors with shape []描述是否准确?感觉 形状为[]的tensor 很难界定

补充:这里是参考了 paddle.reshape 的文档

The data type is ``int32`` . If ``shape`` is a list or tuple, the elements of it should be integers or Tensors with shape [].

感觉这里想表达的意思是 元素为 Tensor 的 list 或 tuple
image

Copy link
Contributor

Choose a reason for hiding this comment

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

@zoooo0820 麻烦判断下Tensors with shape []描述是否准确?感觉 形状为[]的tensor 很难界定

补充:这里是参考了 paddle.reshape 的文档

The data type is ``int32`` . If ``shape`` is a list or tuple, the elements of it should be integers or Tensors with shape [].

感觉这里想表达的意思是 元素为 Tensor 的 list 或 tuple
image

和内部研发同学问了下,形状为[]的tensor为意思是 0维tensor,应该符合描述

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zoooo0820 麻烦判断下Tensors with shape []描述是否准确?感觉 形状为[]的tensor 很难界定

补充:这里是参考了 paddle.reshape 的文档

The data type is ``int32`` . If ``shape`` is a list or tuple, the elements of it should be integers or Tensors with shape [].

感觉这里想表达的意思是 元素为 Tensor 的 list 或 tuple
image

和内部研发同学问了下,形状为[]的tensor为意思是 0维tensor,应该符合描述

好的,学到了~

python/paddle/nn/layer/common.py Outdated Show resolved Hide resolved
@@ -4787,6 +4787,75 @@ def index_add_(x, index, axis, value, name=None):
return _C_ops.index_add_(x, index, value, axis)


def unflatten(x, shape, axis, name=None):
Copy link
Contributor

Choose a reason for hiding this comment

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

对应竞品的API,从复现模型的易用角度出发,这里以及Unflatten类中的shape, axis的前后顺序可调整下,对应RFC也辛苦修改一下呢

Copy link
Contributor Author

Choose a reason for hiding this comment

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

对应竞品的API,从复现模型的易用角度出发,这里以及Unflatten类中的shape, axis的前后顺序可调整下,对应RFC也辛苦修改一下呢

好的

Copy link
Contributor Author

@Ainavo Ainavo May 8, 2023

Choose a reason for hiding this comment

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

对应竞品的API,从复现模型的易用角度出发,这里以及Unflatten类中的shape, axis的前后顺序可调整下,对应RFC也辛苦修改一下呢

修改后的 RFC 链接:

主要修改参数顺序及参数说明,另外删除一些单测中的错误检测

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

Copy link
Contributor

@zoooo0820 zoooo0820 left a comment

Choose a reason for hiding this comment

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

LGTM

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 9cd0a5b into PaddlePaddle:develop May 9, 2023
23 of 24 checks passed
@Ainavo Ainavo deleted the api_for_unflatten branch May 9, 2023 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API contributor External developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants