-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Conversation
@Roshrini Please review. |
Thanks for doing this. |
@mxnet-label-bot add [pr-awaiting-response] |
- Removed unnecessary forward_pass() call - Modified infer_output_shape to return multiple shapes for multiple outputs as well as output names.
1739a81
to
11e7012
Compare
@anirudhacharya all issues addressed. Please review asap. I'd like this to be part of tonight's nightly if possible. |
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.
minor comments/questions. mostly LGTM.
@mxnet-label-bot remove [pr-awaiting-response] |
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.
Spoke with Sina offline, let's make sure that the infer shape solution works with network that have loss layers, otherwise LGTM
The current implementation of handling existence of training labels as part of the symbolic graph very broken. There are multiple issues with it:
My PR does not change this fragile implementation behavior, but this needs to be fixed. I created #13407 to address this issue. |
Description
ONNX export does not create a correct graph when there are multiple outputs. This PR fixes this bug.
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.
Changes
Comments
Before this change, export_onnx would treat the graph as a sequential graph with output as the last node in the sequence. This change fixes this behavior by using sym.list_outputs() to record all output nodes.