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

Add 3D convolution transposed operator. #5118

Merged
merged 11 commits into from
Nov 6, 2017
Merged

Add 3D convolution transposed operator. #5118

merged 11 commits into from
Nov 6, 2017

Conversation

chengduoZH
Copy link
Contributor

@chengduoZH chengduoZH commented Oct 26, 2017 •
edited
Loading

fix #5116

@chengduoZH chengduoZH force-pushed the Add_deconv3d_op branch 2 times, most recently from 6dc08d9 to e7ff9a7 Compare October 26, 2017 07:09
@zchen0211
Copy link
Contributor

Generally looks good. Is this the C3D model as in http://vlg.cs.dartmouth.edu/c3d/?

@chengduoZH
Copy link
Contributor Author

Is this the C3D model as in http://vlg.cs.dartmouth.edu/c3d/?

Yes. Conv3D and DeConv3D are needed in video processing or medical image processing. This requirement was put forward by IDL medical image processing team.

@qingqing01 qingqing01 changed the title Add deconv3d op Add 3D convolution transposed operator. Oct 31, 2017
@zchen0211
Copy link
Contributor

I see. Just had a look and still looks pretty good to me.

zchen0211
zchen0211 previously approved these changes Oct 31, 2017
View reviewed changes
@zchen0211
Copy link
Contributor

Approved from my side. Maybe Qingqing and Daoyuan could have a double check :)

qingqing01
qingqing01 requested changes Nov 2, 2017
View reviewed changes
paddle/operators/conv_transpose_op.h Outdated
const int c = output->dims()[1]; // output channels
const int o_d = output->dims()[2];
const int o_h = output->dims()[3];
const int o_w = output->dims()[4];
Copy link
Contributor

Choose a reason for hiding this comment

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

DDim里存的是int64_t，如果编译有warning的话，可以用int64_t类型。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

编译没有warning，不过最好还是改一下

paddle/operators/conv_transpose_op.h Outdated

// vol2col: (c * d * h * w, k_d * k_h * k_w)
vol2col(context.device_context(), output_grad_batch, col, strides[0],
strides[1], strides[2], paddings[0], paddings[1], paddings[2]);
Copy link
Contributor

Choose a reason for hiding this comment

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

line 396 和 line 431的if分支分开写，导致调用了两次 vol2col，这样多了一次计算。两个分支可以合并。

if ( (!input_grad) && (! filter_grad)) {
  return;
}
下面计算~

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

@chengduoZH chengduoZH force-pushed the Add_deconv3d_op branch 4 times, most recently from 047876f to 5f36177 Compare November 6, 2017 10:50
qingqing01
qingqing01 approved these changes Nov 6, 2017
View reviewed changes
Copy link
Contributor

@qingqing01 qingqing01 left a comment

Choose a reason for hiding this comment

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

LGTM.

@qingqing01 qingqing01 merged commit 43a64a7 into PaddlePaddle:develop Nov 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qingqing01 qingqing01 qingqing01 approved these changes

@zchen0211 zchen0211 zchen0211 left review comments

@hedaoyuan hedaoyuan Awaiting requested review from hedaoyuan

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

Successfully merging this pull request may close these issues.

Add deconv3d op
3 participants
@chengduoZH @zchen0211 @qingqing01