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

add crop_tensor_op #19314

Merged
merged 1 commit into from
Sep 20, 2019
Merged

Conversation

zhangting2020
Copy link
Contributor

@zhangting2020 zhangting2020 commented Aug 20, 2019

add crop_tensor op. The main difference with crop is :

  • If the argument shape is a list, each element is an integer or a tensor variable with shape: [1]. This way is suitable for the case that the shape may be changed each iteration.
  • If the argument shape is a variable. Its rank must be 1. In crop op, the rank of shape must be the same as x
  • offsets can be a list, in which each element is an integer or a tensor variavle with shape: [1].

@zhangting2020
Copy link
Contributor Author

zhangting2020 commented Aug 21, 2019

document preview

  • modified the doc of crop:
    image

  • add the doc of crop_tensor:
    image
    image

Copy link
Contributor

@heavengate heavengate left a comment

Choose a reason for hiding this comment

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

Please add python API test in test_layers.py

@@ -0,0 +1,269 @@
/* Copyright (c) 2018 PaddlePaddle Authors. All Rights Reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

2018 -> 2019

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.

  • add python API test in test_layers.py
  • modified Copyright (c) 2018 to Copyright (c) 2019 of 4 files: crop_tensor_op.cc、crop_tensor_op.cu、crop_tensor_op.h、test_crop_tensor_op.py

@zhangting2020 zhangting2020 force-pushed the crop_tensor branch 3 times, most recently from b39085c to 5bc8cf2 Compare August 30, 2019 05:55
heavengate
heavengate previously approved these changes Aug 30, 2019
using framework::OperatorWithKernel::OperatorWithKernel;

void InferShape(framework::InferShapeContext *ctx) const override {
PADDLE_ENFORCE(ctx->HasInput("X"),
Copy link
Contributor

Choose a reason for hiding this comment

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

PADDLE_ENFORCE都需要改为其他接口,PADDLE_ENFORCE_GT等

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.

assert unk_dim_idx == -1, (
"Only one element in shape can be unknown.")
assert dim_idx == 0, (
"Only the first element in shape can be -1.")
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.

第一维是batch size,可以感觉输入得到。在其他维度如果出现-1,crop_tensor操作并不能根据已知的维度推断这个-1所在的维度应该是多少。原有的crop同样不支持第一维之外的其他维度为-1。

Copy link
Collaborator

Choose a reason for hiding this comment

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

我们的batch size 一定要第一维么。。还是这个op就有这种要求

Copy link
Contributor Author

Choose a reason for hiding this comment

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

目前保持原始crop的逻辑。-1的意义已和佳军沟通,进一步确认需求后再进行修改。本次PR只修改参数支持tensor

lanxianghit
lanxianghit previously approved these changes Aug 30, 2019
@@ -8971,6 +8975,152 @@ def crop(x, shape=None, offsets=None, name=None):
return out


def crop_tensor(x, shape=None, offsets=None, name=None):
Copy link
Collaborator

Choose a reason for hiding this comment

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

新接口就叫做crop_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.

是的

element of list can be a integer or a tensor Variable with shape: [1].
This way is suitable for the case that the shape may be changed each
iteration.
offsets (Variable|list/tuple of integer|None): Specifies the cropping
Copy link
Collaborator

Choose a reason for hiding this comment

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

offset 不可以为 list(Variable)么

Copy link
Contributor Author

Choose a reason for hiding this comment

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

修改后已支持offset为list(Variable)

assert unk_dim_idx == -1, (
"Only one element in shape can be unknown.")
assert dim_idx == 0, (
"Only the first element in shape can be -1.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

我们的batch size 一定要第一维么。。还是这个op就有这种要求


.. code-block:: text

* Case 1:
Copy link
Collaborator

Choose a reason for hiding this comment

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

是不是可以给一个3D的例子

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • 这个op的应用场景主要是图像领域,batch size通常在第一维
  • 在文档中添加了3-D的例子

@jiangjiajun
Copy link
Collaborator

jiangjiajun commented Sep 2, 2019

测试了Python接口,offsets使用存在一些问题,已反馈。

@zhangting2020
Copy link
Contributor Author

测试了Python接口,offsets使用存在一些问题,已反馈。

已增加offset支持list(variable)

phlrain
phlrain previously approved these changes Sep 9, 2019
Copy link
Collaborator

@phlrain phlrain left a comment

Choose a reason for hiding this comment

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

LGTM

lanxianghit
lanxianghit previously approved these changes Sep 9, 2019
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

heavengate
heavengate previously approved these changes Sep 11, 2019
raindrops2sea
raindrops2sea previously approved these changes Sep 12, 2019
Copy link
Collaborator

@raindrops2sea raindrops2sea left a comment

Choose a reason for hiding this comment

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

LGTM

heavengate
heavengate previously approved these changes Sep 16, 2019
@phlrain phlrain self-requested a review September 16, 2019 02:57
phlrain
phlrain previously approved these changes Sep 16, 2019
lanxianghit
lanxianghit previously approved these changes Sep 16, 2019
lanxianghit
lanxianghit previously approved these changes Sep 17, 2019
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

Copy link
Collaborator

@raindrops2sea raindrops2sea left a comment

Choose a reason for hiding this comment

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

LGTM

@Aurelius84 Aurelius84 merged commit b388894 into PaddlePaddle:develop Sep 20, 2019
mapingshuo pushed a commit to mapingshuo/Paddle that referenced this pull request Sep 20, 2019
…#19314)

add crop_tensor op. The main difference with crop is :

1. If the argument shape is a list, each element is an integer or a tensor variable with shape: [1]. This way is suitable for the case that the shape may be changed each iteration.

2. If the argument shape is a variable. Its rank must be 1. In crop op, the rank of shape must be the same as x

offsets can be a list, in which each element is an integer or a tensor variavle with shape: [1].
@zhangting2020 zhangting2020 deleted the crop_tensor branch October 21, 2019 09:29
seiriosPlus pushed a commit to seiriosPlus/Paddle that referenced this pull request Dec 9, 2019
…#19314)

add crop_tensor op. The main difference with crop is :

1. If the argument shape is a list, each element is an integer or a tensor variable with shape: [1]. This way is suitable for the case that the shape may be changed each iteration.

2. If the argument shape is a variable. Its rank must be 1. In crop op, the rank of shape must be the same as x

offsets can be a list, in which each element is an integer or a tensor variavle with shape: [1].
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.

7 participants