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

Change Dense.bias type to Optional #1062

Closed
wants to merge 3 commits into from

Conversation

efremale
Copy link
Contributor

@efremale efremale commented Aug 18, 2020

TrivialModelTests.swift test is currently failing.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@dan-zheng dan-zheng marked this pull request as draft August 18, 2020 22:21
@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

There are two remaining test failures:

```
$ swift test --filter OptimizerTests
Tests/TensorFlowTests/OptimizerTests.swift:117: error: -[TensorFlowTests.OptimizerTests testAdaMax] : XCTAssertTrue failed
Tests/TensorFlowTests/OptimizerTests.swift:123: error: -[TensorFlowTests.OptimizerTests testAMSGrad] : XCTAssertTrue failed
```
@efremale efremale marked this pull request as ready for review September 21, 2020 17:29
@efremale efremale changed the title [WIP] Change Dense.bias type to Optional Change Dense.bias type to Optional Sep 21, 2020
@saeta
Copy link
Contributor

saeta commented Sep 30, 2020

Could you add a test to verify that this will work with CopyableToDevice (if there isn't one already)? Thanks!

@dan-zheng
Copy link
Member

dan-zheng commented Dec 14, 2020

CI fails for a test that seems related to key path iteration:

Test Case 'TensorVisitorPlanTests.testMask' started at 2020-12-09 18:57:37.697
/swift-apis/Tests/x10/TensorVisitorPlanTest.swift:55: error: TensorVisitorPlanTests.testMask : XCTAssertEqual failed: ("[true, false, true, false]") is not equal to ("[true, true]") -
Test Case 'TensorVisitorPlanTests.testMask' failed (0.001 seconds)


Closing for now because optional differentiation isn't fully supported (SR-13700), so usability wouldn't be great (if users can't differentiate through dense.bias ?? default nil coalescing).

Investigating the test failure on this branch would be a great step towards a smooth migration to using Optional differentiation in deep learning APIs and models (e.g. ResNet) instead of the current, less efficient workarounds!

@dan-zheng dan-zheng closed this Dec 14, 2020
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