Skip to content
This repository was archived by the owner on Jul 1, 2023. It is now read-only.

_vjpConv2DBackpropInput using shape instead of using filter size for … #331

Merged
merged 3 commits into from
Jul 3, 2019

Conversation

sjaz24
Copy link
Contributor

@sjaz24 sjaz24 commented Jul 3, 2019

…filterSizes argument.

@@ -184,8 +184,8 @@ func _vjpConv2DBackpropInput<Scalar: TensorFlowFloatingPoint>(
) -> (Tensor<Scalar>, (Tensor<Scalar>) -> (Tensor<Scalar>, Tensor<Scalar>)) {
let value = conv2DBackpropInput(x, shape: shape, filter: filter,
strides: strides, padding: padding, dilations: dilations)
return (value, { v in
(conv2DBackpropFilter(x, input: v, filterSizes: shape, strides: strides,
return (value, { [filterShape = filter.shapeTensor] v in
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually this pullback is already capturing filter (on line 190), so I think your original implementation is better! I reverted it for you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!

@rxwei rxwei merged commit 8e1a71d into tensorflow:master Jul 3, 2019
@Shashi456
Copy link
Contributor

Shashi456 commented Jul 3, 2019

@sjaz24, would this solve #288?

@sjaz24
Copy link
Contributor Author

sjaz24 commented Jul 3, 2019

As I mentioned in my comment on #288, I'm not having issues with the original code as it was before your change. Your test passes for me. The issue I had was with the calculation of the gradients during backprop which this PR fixed. I'm wondering if your issue was with your original test. It looks like you might have had Conv2D instead of TransposeConv2D. Have you tried running your updated test against the original code?

sjaz24 added a commit to sjaz24/swift-apis that referenced this pull request Jul 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants