Skip to content

Conversation

@kanvi-nervana
Copy link
Contributor

Redo BatchMatMul since nGraph has the op support for it now
Add tests

@kanvi-nervana kanvi-nervana changed the title Redo BatchMatMul Kanvi/Redo BatchMatMul May 10, 2019
Copy link
Contributor

@sayantan-nervana sayantan-nervana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good in general. Lets wait for CPU's feedback from Gauri.
Minor comment about one of the tests

Copy link
Contributor

@sindhu-nervana sindhu-nervana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@sayantan-nervana sayantan-nervana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@avijit-nervana avijit-nervana merged commit 2ec1054 into master May 16, 2019
@avijit-nervana avijit-nervana deleted the kanvi/redo-batchmatmul branch May 16, 2019 22:04
gopoka pushed a commit that referenced this pull request Oct 28, 2019
* Implement keep_dims for mean and sum; add a few tactical hacks for cases where DT_STRING is being placed on nGraph (not yet a complete solution)
* Revert DT_STRING stuff since #68 fixes the issue
* Remove tf_keep_dims 'Unimplemented' check for Mean in builder
gopoka pushed a commit that referenced this pull request Oct 28, 2019
* Redo BatchMatMul since nGraph has the op supposrt for it now and add tests
* Skip BatchMatMul3* tests for GPU since GPU does not support this op
* Changes to support nGraph BatchMatMul for CPU and the older way of translating for other backends
* Redo the translation for BatchMatMul so that it uses the CPU BatchMatMul
when number of dimensions = 3 and backend = CPU
Add logic to handle the attributes correctly for the same
Redo the tests for BatchMatMul
* Reorganize the code to add the correct backend as attribute
* Add logic to pass higher-order(>3) BatchMatMul to CPU as well.
* Update test/test_math_ops.cpp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants