Added support for 'gathering' and its VJP. #151
Conversation
|
I think this one needs to pull from master. |
|
@rxwei this should be ready now. |
| /// // [2, 1, 1]] | ||
| /// ``` | ||
| /// | ||
| /// - Returns: A tensor with shape `(num_true, rank(condition))`. |
There was a problem hiding this comment.
num_true isn't defined anywhere. Maybe reword this?
| } | ||
| } | ||
|
|
||
| public extension Tensor { |
There was a problem hiding this comment.
I think that this API should be kept internal until we find a good spelling/semantics for it. The fact that it applies to Tensor<Bool> makes the nonZero name weird.
There was a problem hiding this comment.
We could split it in two functions: nonZeroIndices and falseIndices or something like that. How does that sound?
There was a problem hiding this comment.
Linguistically those adjectives are attached to "indices", which is not the intended meaning and could cause confusion. Also, which Python TF operator does it correspond to?
There was a problem hiding this comment.
It's part of the tf.where functionality when no x and y are provided (i.e., only condition is provided).
There was a problem hiding this comment.
It seems this is not being used by the implementation of gathering. Could we defer this to later PRs instead?
There was a problem hiding this comment.
That's true. I think it's used by one of the other PRs so I'll just move it there.
|
@eaplatanios Is it correct that this PR isn't blocked on any bugs? |
|
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. |
|
Looks like #156 has already added everything! |
No description provided.