Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mismatch in the description of BatchDot and TensorFlow's implementation. #30846

Open
zachmayer opened this issue Jul 18, 2019 · 1 comment
Open

Comments

@zachmayer
Copy link

@zachmayer zachmayer commented Jul 18, 2019

URL(s) with the issue:

https://www.tensorflow.org/api_docs/python/tf/keras/backend/batch_dot

Description of issue (what needs changing):

I raised an issue on the plaidML repo, and after some back and forth we determined the documentation for BatchDot doesn't quite match the actual implementation in the tensorflow code.

Clear description

A BatchDot with x.shape=(1,2,6,2) and y.shape=(1,2,2,3) and axes = (3, 1)has an output shape of (1,2,6,3)) whereas by the TF definition for output shape "A tensor with shape equal to the concatenation of x's shape (less the dimension that was summed over) and y's shape (less the batch dimension and the dimension that was summed over). If the final rank is 1, we reshape it to (batch_size, 1)." sounds like it should have an output shape of (1,2,6,2,3).

Submit a pull request?

I am not planning to submit a PR at this time, but I may do it later

@dgkutnic

This comment has been minimized.

Copy link

@dgkutnic dgkutnic commented Jul 18, 2019

To further elaborate on this issue: there also seems to be output shape mismatches between the TensorFlow and Theano backend implementations of BatchDot. Theano's implementation seems to be much closer to the documented definition of BatchDot than TensorFlow.

Could we either change the documentation to more clearly describe TensorFlow's engineering decisions behind BatchDot or modify the TensorFlow implementation of BatchDot to more closely match what is described in the documentation?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.