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

Add derivative for 'padded(forSizes:with:)'. #184

Merged
merged 13 commits into from
Jun 10, 2019
Merged

Add derivative for 'padded(forSizes:with:)'. #184

merged 13 commits into from
Jun 10, 2019

Conversation

lakshya-sky
Copy link
Contributor

@lakshya-sky lakshya-sky commented Jun 8, 2019

Resolves #179.

@lakshya-sky
Copy link
Contributor Author

Fix: #179

@dan-zheng dan-zheng requested review from rxwei and dan-zheng June 8, 2019 19:08
@@ -491,6 +502,7 @@ final class BasicOperatorTests: XCTestCase {
("testUnbroadcast1", testUnbroadcast1),
("testUnbroadcast2", testUnbroadcast2),
("testSliceUpdate", testSliceUpdate),
("testVJPPadded", testVJPPadded),
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a test for padded too as part of this PR since it’s missing?

And also maybe move them both right after testGathering in an attempt to keep the order consistent between the test files and the source files?

@@ -674,6 +674,7 @@ extension Tensor where Scalar: TensorFlowFloatingPoint {
public extension Tensor where Scalar: Numeric {
/// Returns a padded tensor according to the specified padding sizes.
@inlinable
@differentiable(wrt:self, vjp: _vjpPadded(forSizes:with:) where Scalar: TensorFlowFloatingPoint)
Copy link
Contributor

Choose a reason for hiding this comment

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

wrt:self -> wrt: self

begin: Tensor<Int32>([0, 0]),
size: Tensor<Int32>(stacking: [rank, Tensor<Int32>(1)]))
let begin = Raw.reshape(padBefore, shape: Tensor<Int32>([-1]))
return Raw.slice(v,begin: begin,size: shape)
Copy link
Contributor

Choose a reason for hiding this comment

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

v,begin -> v, begin

Copy link
Contributor

Choose a reason for hiding this comment

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

begin,size -> begin, size

return (result, { [rank = rankTensor, shape = shapeTensor] v in
let paddings = Tensor<Int32>(
shape: [sizes.count, 2],
scalars: sizes.flatMap {[Int32($0.before), Int32($0.after)] })
Copy link
Contributor

Choose a reason for hiding this comment

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

Space after {.

@@ -466,6 +466,17 @@ final class BasicOperatorTests: XCTestCase {
XCTAssertEqual(target, Tensor(repeating: 1, shape: [2, 3, 4]))
}

func testVJPPadded() {
// 1 -> 2 x 3 x 4
let x = Tensor<Float>(ones:[3,2])
Copy link
Contributor

Choose a reason for hiding this comment

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

In the test function code, could you please add spaces after all the argument labels?

And also between the literals in arrays and tuples?

@lakshya-sky
Copy link
Contributor Author

@eaplatanios All the requested changes are done.

Copy link
Contributor

@rxwei rxwei left a comment

Choose a reason for hiding this comment

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

Thanks!

@rxwei rxwei changed the title Derivative for padded(forSizes:with:). Add derivative for 'padded(forSizes:with:)'. Jun 10, 2019
@rxwei rxwei merged commit 4c0a09c into tensorflow:master Jun 10, 2019
dan12411 pushed a commit to dan12411/swift-apis that referenced this pull request Jun 15, 2019
@lakshya-sky lakshya-sky deleted the pad_deriv branch June 20, 2019 04:45
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.

Define a derivative for 'padded(forSizes:with:)'.
4 participants