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

[FIX][ONNX][Relay] onnx converter on matmul with scalar; bring back nn.matmul check #13448

Merged
merged 1 commit into from
Nov 21, 2022

Conversation

ganler
Copy link
Contributor

@ganler ganler commented Nov 21, 2022

This PR brings 2 bug fixes:

  1. ONNX converter for matmul: ONNX matmul follows NumPy rules:

If the first argument is 1-D, it is promoted to a matrix by prepending a 1 to its dimensions. After matrix multiplication the prepended 1 is removed.
If the second argument is 1-D, it is promoted to a matrix by appending a 1 to its dimensions. After matrix multiplication the appended 1 is removed.

The (my) previous fix #11174 did not consider the second rule (append 1 dimension for the rhs vector).

  1. Relay's nn.matmul takes 2-D matrices and the checker was removed in a recent PR [FIX] Handle matmul where one inner dimension is unknown #13287. This PR puts the checker back to prevent process crashes (make it a readable TVMError) for readability (and also for that the CI in Upstream sharpened GraphIR ise-uiuc/nnsmith#64 won't be terminated while using TVM-10).

cc: @masahi @tkonolige

@tvm-bot
Copy link
Collaborator

tvm-bot commented Nov 21, 2022

Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment.

Generated by tvm-bot

Copy link
Member

@junrushao junrushao left a comment

Choose a reason for hiding this comment

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

LGTM!

@junrushao junrushao merged commit 41b0400 into apache:main Nov 21, 2022
@ganler ganler deleted the fix-matmul branch November 21, 2022 03:55
xinetzone pushed a commit to daobook/tvm that referenced this pull request Nov 25, 2022
…n.matmul check (apache#13448)

This PR brings 2 bug fixes:
1. ONNX converter for matmul: ONNX matmul follows NumPy [rules](https://numpy.org/doc/stable/reference/generated/numpy.matmul.html):
> If the first argument is 1-D, it is promoted to a matrix by prepending a 1 to its dimensions. After matrix multiplication the prepended 1 is removed.
> If the second argument is 1-D, it is promoted to a matrix by appending a 1 to its dimensions. After matrix multiplication the appended 1 is removed.

The (my) previous fix apache#11174 did not consider the second rule (append 1 dimension for the rhs vector).

2. Relay's `nn.matmul` takes 2-D matrices and the checker was removed in a recent PR apache#13287. This PR puts the checker back to prevent process crashes (make it a readable TVMError) for readability (and also for that the CI in ise-uiuc/nnsmith#64 won't be terminated while using TVM-10).
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 this pull request may close these issues.

None yet

3 participants