Skip to content

Comments

Add conv3d/pool3d/conv3d_trans Python API#11437

Merged
chengduoZH merged 5 commits intoPaddlePaddle:developfrom
chengduoZH:Add_conv3d_Python_API
Jun 15, 2018
Merged

Add conv3d/pool3d/conv3d_trans Python API#11437
chengduoZH merged 5 commits intoPaddlePaddle:developfrom
chengduoZH:Add_conv3d_Python_API

Conversation

@chengduoZH
Copy link
Contributor

@chengduoZH chengduoZH commented Jun 13, 2018

fix #11433

@chengduoZH chengduoZH requested a review from typhoonzero June 13, 2018 09:28
Copy link
Contributor

Choose a reason for hiding this comment

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

Can use @templatedoc to avoid copying comments, see yu laoshi's PR: #11308

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@typhoonzero Sound great!
But Convlution Layer and Convlution Op are not completely equal.
The equation of Convlution Layer is $Out = \sigma (W \ast X + b)$. But Convlution Op's is $Out = W \ast X$. So their comment should not be the same.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see.

@chengduoZH chengduoZH force-pushed the Add_conv3d_Python_API branch from 0af11ef to 9b13b4c Compare June 13, 2018 13:47
typhoonzero
typhoonzero previously approved these changes Jun 14, 2018
Copy link
Contributor

@typhoonzero typhoonzero left a comment

Choose a reason for hiding this comment

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

LGTM++, 2d and 3d layers have a lot common codes, we may need to put them to some base class then.

Copy link
Contributor

@typhoonzero typhoonzero left a comment

Choose a reason for hiding this comment

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

LGTM again

@chengduoZH chengduoZH merged commit f4dce56 into PaddlePaddle:develop Jun 15, 2018
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.

3D conv is needed in python-api

2 participants