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

Fix implementation of root(Tensor, Int). #238

Merged
merged 1 commit into from
Jun 14, 2019

Conversation

dan-zheng
Copy link
Member

Fix test failures regarding root(Tensor, Int) erroneous returning NaNs.

Inspired by the implementation of root for FloatingPoint types in
swift/stdlib/public/core/MathFunctions.swift.gyb.

Fix test failures regarding `root(Tensor, Int)` erroneous returning NaNs.

Inspired by the implementation of `root` for `FloatingPoint` types in
swift/stdlib/public/core/MathFunctions.swift.gyb.
@dan-zheng dan-zheng requested a review from rxwei June 14, 2019 03:50
@dan-zheng
Copy link
Member Author

For reference, here are the previous failures:

/Users/danielzheng/swift-apis/Tests/TensorFlowTests/OperatorTests/MathTests.swift:58: error: -[TensorFlowTests.MathOperatorTests testElementaryFunctions] : XCTAssertTrue failed - -nan is not equal to -1.1927887 - root
/Users/danielzheng/swift-apis/Tests/TensorFlowTests/OperatorTests/MathTests.swift:58: error: -[TensorFlowTests.MathOperatorTests testElementaryFunctions] : XCTAssertTrue failed - -nan is not equal to -0.7837214 - root
/Users/danielzheng/swift-apis/Tests/TensorFlowTests/OperatorTests/MathTests.swift:58: error: -[TensorFlowTests.MathOperatorTests testElementaryFunctions] : XCTAssertTrue failed - -nan is not equal to -0.8716413 - root
/Users/danielzheng/swift-apis/Tests/TensorFlowTests/OperatorTests/MathTests.swift:58: error: -[TensorFlowTests.MathOperatorTests testElementaryFunctions] : XCTAssertTrue failed - -nan is not equal to -1.0000541 - root
/Users/danielzheng/swift-apis/Tests/TensorFlowTests/OperatorTests/MathTests.swift:58: error: -[TensorFlowTests.MathOperatorTests testElementaryFunctions] : XCTAssertTrue failed - -nan is not equal to -0.522651 - root
/Users/danielzheng/swift-apis/Tests/TensorFlowTests/OperatorTests/MathTests.swift:58: error: -[TensorFlowTests.MathOperatorTests testElementaryFunctions] : XCTAssertTrue failed - -nan is not equal to -0.82105964 - root
/Users/danielzheng/swift-apis/Tests/TensorFlowTests/OperatorTests/MathTests.swift:58: error: -[TensorFlowTests.MathOperatorTests testElementaryFunctions] : XCTAssertTrue failed - -nan is not equal to -0.56709105 - root
/Users/danielzheng/swift-apis/Tests/TensorFlowTests/OperatorTests/MathTests.swift:58: error: -[TensorFlowTests.MathOperatorTests testElementaryFunctions] : XCTAssertTrue failed - -nan is not equal to -1.1245675 - root

With this patch, tests pass:

Test Case '-[TensorFlowTests.MathOperatorTests testElementaryFunctions]' started.
Test Case '-[TensorFlowTests.MathOperatorTests testElementaryFunctions]' passed (0.002 seconds).

@dan-zheng
Copy link
Member Author

Merging to unblock progress, happy to address comments later.

@dan-zheng dan-zheng merged commit e5a7d2c into tensorflow:master Jun 14, 2019
@dan-zheng dan-zheng deleted the fix-root branch June 14, 2019 04:01
dan12411 pushed a commit to dan12411/swift-apis that referenced this pull request Jun 15, 2019
Fix test failures regarding `root(Tensor, Int)` erroneous returning NaNs.

Inspired by the implementation of `root` for `FloatingPoint` types in
swift/stdlib/public/core/MathFunctions.swift.gyb.
@eaplatanios
Copy link
Contributor

@dan-zheng Sorry to comment on this so late, but I believe this may be unexpected behavior to some users. Mathematically, this is only correct for odd roots. Even roots should not be defined for negative values. This should at least be mentioned in the documentation, if not modified.

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.

2 participants