Remove check for equal shapes in isAlmostEqual
#367
Conversation
Co-Authored-By: Anthony Platanios <[email protected]>
|
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the ℹ️ Googlers: Go here for more info. |
| @@ -229,14 +229,12 @@ public extension StringTensor { | |||
| } | |||
|
|
|||
| public extension Tensor where Scalar: TensorFlowFloatingPoint { | |||
| /// Returns `true` if tensors are of equal shape and all pairs of scalars are approximately | |||
| /// equal. | |||
| /// Returns `true` if all elements of this tensor are approximately equal to those of `other`. | |||
There was a problem hiding this comment.
Semantically, does isAlmostEqual(to:tolerance:) support arguments with incompatible shapes? If only compatible shapes are accepted, saying "all elements ... are approximately equal" sounds like a scalar comparison on flattened arrays. It'd be good to say something about shapes here as well as in elementsAlmostEqual(_:tolerance:).
There was a problem hiding this comment.
The underlying TF op requires the shapes to be the same, see https://github.com/tensorflow/tensorflow/blob/master/tensorflow/core/kernels/cwise_ops_common.h#L172
I added that condition to the comment.
Is this something we want to catch at the Swift level? It seems generally we don't catch shape errors at the Swift level, right?
There was a problem hiding this comment.
@bgogul has been working on tracking Tensor shapes in the Swift Tensor object as part of his work on Lazy Tensor. I'll let him comment further. :-)
There was a problem hiding this comment.
@saeta could we also discuss this during Friday's meeting?
There was a problem hiding this comment.
Here is a work in progress prototype of shape inference: https://github.com/tensorflow/swift-apis/pull/388/files
Note that this requires some changes from TF. So, you won't be able to try it right away. However, this gives the gist of what will be available on the Swift side. Essentially, we will be able to examine shapes on Swift side without actually executing the op (with a few exceptions, where we may need to materialize.)
Happy to fill in more during the open design meeting this Friday.
Co-Authored-By: Richard Wei <[email protected]>
|
Can this be merged? |
|
Yes. Thanks for the ping! |
No description provided.