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

Refine squeeze, test=develop #25281

Merged
merged 7 commits into from
Jul 23, 2020
Merged

Conversation

zhiqiu
Copy link
Contributor

@zhiqiu zhiqiu commented Jun 30, 2020

PR types

Others

PR changes

APIs

Describe

Enhance paddle.squeeze

  • Update signature
    paddle.squeeze(input, axes, out=None, name=None) -> paddle.squeeze(x, axis=None, name=None)
  • Support None, int, and list for axis
  • Return x unchanged if its shape is [A, 1, B, 1] and axis is 0 instead of raising exception
  • Code clean

@paddle-bot-old
Copy link

Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

@zhiqiu zhiqiu force-pushed the dev/refine_squeeze branch 3 times, most recently from 45ca069 to 3d81973 Compare July 15, 2020 05:37
@zhiqiu zhiqiu requested a review from lanxianghit July 17, 2020 04:28
If axes is negative, :math:`axes=axes+rank(input)`.
axis (int|list, optional): An integer or list of integers, indicating the dimensions to be squeezed. Default is None.
The range of axis is :math:`[-rank(input), rank(input))`.
If axis is negative, :math:`axes=axes+rank(input)`.
Copy link
Contributor

Choose a reason for hiding this comment

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

axis和axes混用

Copy link
Contributor Author

@zhiqiu zhiqiu Jul 20, 2020

Choose a reason for hiding this comment

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

Done, thx.

axes (list): One integer or List of integers, indicating the dimensions to be squeezed.
Axes range is :math:`[-rank(input), rank(input))`.
If axes is negative, :math:`axes=axes+rank(input)`.
axis (int|list, optional): An integer or list of integers, indicating the dimensions to be squeezed. Default is None.
Copy link
Contributor

Choose a reason for hiding this comment

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

不支持tuple吗?

Copy link
Contributor Author

@zhiqiu zhiqiu Jul 20, 2020

Choose a reason for hiding this comment

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

之前是不支持的,已经添加了tuple支持。

@@ -430,83 +431,79 @@ def _get_SectionsTensorList(one_list):
return outs


def squeeze(input, axes, out=None, name=None):
def squeeze(x, axis=None, 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.

与1.8API同名,但是参数列表发生变化了,应该给出旧版API对应关系,可以联系 @TCChenlong 给一下具体写法

Copy link
Contributor Author

Choose a reason for hiding this comment

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

联系@TCChenlong后,添加了update_api的标识。


If axis is provided, it will remove the dimension(s) by given axis that of size 1.
If the dimension of given axis is not of size 1, the dimension remain unchanged.
If axis is not provided, all dims equal of size 1 will be removed.

.. code-block:: text

Case1:
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.

好的

@PaddlePaddle PaddlePaddle locked and limited conversation to collaborators Jul 21, 2020
@PaddlePaddle PaddlePaddle unlocked this conversation Jul 21, 2020
lanxianghit
lanxianghit previously approved these changes Jul 21, 2020
Copy link
Contributor

@lanxianghit lanxianghit left a comment

Choose a reason for hiding this comment

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

LGTM

@zhiqiu zhiqiu requested a review from swtkiwi July 22, 2020 02:58

Input:
X.shape = [1,3,1,5]
axes = [-2]
x.shape = [1, 3, 1, 5] # If axis is negative, axis = axis + rank(input)
Copy link
Contributor

Choose a reason for hiding this comment

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

axis = axis + number of dimensions in x

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

This OP will squeeze single-dimensional entries of input tensor's shape. If axes is provided, will
remove the dims by axes, the dims selected by axes should be one. If not provide axes, all dims equal
to one will be deleted.
This OP will squeeze the dimension(s) of size 1 of input tensor's shape.

Copy link
Contributor

Choose a reason for hiding this comment

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

input tensor -> input tensor x

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Output:
Out.shape = [1,3,5]
out.shape = [1, 3, 5]

Args:
input (Variable): The input Tensor. Support data type: float32, float64, int8, int32, int64.
Copy link
Contributor

Choose a reason for hiding this comment

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

input (Variable) -> x (Tensor)
已经明确了2.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.

好的

If axes is negative, :math:`axes=axes+rank(input)`.
axis (int|list|tuple, optional): An integer or list of integers, indicating the dimensions to be squeezed. Default is None.
The range of axis is :math:`[-rank(input), rank(input))`.
If axis is negative, :math:`axis = axis + rank(input)`.
Copy link
Contributor

Choose a reason for hiding this comment

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

rank -> ndim

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

"""
:alias_main: paddle.squeeze
:alias: paddle.squeeze,paddle.tensor.squeeze,paddle.tensor.manipulation.squeeze
:update_api: paddle.fluid.layers.squeeze
Copy link
Contributor

Choose a reason for hiding this comment

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

根据线下讨论,新的API文档当中去掉这个标签,在旧版本的API文档中增加Note来提示用户。
上面的alias列表里的逗号后增加空格。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好的

Copy link
Contributor

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

@lanxianghit lanxianghit left a comment

Choose a reason for hiding this comment

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

LGTM

@zhiqiu zhiqiu merged commit 4ec1251 into PaddlePaddle:develop Jul 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants