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

[TF] Change APIs to use Int instead of Int32. #84

Merged
merged 1 commit into from
Apr 16, 2019

Conversation

dan-zheng
Copy link
Member

@lattner
Copy link

lattner commented Apr 14, 2019

This looks really great Dan!

@dan-zheng dan-zheng force-pushed the use-int-in-tf-apis branch from 48902c2 to 292bdcf Compare April 14, 2019 05:46
@dan-zheng
Copy link
Member Author

Ready for review.

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.

The swift-apis side LGTM. Stdlib needs some more separation of concerns that I mentioned in that PR.

@eaplatanios
Copy link
Contributor

This is great! It's been very annoying having to cast between Int and Int32 all the time.

@dan-zheng dan-zheng force-pushed the use-int-in-tf-apis branch 2 times, most recently from 88c71b8 to 686570d Compare April 15, 2019 20:04
@dan-zheng dan-zheng force-pushed the use-int-in-tf-apis branch from 686570d to af1333e Compare April 16, 2019 20:38
@dan-zheng
Copy link
Member Author

Locally verified that all tests pass. Merging now, then testing/merging swiftlang/swift#24012.

Note: I built a toolchain and swift build almost works:

$ swift build
Compile Swift Module 'DeepLearning' (8 sources)
/Users/danielzheng/swift-dev/tensorflow-swift-apis/Sources/DeepLearning/Layer.swift:49:35: error: ambiguous use of 'inference'
        return withLearningPhase(.inference) {
                                  ^
/Users/danielzheng/swift-dev/tensorflow-swift-apis/Sources/DeepLearning/Context.swift:28:10: note: found this candidate
    case inference
         ^
TensorFlow.LearningPhase:3:10: note: found this candidate
    case inference
         ^
/Users/danielzheng/swift-dev/tensorflow-swift-apis/Sources/DeepLearning/Layer.swift:60:35: error: ambiguous use of 'inference'
        return withLearningPhase(.inference) {
                                  ^
/Users/danielzheng/swift-dev/tensorflow-swift-apis/Sources/DeepLearning/Context.swift:28:10: note: found this candidate
    case inference
         ^
TensorFlow.LearningPhase:3:10: note: found this candidate
    case inference
         ^

If LearningPhase.inference is explicitly qualified, swift build succeeds.

@dan-zheng dan-zheng merged commit 23c16ae into master Apr 16, 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