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

New style convolution_2d and deconvolution_2d #3163

Merged
merged 2 commits into from
Aug 24, 2017

Conversation

beam2d
Copy link
Member

@beam2d beam2d commented Aug 16, 2017

This PR implements the new style version of F.convolution_2d and F.deconvolution_2d. These functions are now twice differentiable (but not three times differentiable until some functions used in backward, e.g. F.sum, support double backward).

This PR depends on #3096, and is a part of #3147.

@beam2d beam2d added backport Pull request that is backported from a more recent version. cat:bug Bug report or fix. labels Aug 16, 2017
Copy link
Member

@okuta okuta left a comment

Choose a reason for hiding this comment

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

Please check travis result.

else:
return func(x, W, b)
func = Deconvolution2DFunction(stride, pad, outsize)
args = x, W
Copy link
Member

Choose a reason for hiding this comment

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

How about following code?

if b is None:
    args = x. W
else:
    args = x, W, b

@beam2d beam2d mentioned this pull request Aug 21, 2017
27 tasks
@beam2d beam2d added cat:feature Implementation that introduces new interfaces. and removed backport Pull request that is backported from a more recent version. cat:bug Bug report or fix. labels Aug 21, 2017
@okuta
Copy link
Member

okuta commented Aug 23, 2017

Please rebase or merge master

@okuta okuta added this to the v3.0.0rc1 milestone Aug 23, 2017
@okuta okuta self-assigned this Aug 23, 2017
@okuta
Copy link
Member

okuta commented Aug 23, 2017

LGTM except comment.

@beam2d
Copy link
Member Author

beam2d commented Aug 24, 2017

Rebased.

**kwargs):
cover_all = None
conv_desc = None
filter_desc = None
Copy link
Member

@okuta okuta Aug 24, 2017

Choose a reason for hiding this comment

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

Why do you cache this descriptor?

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't change the behavior of caching descriptors. Is it better to remove it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Note: the above change is just about the default values for these attributes.

Copy link
Member

Choose a reason for hiding this comment

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

I think deleting it is okay.

@beam2d
Copy link
Member Author

beam2d commented Aug 24, 2017

Removed the descriptors.

@okuta
Copy link
Member

okuta commented Aug 24, 2017

LGTM!

@okuta okuta merged commit a1d03da into chainer:master Aug 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cat:feature Implementation that introduces new interfaces.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants