-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Conversation
@mxnet-label-bot add [Doc, pr-awaiting-review] |
@IvyBazan for review |
LGTM, thanks for the fix! |
@aaronmarkham for review/merge |
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.
LGTM. But... can you also update the tutorial, or add that in another PR?
https://github.com/apache/incubator-mxnet/blob/master/docs/tutorials/onnx/export_mxnet_to_onnx.md
Or is the tutorial fine, and the version requirements fine?
The tutorial was written with MXNet 1.3 and ONNX 1.2.1. Since the importer-exporter is backward compatible, this should still work and the statement "Note: MXNet-ONNX importer and exporter follows version 7 of ONNX operator set which comes with ONNX v1.2.1." holds true. Should I update the tutorial with information that MXNet now supports ONNX 1.3 with opset 8? |
Since we have so much fun getting stuff through CI, let's merge this as is. Please add the extra info in another PR. |
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.
LGTM
Description
Fixes #15437
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.
Changes
Comments