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

DotLayer, use single reduce argument #636

Closed
albertz opened this issue Sep 7, 2021 · 2 comments · Fixed by #837
Closed

DotLayer, use single reduce argument #636

albertz opened this issue Sep 7, 2021 · 2 comments · Fixed by #837

Comments

@albertz
Copy link
Member

albertz commented Sep 7, 2021

Related to #629 and #627, it can be redundant to specify both red1 and red2 as arguments. These axes really need to be the same, and also share the same dyn lengths.

Via unique dim tags (#632), we can use a single reduce argument.

Originally posted by @Zettelkasten in #629 (comment)

@albertz
Copy link
Member Author

albertz commented Sep 23, 2021

An alternative to this proposal is to simply use out_dims, as it was proposed in #597 (here). By simply specifying the output dims / shape, we can infer all information.

Mesh Tensorflow mtf.einsum(inputs, output_shape) also follows exactly this logic.

For rec optimization, this is also easy to handle (as we already do now, via #628).

Although we do not have to do either-or, we can also do both. It might be helpful for readability to have reduce explicit.

@albertz
Copy link
Member Author

albertz commented Nov 8, 2021

@Zettelkasten To clarify, you also propose to make var1/var2 optional, as you stated here? Although this is kind of orthogonal to using a single reduce argument. Edit Separate issue on this: #738

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

Successfully merging a pull request may close this issue.

1 participant