vandanavk commented on a change in pull request #19017:
URL: https://github.com/apache/incubator-mxnet/pull/19017#discussion_r482703828



##########
File path: python/mxnet/contrib/onnx/mx2onnx/_op_translations.py
##########
@@ -462,6 +462,7 @@ def convert_pad(node, **kwargs):
     """Map MXNet's pad operator attributes to onnx's Pad operator
     and return the created node.
     """
+    opset_version = kwargs["opset_version"]

Review comment:
       Is this info always available? We did not get this in the earlier code 
because there were times when the opset_version was missing. I don't have an 
example ready in hand, but in case this happens, can we fallback to a default 
opset_version?

##########
File path: python/mxnet/contrib/onnx/mx2onnx/_op_translations.py
##########
@@ -470,28 +471,74 @@ def convert_pad(node, **kwargs):
     pad_mode = attrs.get("mode")
 
     if pad_mode == "constant":
-        pad_value = float(attrs.get("constant_value")) \
-            if "constant_value" in attrs else 0.0
-        node = onnx.helper.make_node(
-            'Pad',
-            inputs=input_nodes,
-            outputs=[name],
-            mode='constant',
-            value=pad_value,
-            pads=onnx_pad_width,
-            name=name
-        )
+        pad_value = np.float32(attrs.get("constant_value", 0.0))
+        if opset_version >= 11:
+            # starting with opset 11, pads and constant_value are inputs 
instead of attributes
+            from onnx.helper import make_tensor, make_tensor_value_info
+            initializer = kwargs["initializer"]
+            pads_input_name = name + "_pads"
+            pads_input_type = onnx.TensorProto.INT64
+            pads_input_shape = np.shape(np.array(onnx_pad_width))
+            pads_value_node = make_tensor_value_info(pads_input_name, 
pads_input_type, pads_input_shape)
+            pads_tensor_node = make_tensor(pads_input_name, pads_input_type, 
pads_input_shape, onnx_pad_width)
+            initializer.append(pads_tensor_node)
+            input_nodes.append(pads_input_name)

Review comment:
       L476-L485 is repeated below in L515-523. move these lines to a function?

##########
File path: python/mxnet/contrib/onnx/mx2onnx/_op_translations.py
##########
@@ -945,17 +1006,35 @@ def convert_dropout(node, **kwargs):
     and return the created node.
     """
     name, input_nodes, attrs = get_inputs(node, kwargs)
+    opset_version = kwargs["opset_version"]

Review comment:
       can we get this once in get_inputs and return to the caller?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to