MatMul spec update #2765
Merged
MatMul spec update #2765
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
slyalin
reviewed
Oct 22, 2020
slyalin
requested changes
Oct 22, 2020
mitruska
requested review from
rkazants,
slyalin,
GlebKazantaev,
vladimir-paramuzov,
dmitry-gorokhov,
sdurawa,
lazarevevgeny and
jane-intel
mitruska
commented
Oct 23, 2020
slyalin
approved these changes
Oct 30, 2020
slyalin
requested changes
Oct 30, 2020
slyalin
requested changes
Nov 3, 2020
slyalin
approved these changes
Nov 6, 2020
ilyachur
approved these changes
Nov 6, 2020
vladimir-paramuzov
approved these changes
Nov 6, 2020
lazarevevgeny
approved these changes
Nov 6, 2020
dmitry-gorokhov
approved these changes
Nov 6, 2020
mryzhov
pushed a commit
to mryzhov/openvino
that referenced
this pull request
Dec 16, 2020
Aligned specification with nGraph MatMul shape inference operation. Affects description of the behavior for 1D tensors.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We are considering change in OpenVino MatMul specification, because of discrepancy with the current ngraph implementation for 1D tensors.
The tests on CPU proved that the current implementation follows rather numpy matmul shape inference logic instead of the openvino MatMul spec (1D x 1D seems to be not supported by CPU at all).
We shouldn't change the current behaviour of ngraph MatMul to ensure backward compatibility and numpy approach is rather common for other frameworks.
Multiplying two matrices of shape 2D and bigger seems to be the same and correct anyway.
I would like to ask people from other plugins to check support for MatMul 1D cases and specification review to ensure that we should update the MatMul spec.
This topic is related with fused op removal PR #2866.
Feel free to add other reviewers.
Ticket: 37453