You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.
ONNX implementation in mx2onnx.export_model:export_model() makes multiple incorrect assumptions about the symbolic graph which makes its implementation very fragile to any graph that doesn't fit its assumptions.
Incorrect assumptions:
Order that data is fed to network is the order that sym.list_inputs() lists them:
This is an incorrect assumption because the order data is fed to network (particularly a gluon block) is determined by custom block's API. However the order that data is listed in sym.list_inputs() is by the dependency in computation graph.
There can be one and only one label in the graph and this label is named after the last node of the graph on sym.get_internals().
This is clearly an incorrect assumption. Perhaps if the last node is called 'softmax' and the network's label was called 'softmax_label' (which happens to be the default label name) then this assumption will hold. However under any other condition, I don't think this is a valid assumption.
A proper implementation would be to change the export_model() API to accept a dictionary of name: shape for input instead of a list of shapes. Any input that is not in params or in inputs will then be assumed to be a label and may be ignored. However if incorrectly specified as unnecessary label, then ONNX export will fail.
For backward compatibility, if a list of shapes is provided as inputs, the current automatic name to shape mapping will happen but a warning log is created, warning user to use a dictionary and also warns about the default mapping between shapes and inputs.
The text was updated successfully, but these errors were encountered:
Thanks Sina for creating issue for it. I am aware of this issue and started working on the fix for it #12946 but got sidelined by other things. I will rebase the PR with latest changes and continue working on the fix.
I also have added optional parameters "label_names", "label_shapes"([name, shape]) to export_model API.
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Description
ONNX implementation in mx2onnx.export_model:export_model() makes multiple incorrect assumptions about the symbolic graph which makes its implementation very fragile to any graph that doesn't fit its assumptions.
Incorrect assumptions:
sym.list_inputs()
lists them:sym.list_inputs()
is by the dependency in computation graph.A proper implementation would be to change the
export_model()
API to accept a dictionary ofname: shape
for input instead of a list of shapes. Any input that is not in params or in inputs will then be assumed to be a label and may be ignored. However if incorrectly specified as unnecessary label, then ONNX export will fail.For backward compatibility, if a list of shapes is provided as inputs, the current automatic name to shape mapping will happen but a warning log is created, warning user to use a dictionary and also warns about the default mapping between shapes and inputs.
The text was updated successfully, but these errors were encountered: