Skip to content
This repository was archived by the owner on Nov 17, 2023. It is now read-only.

[v1.x] ONNX: fix error handling when op is not registered #20261

Merged
Zha0q1 merged 2 commits into from
May 13, 2021
Merged

[v1.x] ONNX: fix error handling when op is not registered #20261

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2c89b0e
fix
Zha0q1 May 12, 2021
21a4ab5
fix pylint
Zha0q1 May 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion python/mxnet/onnx/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ mx.onnx.export_model(mx_sym, mx_params, in_shapes, in_dtypes, onnx_file,
|_rminus_scalar|1.7 1.8 |
|_rnn_param_concat|1.7 1.8 |
|_sample_multinomial|1.7 1.8 |
|_split_v2|1.7 1.8 |
|_zeros|1.7 1.8 |
|abs|1.7 1.8 |
|add_n|1.7 1.8 |
Expand All @@ -173,6 +174,7 @@ mx.onnx.export_model(mx_sym, mx_params, in_shapes, in_dtypes, onnx_file,
|broadcast_minimum|1.7 1.8 |
|broadcast_mod|1.7 1.8 |
|broadcast_mul|1.7 1.8 |
|broadcast_not_equal|1.7 1.8 |
|broadcast_power|1.7 1.8 |
|broadcast_sub|1.7 1.8 |
|broadcast_to|1.7 1.8 |
Expand Down Expand Up @@ -405,4 +407,4 @@ mx.onnx.export_model(mx_sym, mx_params, in_shapes, in_dtypes, onnx_file,
|standard_lstm_lm_200|
|standard_lstm_lm_650|
|standard_lstm_lm_1500|
|transformer_en_de_512|
|transformer_en_de_512|
16 changes: 12 additions & 4 deletions python/mxnet/onnx/mx2onnx/_export_onnx.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@

# coding: utf-8
# pylint: disable=invalid-name,too-many-locals,no-self-use,too-many-arguments,
# pylint: disable=maybe-no-member,too-many-nested-blocks
# pylint: disable=maybe-no-member,too-many-nested-blocks,logging-not-lazy
"""MXNet to ONNX graph converter functions"""
import logging
import json
Expand Down Expand Up @@ -91,15 +91,23 @@ def convert_layer(node, **kwargs):

op = str(node["op"])
opset_version = kwargs.get("opset_version", onnx_opset_version())
# fallback to older opset versions if op is not registered in current version
if opset_version < 12:
logging.warning('Your ONNX op set version is %s, ' % str(opset_version) +
'which is lower than then lowest tested op set (12), please consider '
Copy link
Contributor

@samskalicky samskalicky May 12, 2021

Choose a reason for hiding this comment

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

is there some variable we can use to print the op set version instead of hard coding "12" in this message? its likely we may forget to update it later...

Copy link
Contributor Author

@Zha0q1 Zha0q1 May 12, 2021

Choose a reason for hiding this comment

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

I think unlike cuda where we keep a supported window, the minimum supported version for onnx is fixed at 12 (latest is 14). When new opeset is released we can add support for it without removing the previous opset support

'updating ONNX')
opset_version = 12
# Fallback to older opset versions if op is not registered in current version
convert_func = None
for op_version in range(opset_version, 11, -1):
if op_version not in MXNetGraph.registry_ or op not in MXNetGraph.registry_[op_version]:
if opset_version == 12:
raise AttributeError("No conversion function registered for op type %s yet." % op)
continue
convert_func = MXNetGraph.registry_[op_version][op]
break

# The conversion logic is not implemented
if convert_func is None:
raise AttributeError("No conversion function registered for op type %s yet." % op)
Copy link
Contributor

@samskalicky samskalicky May 12, 2021

Choose a reason for hiding this comment

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

can you add a comment here to explain why this would happen? (ie. is it because the op is not supported or because the installed version of onnx is too old?)

Copy link
Contributor Author

@Zha0q1 Zha0q1 May 12, 2021

Choose a reason for hiding this comment

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

added!


ret = convert_func(node, **kwargs)
# in case the conversion function does not specify the returned dtype, we just return None
# as the second value
Expand Down