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

Add Separable Conv2D Layer #362

Merged
merged 6 commits into from
Aug 9, 2019
Merged

Conversation

Shashi456
Copy link
Contributor

@Shashi456 Shashi456 commented Jul 14, 2019

Build Passes.
Will add a test in a while.

The last of Convolution Layer to be added.

@Shashi456
Copy link
Contributor Author

Shashi456 commented Jul 14, 2019

@rxwei mind adding a build to this, I'll not let this be merged until the test is added, just want to confirm its stable till here.

@Shashi456
Copy link
Contributor Author

sorry for being AWOL, I'll take care of the rest of the PRs soon. This passes locally, If you want me to do any other changes let me know.
cc: @eaplatanios , @rxwei

@Shashi456
Copy link
Contributor Author

@eaplatanios or @rxwei could you take a look at this?

@Shashi456
Copy link
Contributor Author

This is ready for review.

Copy link
Member

@dan-zheng dan-zheng left a comment

Choose a reason for hiding this comment

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

Thank you @Shashi456!

It would nice to test derivatives for SeparableConv2D, but that isn't set up yet. Filed #402 to track layer derivative tests.

@Shashi456
Copy link
Contributor Author

@dan-zheng would it be okay, if I wrote the derivative tests in some other PR, I'm not very sure about how to do them, I'll go over your suggestions in the vjpconv2d PR. But I wouldn't want to block this PR because of that. WDYT? Moreover since separable is inherently dependent on Conv2D itself, if we test that then we might not have to test this as well.

@Shashi456
Copy link
Contributor Author

I think this can be tested and merged.

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.

LGTM

@Shashi456
Copy link
Contributor Author

Strange that it failed, I had tested and built it locally.

@Shashi456
Copy link
Contributor Author

Shashi456 commented Aug 7, 2019

@rxwei So the test passed.

Test Case 'LayerTests.testSeparableConv2D' started at 2019-08-07 16:09:58.001
Test Case 'LayerTests.testSeparableConv2D' passed (0.0 seconds)

The error is this :

Fatal error: Incompatible shapes: [6] vs. [4]: file /swift-apis/Sources/TensorFlow/Bindings/EagerExecution.swift, line 299
Current stack trace:
0    libswiftCore.so                    0x00007f84c7360020 swift_reportError + 50
1    libswiftCore.so                    0x00007f84c73cf020 _swift_stdlib_reportFatalErrorInFile + 115
2    libswiftCore.so                    0x00007f84c72f8698 <unavailable> + 3802776
3    libswiftCore.so                    0x00007f84c72f8827 <unavailable> + 3803175
4    libswiftCore.so                    0x00007f84c70b9338 <unavailable> + 1446712
5    libswiftCore.so                    0x00007f84c72c83ee <unavailable> + 3605486
6    libswiftCore.so                    0x00007f84c70b8a59 <unavailable> + 1444441
7    TensorFlowPackageTests.xctest      0x000056364966a140 <unavailable> + 2621760
8    TensorFlowPackageTests.xctest      0x000056364966a2f2 <unavailable> + 2622194
9    TensorFlowPackageTests.xctest      0x0000563649467ad2 <unavailable> + 514770
10   TensorFlowPackageTests.xctest      0x0000563649467c64 <unavailable> + 515172
11   TensorFlowPackageTests.xctest      0x0000563649471824 <unavailable> + 555044
12   TensorFlowPackageTests.xctest      0x0000563649515e27 <unavailable> + 1228327
13   TensorFlowPackageTests.xctest      0x00005636497fb866 <unavailable> + 4266086
14   TensorFlowPackageTests.xctest      0x00005636497fcf11 <unavailable> + 4271889
15   TensorFlowPackageTests.xctest      0x0000563649801e33 <unavailable> + 4292147
16   TensorFlowPackageTests.xctest      0x000056364990373f <unavailable> + 5347135
17   TensorFlowPackageTests.xctest      0x0000563649903a1b <unavailable> + 5347867
18   TensorFlowPackageTests.xctest      0x00005636499086c3 <unavailable> + 5367491
19   TensorFlowPackageTests.xctest      0x00005636498f4dfe <unavailable> + 5287422
20   TensorFlowPackageTests.xctest      0x00005636495deaed <unavailable> + 2050797
21   TensorFlowPackageTests.xctest      0x00005636495e042d <unavailable> + 2057261
22   TensorFlowPackageTests.xctest      0x00005636495de44f <unavailable> + 2049103
23   TensorFlowPackageTests.xctest      0x00005636495df960 <unavailable> + 2054496
24   TensorFlowPackageTests.xctest      0x00005636498f455c <unavailable> + 5285212
25   TensorFlowPackageTests.xctest      0x000056364988c39c <unavailable> + 4858780
26   TensorFlowPackageTests.xctest      0x000056364990c311 <unavailable> + 5382929
27   libXCTest.so                       0x00007f84c7631bb1 <unavailable> + 187313
28   libXCTest.so                       0x00007f84c7631c0e <unavailable> + 187406
29   libXCTest.so                       0x00007f84c7631b84 <unavailable> + 187268
30   libXCTest.so                       0x00007f84c7624a27 <unavailable> + 133671
31   libXCTest.so                       0x00007f84c7630e80 XCTestCase.invokeTest() + 73
32   libXCTest.so                       0x00007f84c7630ae0 XCTestCase.perform(_:) + 172
33   libXCTest.so                       0x00007f84c7634da0 XCTest.run() + 191
34   libXCTest.so                       0x00007f84c7632980 XCTestSuite.perform(_:) + 232
35   libXCTest.so                       0x00007f84c7634da0 XCTest.run() + 191
36   libXCTest.so                       0x00007f84c7632980 XCTestSuite.perform(_:) + 232
37   libXCTest.so                       0x00007f84c7634da0 XCTest.run() + 191
38   libXCTest.so                       0x00007f84c7632980 XCTestSuite.perform(_:) + 256
39   libXCTest.so                       0x00007f84c7634da0 XCTest.run() + 191
40   libXCTest.so                       0x00007f84c762f700 XCTMain(_:) + 1091
41   TensorFlowPackageTests.xctest      0x0000563649889d44 <unavailable> + 4848964
42   libc.so.6                          0x00007f84a9c1cab0 __libc_start_main + 231
43   TensorFlowPackageTests.xctest      0x00005636494564ca <unavailable> + 443594

line no 299 in EagerExecution.swift belongs to an internal function called evaluateUnsafe:

_TFCEagerExecute(op, UnsafeMutablePointer<CTensorHandle?>(buffer), &count, status)

@Shashi456
Copy link
Contributor Author

@rxwei even the build in #413 has the same test error.

@rxwei
Copy link
Contributor

rxwei commented Aug 7, 2019

The nightly toolchain used for CI is not up to date because of toolchain build failures. The team is looking into this.

@Shashi456
Copy link
Contributor Author

@rxwei Looks like the CI issue was fixed, Could you add a build to this?

@Shashi456
Copy link
Contributor Author

The errors were because of being up to date with master. Fixed.

@Shashi456
Copy link
Contributor Author

This can be merged.

@eaplatanios eaplatanios merged commit b1db1f8 into tensorflow:master Aug 9, 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.

6 participants