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

Added Dot Layer #275

Merged
merged 3 commits into from
Dec 3, 2021
Merged

Added Dot Layer #275

merged 3 commits into from
Dec 3, 2021

Conversation

therealansh
Copy link
Contributor

This PR adds Dot layer and closes #144. @zaleslaw would you mind reviewing it for this initial commit. I am still working on some test cases.

@zaleslaw zaleslaw added the Review This PR is under review label Nov 17, 2021
@zaleslaw
Copy link
Collaborator

zaleslaw commented Dec 3, 2021

Hi @therealansh do you have plans to fix a review comments here? It will be great to merge it in December in the codebase

@therealansh
Copy link
Contributor Author

hey @zaleslaw sorry about this delay, i am actually figuring a way out for test cases I'll fix this and add docs accordingly. Maybe i can add DotTestCase in PR for #276 what do you think?

@therealansh
Copy link
Contributor Author

Added the docs! I have removed the Dot Test case for now. I'll be doing it in the PR for #276. Should i add a TODO as well for refactoing l2Normalize and batchDot for future refactoring?

@therealansh therealansh changed the title [WIP] Added Dot Layer Added Dot Layer Dec 3, 2021
@zaleslaw
Copy link
Collaborator

zaleslaw commented Dec 3, 2021

No needs to add TODO, I make this refactoring in the nearest future

@zaleslaw zaleslaw merged commit bf33db7 into Kotlin:master Dec 3, 2021
@zaleslaw
Copy link
Collaborator

zaleslaw commented Dec 3, 2021

Great, now we have DOT!

@therealansh
Copy link
Contributor Author

Hey @zaleslaw , while writing test cases for Dot i have run into some errors,

In[0] is not a matrix. Instead it has shape [2,1,5,2]

at

 out = MatMul.create(scope, x2, y2, MatMul.transposeA(adjX), MatMul.transposeB(adjY))

in batchDot. Can you review it once I cannot understand what the issue is about?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Review This PR is under review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Dot layer
2 participants