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

Add Support for MatMulInteger #2072

Merged
merged 11 commits into from
Nov 4, 2022
Merged

Add Support for MatMulInteger #2072

merged 11 commits into from
Nov 4, 2022

Conversation

GregoryMorse
Copy link
Contributor

@GregoryMorse GregoryMorse commented Oct 29, 2022

MatMulInteger was supported in ONNX opset v10, this specific type combination is support in TensorFlow, but the node type not identified and handled properly here.

Handles #2071

@fatcat-z
Copy link
Collaborator

fatcat-z commented Oct 30, 2022

Thank you for your contributions.
Please following the details to fix pylint issues, and sign-off your commit to resolve DCO issue.

@GregoryMorse
Copy link
Contributor Author

And also sign-off your commit to resolve DCO issue.

I wanted to ask if this change will require unit tests? If so might as well address that also.

@fatcat-z
Copy link
Collaborator

And also sign-off your commit to resolve DCO issue.

I wanted to ask if this change will require unit tests? If so might as well address that also.

Yes, please add a unit test into here.

@fatcat-z
Copy link
Collaborator

fatcat-z commented Nov 4, 2022

Thanks for your contribution.

Could you please follow the details of DCO failure to make it pass? And also please add a new unit test here.

MatMulInteger was supported in ONNX opset v10 (not checked in proposed change, the error can be addressed on save), this specific type combination is support in TensorFlow, but the node type not identified and handled properly here.

Handles onnx#2071

Signed-off-by: Gregory Morse <gregory.morse@live.com>
Signed-off-by: Gregory Morse <gregory.morse@live.com>
Signed-off-by: Gregory Morse <gregory.morse@live.com>
Signed-off-by: Gregory Morse <gregory.morse@live.com>
Signed-off-by: Gregory Morse <gregory.morse@live.com>
Signed-off-by: Gregory Morse <gregory.morse@live.com>
Signed-off-by: Gregory Morse <gregory.morse@live.com>
Signed-off-by: Gregory Morse <gregory.morse@live.com>
Signed-off-by: Gregory Morse <gregory.morse@live.com>
Signed-off-by: Gregory Morse <gregory.morse@live.com>
@GregoryMorse
Copy link
Contributor Author

Thanks for your contribution.

Could you please follow the details of DCO failure to make it pass? And also please add a new unit test here.

All set, kindly ask for a review.

Copy link
Collaborator

@fatcat-z fatcat-z left a comment

Choose a reason for hiding this comment

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

Thanks for your efforts, LGTM!

@fatcat-z fatcat-z merged commit 2c1db54 into onnx:main Nov 4, 2022
fsalv pushed a commit to fsalv/tensorflow-onnx that referenced this pull request Dec 6, 2022
* Add Support for MatMulInteger

MatMulInteger was supported in ONNX opset v10 (not checked in proposed change, the error can be addressed on save), this specific type combination is support in TensorFlow, but the node type not identified and handled properly here.

Handles onnx#2071

Signed-off-by: Gregory Morse <gregory.morse@live.com>

* Update math.py

Signed-off-by: Gregory Morse <gregory.morse@live.com>

* Update support_status.md

Signed-off-by: Gregory Morse <gregory.morse@live.com>

* Update test_backend.py

Signed-off-by: Gregory Morse <gregory.morse@live.com>

Signed-off-by: Gregory Morse <gregory.morse@live.com>
Co-authored-by: Jay Zhang <36183870+fatcat-z@users.noreply.github.com>
Signed-off-by: Salvetti, Francesco <Francesco.Salvetti@nuance.com>
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

2 participants