-
Notifications
You must be signed in to change notification settings - Fork 6.8k
ONNX export: Add Flatten before Gemm #13356
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how are we testing this change?
@anirudhacharya the test is WIP (added a note in the PR description). Adding a test for fully connected in mxnet_export_test. |
@mxnet-label-bot add [ONNX, pr-work-in-progress] |
372cb0a
to
8cd959d
Compare
d70ff8a
to
bcc947d
Compare
@mxnet-label-bot update [ONNX, pr-awaiting-review] |
@Roshrini, @anirudhacharya for review? |
2421bd0
to
2975ab8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested this fix on Arcface model in model zoo, LGTM
@vandanavk
|
b323418
to
739f181
Compare
Codecov Report
@@ Coverage Diff @@
## master #13356 +/- ##
==========================================
+ Coverage 82.73% 83.26% +0.53%
==========================================
Files 721 746 +25
Lines 79494 83497 +4003
Branches 3193 3193
==========================================
+ Hits 65770 69526 +3756
- Misses 12864 13113 +249
+ Partials 860 858 -2
Continue to review full report at Codecov.
|
@zhreshold for review |
dd22dc1
to
24f47c2
Compare
@zhreshold Can you please review/merge this PR? |
@mxnet-label-bot update [ONNX, pr-awaiting-merge] |
* Add Flatten before Gemm * ONNX export test: Allow multiple inputs in forward pass * ONNX export: Test for fully connected
* Add Flatten before Gemm * ONNX export test: Allow multiple inputs in forward pass * ONNX export: Test for fully connected
Description
ONNX's spec specifies that Gemm input data should be 2D, but this wasn't the case for VGG and ResNet models trained in MXNet and exported to ONNX. This PR adds a flatten node before Gemm to make the input data 2D.
Related issues: onnx/models#90, onnx/onnx#1101, onnx/models#91 (comment)
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.
Changes
Comments