[GitHub] sandeep-krishnamurthy commented on a change in pull request #10118: [MXNET-106][ONNX-MXNET] Adding ONNX Model zoo tests.
sandeep-krishnamurthy commented on a change in pull request #10118: [MXNET-106][ONNX-MXNET] Adding ONNX Model zoo tests. URL: https://github.com/apache/incubator-mxnet/pull/10118#discussion_r177316857 ## File path: tests/python-pytest/onnx/onnx_backend_test.py ## @@ -57,37 +57,40 @@ 'test_floor', ## Joining and spliting -#'test_concat.*', #---Failing test +'test_concat', #Basic neural network functions 'test_sigmoid', 'test_relu', -#'test_constant_pad', -#'test_edge_pad', -#'test_reflect_pad', +'test_constant_pad', +'test_edge_pad', +'test_reflect_pad', 'test_matmul', 'test_leakyrelu', 'test_elu', -#'test_softmax*', +'test_softmax_example', +'test_softmax_large_number', +'test_softmax_axis_2', 'test_conv', 'test_basic_conv', -#'test_globalmaxpool', -#'test_globalaveragepool', -#'test_batch_norm', +'test_transpose', +#'test_globalmaxpool', - tests to be added +#'test_globalaveragepool', - tests to be added +#'test_batch_norm', - tests to be added +#'test_gather', Review comment: Makes sense. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sandeep-krishnamurthy commented on a change in pull request #10118: [MXNET-106][ONNX-MXNET] Adding ONNX Model zoo tests.
sandeep-krishnamurthy commented on a change in pull request #10118:
[MXNET-106][ONNX-MXNET] Adding ONNX Model zoo tests.
URL: https://github.com/apache/incubator-mxnet/pull/10118#discussion_r177316880
##
File path: tests/python-pytest/onnx/onnx_test.py
##
@@ -21,19 +21,37 @@
ONNX backend test framework. Once we have PRs on the ONNX repo and get
those PRs merged, this file will get EOL'ed.
"""
+# pylint: disable=too-many-locals,wrong-import-position,import-error
from __future__ import absolute_import
import sys
import os
import unittest
import logging
import hashlib
+import tarfile
+from collections import namedtuple
import numpy as np
import numpy.testing as npt
from onnx import helper
-import backend as mxnet_backend
+from onnx import numpy_helper
+from onnx import TensorProto
+from mxnet.test_utils import download
+from mxnet.contrib import onnx as onnx_mxnet
+import mxnet as mx
CURR_PATH = os.path.dirname(os.path.abspath(os.path.expanduser(__file__)))
sys.path.insert(0, os.path.join(CURR_PATH, '../../python/unittest'))
from common import with_seed
+import backend as mxnet_backend
+
+
+URLS = {
+'bvlc_googlenet' :
Review comment:
Great!
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] sandeep-krishnamurthy commented on a change in pull request #10118: [MXNET-106][ONNX-MXNET] Adding ONNX Model zoo tests.
sandeep-krishnamurthy commented on a change in pull request #10118:
[MXNET-106][ONNX-MXNET] Adding ONNX Model zoo tests.
URL: https://github.com/apache/incubator-mxnet/pull/10118#discussion_r177316822
##
File path: python/mxnet/contrib/onnx/_import/translation_utils.py
##
@@ -90,10 +90,46 @@ def _fix_pooling(pool_type, inputs, new_attr):
stride = new_attr.get('stride')
kernel = new_attr.get('kernel')
padding = new_attr.get('pad')
-pad_width = (0, 0, 0, 0) + _pad_sequence_fix(padding, len(kernel))
-new_pad_op = symbol.pad(inputs[0], mode='constant', pad_width=pad_width)
-new_pooling_op = symbol.Pooling(new_pad_op, pool_type=pool_type,
-stride=stride, kernel=kernel)
+
+# Adding default stride.
+if stride is None:
+stride = (1,) * len(kernel)
+
+# Add padding attr if not provided.
+if padding is None:
+padding = (0,) * len(kernel) * 2
+
+# Mxnet Pad operator supports only 4D/5D tensors.
+# For 1D case, these are the steps:
+#Step 1. Add extra dummy dimension to make it 4D. Adding to axis = 2
+#Step 2. Apply padding to this changed tensor
+#Step 3. Remove the extra dimension added in step 1.
+if len(kernel) == 1:
+dummy_axis = 2
+# setting 0 padding to the new dim to be added.
+padding = (0, padding[0], 0, padding[1])
+pad_width = (0, 0, 0, 0) + _pad_sequence_fix(padding, kernel_dim=2)
+
+# Step 1.
+curr_sym = symbol.expand_dims(inputs[0], axis=dummy_axis)
+
+# Step 2. Common for all tensor sizes
+new_pad_op = symbol.pad(curr_sym, mode='edge', pad_width=pad_width)
+
+# Step 3: Removing extra dim added.
+new_pad_op = symbol.split(new_pad_op, axis=dummy_axis, num_outputs=1,
squeeze_axis=1)
+else:
+# For 2D/3D cases:
+# Apply padding
+pad_width = (0, 0, 0, 0) + _pad_sequence_fix(padding,
kernel_dim=len(kernel))
+curr_sym = inputs[0]
+if pool_type == 'max':
+new_pad_op = symbol.pad(curr_sym, mode='edge', pad_width=pad_width)
+else:
+new_pad_op = symbol.pad(curr_sym, mode='constant',
pad_width=pad_width)
+
+# Apply pooling without pads.
+new_pooling_op = symbol.Pooling(new_pad_op, pool_type=pool_type,
stride=stride, kernel=kernel)
Review comment:
Thanks. Yes please, this explanation as code doc will be helpful.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] sandeep-krishnamurthy commented on a change in pull request #10118: [MXNET-106][ONNX-MXNET] Adding ONNX Model zoo tests.
sandeep-krishnamurthy commented on a change in pull request #10118: [MXNET-106][ONNX-MXNET] Adding ONNX Model zoo tests. URL: https://github.com/apache/incubator-mxnet/pull/10118#discussion_r177305755 ## File path: tests/python-pytest/onnx/onnx_backend_test.py ## @@ -57,37 +57,40 @@ 'test_floor', ## Joining and spliting -#'test_concat.*', #---Failing test +'test_concat', #Basic neural network functions 'test_sigmoid', 'test_relu', -#'test_constant_pad', -#'test_edge_pad', -#'test_reflect_pad', +'test_constant_pad', +'test_edge_pad', +'test_reflect_pad', 'test_matmul', 'test_leakyrelu', 'test_elu', -#'test_softmax*', +'test_softmax_example', +'test_softmax_large_number', +'test_softmax_axis_2', 'test_conv', 'test_basic_conv', -#'test_globalmaxpool', -#'test_globalaveragepool', -#'test_batch_norm', +'test_transpose', +#'test_globalmaxpool', - tests to be added +#'test_globalaveragepool', - tests to be added +#'test_batch_norm', - tests to be added +#'test_gather', Review comment: Does this mean, for an operator we are adding, we do not cover with test cases? May be prone for issues? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sandeep-krishnamurthy commented on a change in pull request #10118: [MXNET-106][ONNX-MXNET] Adding ONNX Model zoo tests.
sandeep-krishnamurthy commented on a change in pull request #10118:
[MXNET-106][ONNX-MXNET] Adding ONNX Model zoo tests.
URL: https://github.com/apache/incubator-mxnet/pull/10118#discussion_r177305637
##
File path: python/mxnet/contrib/onnx/_import/op_translations.py
##
@@ -402,9 +438,9 @@ def max_pooling(attrs, inputs, cls):
'strides': 'stride',
'pads': 'pad',
})
+
new_attrs = translation_utils._add_extra_attributes(new_attrs,
-{'pool_type': 'avg',
- 'pooling_convention':
'valid'
+{'pooling_convention':
'valid'
})
new_op = translation_utils._fix_pooling('max', inputs, new_attrs)
Review comment:
Pooling is a same category for max, avg and so on. Will it help to unify
them, you translator is getting too many detailed information about each
individual operator, may be very hard for maintaining and making changes in
future.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] sandeep-krishnamurthy commented on a change in pull request #10118: [MXNET-106][ONNX-MXNET] Adding ONNX Model zoo tests.
sandeep-krishnamurthy commented on a change in pull request #10118: [MXNET-106][ONNX-MXNET] Adding ONNX Model zoo tests. URL: https://github.com/apache/incubator-mxnet/pull/10118#discussion_r177305494 ## File path: python/mxnet/contrib/onnx/_import/op_translations.py ## @@ -328,6 +354,17 @@ def squeeze(attrs, inputs, cls): mxnet_op = symbol.split(mxnet_op, axis=i-1, num_outputs=1, squeeze_axis=1) return mxnet_op, new_attrs, inputs +def take(attrs, inputs, cls): +""" Takes elements from an input array along the given axis. +Currently only slicing along axis 0 is supported for now.""" +return 'take', attrs, inputs Review comment: same as above. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sandeep-krishnamurthy commented on a change in pull request #10118: [MXNET-106][ONNX-MXNET] Adding ONNX Model zoo tests.
sandeep-krishnamurthy commented on a change in pull request #10118: [MXNET-106][ONNX-MXNET] Adding ONNX Model zoo tests. URL: https://github.com/apache/incubator-mxnet/pull/10118#discussion_r177305478 ## File path: python/mxnet/contrib/onnx/_import/op_translations.py ## @@ -328,6 +354,17 @@ def squeeze(attrs, inputs, cls): mxnet_op = symbol.split(mxnet_op, axis=i-1, num_outputs=1, squeeze_axis=1) return mxnet_op, new_attrs, inputs +def take(attrs, inputs, cls): +""" Takes elements from an input array along the given axis. +Currently only slicing along axis 0 is supported for now.""" +return 'take', attrs, inputs + +def flatten(attrs, inputs, cls): +"""Flattens the input array into a 2-D array by collapsing the higher dimensions.""" +#Mxnet does not have axis support. +new_attrs = translation_utils._remove_attributes(attrs, ['axis']) Review comment: Should we show warning or error from here on such condition? It will be hard for users to go and refer documentation This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sandeep-krishnamurthy commented on a change in pull request #10118: [MXNET-106][ONNX-MXNET] Adding ONNX Model zoo tests.
sandeep-krishnamurthy commented on a change in pull request #10118: [MXNET-106][ONNX-MXNET] Adding ONNX Model zoo tests. URL: https://github.com/apache/incubator-mxnet/pull/10118#discussion_r177271211 ## File path: tests/python-pytest/onnx/backend.py ## @@ -123,7 +124,10 @@ def run_node(cls, node, inputs, device='CPU'): mod.bind(for_training=False, data_shapes=data_shapes, label_shapes=None) # initializing parameters for calculating result of each individual node -mod.init_params() +if arg_params is None or aux_params is None: Review comment: If arg_params is not None and aux_params are None? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sandeep-krishnamurthy commented on a change in pull request #10118: [MXNET-106][ONNX-MXNET] Adding ONNX Model zoo tests.
sandeep-krishnamurthy commented on a change in pull request #10118:
[MXNET-106][ONNX-MXNET] Adding ONNX Model zoo tests.
URL: https://github.com/apache/incubator-mxnet/pull/10118#discussion_r177269859
##
File path: python/mxnet/contrib/onnx/_import/op_translations.py
##
@@ -402,9 +438,9 @@ def max_pooling(attrs, inputs, cls):
'strides': 'stride',
'pads': 'pad',
})
+
new_attrs = translation_utils._add_extra_attributes(new_attrs,
-{'pool_type': 'avg',
- 'pooling_convention':
'valid'
+{'pooling_convention':
'valid'
})
new_op = translation_utils._fix_pooling('max', inputs, new_attrs)
Review comment:
Would it be rather better approach to have a translator for Pooling operator
(also applicable for other category of operators) and all this magic of fix_*
calls is handled within it?
For example, how do I know for pooling, I need to call fix_pooling?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] sandeep-krishnamurthy commented on a change in pull request #10118: [MXNET-106][ONNX-MXNET] Adding ONNX Model zoo tests.
sandeep-krishnamurthy commented on a change in pull request #10118: [MXNET-106][ONNX-MXNET] Adding ONNX Model zoo tests. URL: https://github.com/apache/incubator-mxnet/pull/10118#discussion_r177271067 ## File path: tests/python-pytest/onnx/backend.py ## @@ -94,12 +94,13 @@ def run_node(cls, node, inputs, device='CPU'): result obtained after running the operator """ graph = GraphProto() -sym, _ = graph.from_onnx(MXNetBackend.make_graph(node, inputs)) -data_names = [i for i in sym.get_internals().list_inputs()] +sym, arg_params, aux_params = graph.from_onnx(MXNetBackend.make_graph(node, inputs)) +data_names = [i for i in sym.list_inputs() if i not in (arg_params, aux_params)] Review comment: nit: i -> something more meaningful This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sandeep-krishnamurthy commented on a change in pull request #10118: [MXNET-106][ONNX-MXNET] Adding ONNX Model zoo tests.
sandeep-krishnamurthy commented on a change in pull request #10118: [MXNET-106][ONNX-MXNET] Adding ONNX Model zoo tests. URL: https://github.com/apache/incubator-mxnet/pull/10118#discussion_r177269413 ## File path: python/mxnet/contrib/onnx/_import/op_translations.py ## @@ -328,6 +354,17 @@ def squeeze(attrs, inputs, cls): mxnet_op = symbol.split(mxnet_op, axis=i-1, num_outputs=1, squeeze_axis=1) return mxnet_op, new_attrs, inputs +def take(attrs, inputs, cls): +""" Takes elements from an input array along the given axis. +Currently only slicing along axis 0 is supported for now.""" +return 'take', attrs, inputs + +def flatten(attrs, inputs, cls): +"""Flattens the input array into a 2-D array by collapsing the higher dimensions.""" +#Mxnet does not have axis support. +new_attrs = translation_utils._remove_attributes(attrs, ['axis']) Review comment: How does user know about this? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sandeep-krishnamurthy commented on a change in pull request #10118: [MXNET-106][ONNX-MXNET] Adding ONNX Model zoo tests.
sandeep-krishnamurthy commented on a change in pull request #10118:
[MXNET-106][ONNX-MXNET] Adding ONNX Model zoo tests.
URL: https://github.com/apache/incubator-mxnet/pull/10118#discussion_r177271919
##
File path: tests/python-pytest/onnx/onnx_test.py
##
@@ -126,11 +144,111 @@ def test_super_resolution_example():
output_img_dim = 672
input_image, img_cb, img_cr = super_resolution.get_test_image()
-result_img = super_resolution.perform_inference(sym, params, input_image,
-img_cb, img_cr)
+result_img = super_resolution.perform_inference(sym, arg_params,
aux_params,
+input_image, img_cb,
img_cr)
assert hashlib.md5(result_img.tobytes()).hexdigest() ==
'0d98393a49b1d9942106a2ed89d1e854'
assert result_img.size == (output_img_dim, output_img_dim)
+def get_test_files(name):
+"""Extract tar file and returns model path and input, output data"""
+tar_name = download(URLS.get(name), dirname=CURR_PATH.__str__())
+# extract tar file
+tar_path = os.path.join(CURR_PATH, tar_name)
+tar = tarfile.open(tar_path.__str__(), "r:*")
+tar.extractall(path=CURR_PATH.__str__())
+tar.close()
+data_dir = os.path.join(CURR_PATH, name)
+model_path = os.path.join(data_dir, 'model.onnx')
+
+inputs = []
+outputs = []
+# get test files
+for test_file in os.listdir(data_dir):
+case_dir = os.path.join(data_dir, test_file)
+# skip the non-dir files
+if not os.path.isdir(case_dir):
+continue
+input_file = os.path.join(case_dir, 'input_0.pb')
+input_tensor = TensorProto()
+with open(input_file, 'rb') as proto_file:
+input_tensor.ParseFromString(proto_file.read())
+inputs.append(numpy_helper.to_array(input_tensor))
+
+output_tensor = TensorProto()
+output_file = os.path.join(case_dir, 'output_0.pb')
+with open(output_file, 'rb') as proto_file:
+output_tensor.ParseFromString(proto_file.read())
+outputs.append(numpy_helper.to_array(output_tensor))
+
+return model_path, inputs, outputs
+
+def test_bvlc_googlenet():
+""" Tests Googlenet model"""
+model_path, inputs, outputs = get_test_files('bvlc_googlenet')
+logging.info("Translating Googlenet model from ONNX to Mxnet")
+sym, arg_params, aux_params = onnx_mxnet.import_model(model_path)
+
+# run test for each test file
+for input_data, output_data in zip(inputs, outputs):
+# create module
+mod = mx.mod.Module(symbol=sym, data_names=['input_0'],
context=mx.cpu(), label_names=None)
+mod.bind(for_training=False, data_shapes=[('input_0',
input_data.shape)], label_shapes=None)
+mod.set_params(arg_params=arg_params, aux_params=aux_params,
+ allow_missing=True, allow_extra=True)
+# run inference
+batch = namedtuple('Batch', ['data'])
+mod.forward(batch([mx.nd.array(input_data)]), is_train=False)
+
+# verify the results
+npt.assert_equal(mod.get_outputs()[0].shape, output_data.shape)
+npt.assert_almost_equal(output_data, mod.get_outputs()[0].asnumpy(),
decimal=3)
+logging.info("Googlenet model conversion Successful")
+
+def test_bvlc_reference_caffenet():
+"""Tests the bvlc cafenet model"""
+model_path, inputs, outputs = get_test_files('bvlc_reference_caffenet')
+logging.info("Translating Caffenet model from ONNX to Mxnet")
+sym, arg_params, aux_params = onnx_mxnet.import_model(model_path)
+
+# run test for each test file
+for input_data, output_data in zip(inputs, outputs):
+# create module
+mod = mx.mod.Module(symbol=sym, data_names=['input_0'],
context=mx.cpu(), label_names=None)
Review comment:
Is there a way to get data_names from ONNX model?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] sandeep-krishnamurthy commented on a change in pull request #10118: [MXNET-106][ONNX-MXNET] Adding ONNX Model zoo tests.
sandeep-krishnamurthy commented on a change in pull request #10118: [MXNET-106][ONNX-MXNET] Adding ONNX Model zoo tests. URL: https://github.com/apache/incubator-mxnet/pull/10118#discussion_r177269447 ## File path: python/mxnet/contrib/onnx/_import/op_translations.py ## @@ -328,6 +354,17 @@ def squeeze(attrs, inputs, cls): mxnet_op = symbol.split(mxnet_op, axis=i-1, num_outputs=1, squeeze_axis=1) return mxnet_op, new_attrs, inputs +def take(attrs, inputs, cls): +""" Takes elements from an input array along the given axis. +Currently only slicing along axis 0 is supported for now.""" +return 'take', attrs, inputs Review comment: if axis is not 0? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sandeep-krishnamurthy commented on a change in pull request #10118: [MXNET-106][ONNX-MXNET] Adding ONNX Model zoo tests.
sandeep-krishnamurthy commented on a change in pull request #10118:
[MXNET-106][ONNX-MXNET] Adding ONNX Model zoo tests.
URL: https://github.com/apache/incubator-mxnet/pull/10118#discussion_r177271724
##
File path: tests/python-pytest/onnx/onnx_test.py
##
@@ -21,19 +21,37 @@
ONNX backend test framework. Once we have PRs on the ONNX repo and get
those PRs merged, this file will get EOL'ed.
"""
+# pylint: disable=too-many-locals,wrong-import-position,import-error
from __future__ import absolute_import
import sys
import os
import unittest
import logging
import hashlib
+import tarfile
+from collections import namedtuple
import numpy as np
import numpy.testing as npt
from onnx import helper
-import backend as mxnet_backend
+from onnx import numpy_helper
+from onnx import TensorProto
+from mxnet.test_utils import download
+from mxnet.contrib import onnx as onnx_mxnet
+import mxnet as mx
CURR_PATH = os.path.dirname(os.path.abspath(os.path.expanduser(__file__)))
sys.path.insert(0, os.path.join(CURR_PATH, '../../python/unittest'))
from common import with_seed
+import backend as mxnet_backend
+
+
+URLS = {
+'bvlc_googlenet' :
Review comment:
how big are these models? what is the impact on PR builds and nightly
builds?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] sandeep-krishnamurthy commented on a change in pull request #10118: [MXNET-106][ONNX-MXNET] Adding ONNX Model zoo tests.
sandeep-krishnamurthy commented on a change in pull request #10118: [MXNET-106][ONNX-MXNET] Adding ONNX Model zoo tests. URL: https://github.com/apache/incubator-mxnet/pull/10118#discussion_r177271405 ## File path: tests/python-pytest/onnx/onnx_backend_test.py ## @@ -57,37 +57,40 @@ 'test_floor', ## Joining and spliting -#'test_concat.*', #---Failing test +'test_concat', #Basic neural network functions 'test_sigmoid', 'test_relu', -#'test_constant_pad', -#'test_edge_pad', -#'test_reflect_pad', +'test_constant_pad', +'test_edge_pad', +'test_reflect_pad', 'test_matmul', 'test_leakyrelu', 'test_elu', -#'test_softmax*', +'test_softmax_example', +'test_softmax_large_number', +'test_softmax_axis_2', 'test_conv', 'test_basic_conv', -#'test_globalmaxpool', -#'test_globalaveragepool', -#'test_batch_norm', +'test_transpose', +#'test_globalmaxpool', - tests to be added +#'test_globalaveragepool', - tests to be added +#'test_batch_norm', - tests to be added +#'test_gather', Review comment: Can you please add these tests as part of this PR, because, these operators are added in this PR. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sandeep-krishnamurthy commented on a change in pull request #10118: [MXNET-106][ONNX-MXNET] Adding ONNX Model zoo tests.
sandeep-krishnamurthy commented on a change in pull request #10118:
[MXNET-106][ONNX-MXNET] Adding ONNX Model zoo tests.
URL: https://github.com/apache/incubator-mxnet/pull/10118#discussion_r177270889
##
File path: python/mxnet/contrib/onnx/_import/translation_utils.py
##
@@ -90,10 +90,46 @@ def _fix_pooling(pool_type, inputs, new_attr):
stride = new_attr.get('stride')
kernel = new_attr.get('kernel')
padding = new_attr.get('pad')
-pad_width = (0, 0, 0, 0) + _pad_sequence_fix(padding, len(kernel))
-new_pad_op = symbol.pad(inputs[0], mode='constant', pad_width=pad_width)
-new_pooling_op = symbol.Pooling(new_pad_op, pool_type=pool_type,
-stride=stride, kernel=kernel)
+
+# Adding default stride.
+if stride is None:
+stride = (1,) * len(kernel)
+
+# Add padding attr if not provided.
+if padding is None:
+padding = (0,) * len(kernel) * 2
+
+# Mxnet Pad operator supports only 4D/5D tensors.
+# For 1D case, these are the steps:
+#Step 1. Add extra dummy dimension to make it 4D. Adding to axis = 2
+#Step 2. Apply padding to this changed tensor
+#Step 3. Remove the extra dimension added in step 1.
+if len(kernel) == 1:
+dummy_axis = 2
+# setting 0 padding to the new dim to be added.
+padding = (0, padding[0], 0, padding[1])
+pad_width = (0, 0, 0, 0) + _pad_sequence_fix(padding, kernel_dim=2)
+
+# Step 1.
+curr_sym = symbol.expand_dims(inputs[0], axis=dummy_axis)
+
+# Step 2. Common for all tensor sizes
+new_pad_op = symbol.pad(curr_sym, mode='edge', pad_width=pad_width)
+
+# Step 3: Removing extra dim added.
+new_pad_op = symbol.split(new_pad_op, axis=dummy_axis, num_outputs=1,
squeeze_axis=1)
+else:
+# For 2D/3D cases:
+# Apply padding
+pad_width = (0, 0, 0, 0) + _pad_sequence_fix(padding,
kernel_dim=len(kernel))
+curr_sym = inputs[0]
+if pool_type == 'max':
+new_pad_op = symbol.pad(curr_sym, mode='edge', pad_width=pad_width)
+else:
+new_pad_op = symbol.pad(curr_sym, mode='constant',
pad_width=pad_width)
+
+# Apply pooling without pads.
+new_pooling_op = symbol.Pooling(new_pad_op, pool_type=pool_type,
stride=stride, kernel=kernel)
Review comment:
why not pad with pooling operator?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] sandeep-krishnamurthy commented on a change in pull request #10118: [MXNET-106][ONNX-MXNET] Adding ONNX Model zoo tests.
sandeep-krishnamurthy commented on a change in pull request #10118:
[MXNET-106][ONNX-MXNET] Adding ONNX Model zoo tests.
URL: https://github.com/apache/incubator-mxnet/pull/10118#discussion_r177270800
##
File path: python/mxnet/contrib/onnx/_import/translation_utils.py
##
@@ -90,10 +90,46 @@ def _fix_pooling(pool_type, inputs, new_attr):
stride = new_attr.get('stride')
kernel = new_attr.get('kernel')
padding = new_attr.get('pad')
-pad_width = (0, 0, 0, 0) + _pad_sequence_fix(padding, len(kernel))
-new_pad_op = symbol.pad(inputs[0], mode='constant', pad_width=pad_width)
-new_pooling_op = symbol.Pooling(new_pad_op, pool_type=pool_type,
-stride=stride, kernel=kernel)
+
+# Adding default stride.
+if stride is None:
+stride = (1,) * len(kernel)
+
+# Add padding attr if not provided.
+if padding is None:
+padding = (0,) * len(kernel) * 2
+
+# Mxnet Pad operator supports only 4D/5D tensors.
+# For 1D case, these are the steps:
+#Step 1. Add extra dummy dimension to make it 4D. Adding to axis = 2
+#Step 2. Apply padding to this changed tensor
+#Step 3. Remove the extra dimension added in step 1.
+if len(kernel) == 1:
+dummy_axis = 2
+# setting 0 padding to the new dim to be added.
+padding = (0, padding[0], 0, padding[1])
+pad_width = (0, 0, 0, 0) + _pad_sequence_fix(padding, kernel_dim=2)
+
+# Step 1.
+curr_sym = symbol.expand_dims(inputs[0], axis=dummy_axis)
+
+# Step 2. Common for all tensor sizes
+new_pad_op = symbol.pad(curr_sym, mode='edge', pad_width=pad_width)
+
+# Step 3: Removing extra dim added.
+new_pad_op = symbol.split(new_pad_op, axis=dummy_axis, num_outputs=1,
squeeze_axis=1)
+else:
+# For 2D/3D cases:
+# Apply padding
+pad_width = (0, 0, 0, 0) + _pad_sequence_fix(padding,
kernel_dim=len(kernel))
+curr_sym = inputs[0]
+if pool_type == 'max':
Review comment:
Please add description for mode
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] sandeep-krishnamurthy commented on a change in pull request #10118: [MXNET-106][ONNX-MXNET] Adding ONNX Model zoo tests.
sandeep-krishnamurthy commented on a change in pull request #10118:
[MXNET-106][ONNX-MXNET] Adding ONNX Model zoo tests.
URL: https://github.com/apache/incubator-mxnet/pull/10118#discussion_r177270041
##
File path: python/mxnet/contrib/onnx/_import/translation_utils.py
##
@@ -90,10 +90,46 @@ def _fix_pooling(pool_type, inputs, new_attr):
stride = new_attr.get('stride')
kernel = new_attr.get('kernel')
padding = new_attr.get('pad')
-pad_width = (0, 0, 0, 0) + _pad_sequence_fix(padding, len(kernel))
-new_pad_op = symbol.pad(inputs[0], mode='constant', pad_width=pad_width)
-new_pooling_op = symbol.Pooling(new_pad_op, pool_type=pool_type,
-stride=stride, kernel=kernel)
+
+# Adding default stride.
Review comment:
default for dilation?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
