[GitHub] anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx
Module to import onnx models into mxnet
URL: https://github.com/apache/incubator-mxnet/pull/9963#discussion_r174485137
##
File path: python/mxnet/test_utils.py
##
@@ -1407,12 +1410,15 @@ def download(url, fname=None, dirname=None,
overwrite=False):
if exc.errno != errno.EEXIST:
raise OSError('failed to create ' + dirname)
-if not overwrite and os.path.exists(fname):
+if not overwrite and os.path.exists(fname) and not version_tag:
Review comment:
Have handled it in line 1419-1420
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] anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx
Module to import onnx models into mxnet
URL: https://github.com/apache/incubator-mxnet/pull/9963#discussion_r174484918
##
File path: python/mxnet/test_utils.py
##
@@ -1407,12 +1410,15 @@ def download(url, fname=None, dirname=None,
overwrite=False):
if exc.errno != errno.EEXIST:
raise OSError('failed to create ' + dirname)
-if not overwrite and os.path.exists(fname):
+if not overwrite and os.path.exists(fname) and not version_tag:
logging.info("%s exists, skipping download", fname)
return fname
r = requests.get(url, stream=True)
assert r.status_code == 200, "failed to open %s" % url
+if version_tag and r.headers['ETag'] != version_tag:
+logging.info("The version tag of the file does not match the expected
version. "
+ + "Proceeding with the file download...")
Review comment:
the download method was already present in the test_utils module and in use
by other modules in the project. I did not want the eTag mismatch to fail any
existing tests.
Also I used this as a reference -
https://github.com/apache/incubator-mxnet/blob/master/python/mxnet/gluon/utils.py#L216-L220
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] anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet URL: https://github.com/apache/incubator-mxnet/pull/9963#discussion_r174319252 ## File path: python/mxnet/contrib/onnx/_import/import_helper.py ## @@ -0,0 +1,105 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# coding: utf-8 +# pylint: disable=invalid-name +"""Operator attributes conversion""" +from .op_translations import identity, random_uniform, random_normal +from .op_translations import add, subtract, multiply, divide, absolute, negative, add_n +from .op_translations import tanh +from .op_translations import ceil, floor +from .op_translations import concat +from .op_translations import leaky_relu, _elu, _prelu, softmax, fully_connected +from .op_translations import global_avgpooling, global_maxpooling, linalg_gemm +from .op_translations import sigmoid, pad, relu, matrix_multiplication, batch_norm +from .op_translations import dropout, local_response_norm, conv, deconv Review comment: it is not as per mxnet convention to do ```python from op_translations import * ``` It was an earlier comment by one of the reviewers/committers. 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] anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet URL: https://github.com/apache/incubator-mxnet/pull/9963#discussion_r174318632 ## File path: python/mxnet/contrib/onnx/_import/import_helper.py ## @@ -0,0 +1,105 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# coding: utf-8 +# pylint: disable=invalid-name +"""Operator attributes conversion""" +from .op_translations import identity, random_uniform, random_normal +from .op_translations import add, subtract, multiply, divide, absolute, negative, add_n +from .op_translations import tanh +from .op_translations import ceil, floor +from .op_translations import concat +from .op_translations import leaky_relu, _elu, _prelu, softmax, fully_connected +from .op_translations import global_avgpooling, global_maxpooling, linalg_gemm +from .op_translations import sigmoid, pad, relu, matrix_multiplication, batch_norm +from .op_translations import dropout, local_response_norm, conv, deconv +from .op_translations import reshape, cast, split, _slice, transpose, squeeze +from .op_translations import reciprocal, squareroot, power, exponent, _log +from .op_translations import reduce_max, reduce_mean, reduce_min, reduce_sum +from .op_translations import reduce_prod, avg_pooling, max_pooling +from .op_translations import argmax, argmin, maximum, minimum + +# convert_map defines maps of ONNX operator names to converter functor(callable) Review comment: its a typo in the comment. I will fix it. 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] anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx
Module to import onnx models into mxnet
URL: https://github.com/apache/incubator-mxnet/pull/9963#discussion_r173690286
##
File path: example/onnx/test_super_resolution.py
##
@@ -0,0 +1,112 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Testing super_resolution model conversion"""
+from __future__ import absolute_import as _abs
+from __future__ import print_function
+from collections import namedtuple
+import logging
+import numpy as np
+from PIL import Image
+import mxnet as mx
+from mxnet.test_utils import download
+import mxnet.contrib.onnx as onnx_mxnet
+
+# set up logger
+logging.basicConfig()
+LOGGER = logging.getLogger()
+LOGGER.setLevel(logging.INFO)
+
+def download_onnx_model():
+"""Download the onnx model"""
+model_url =
'https://s3.amazonaws.com/onnx-mxnet/examples/super_resolution.onnx'
+download(model_url, 'super_resolution.onnx')
+
+def import_onnx():
+"""Import the onnx model into mxnet"""
+LOGGER.info("Converting onnx format to mxnet's symbol and params...")
+sym, params = onnx_mxnet.import_model('super_resolution.onnx')
Review comment:
This PR currently has tests for all the operator translations, I plan on
making a follow up PR for adding more tests for onnx model translations. I will
add a version to the above files and make this change in that follow up PR.
Will that be okay?
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] anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx
Module to import onnx models into mxnet
URL: https://github.com/apache/incubator-mxnet/pull/9963#discussion_r173687805
##
File path: python/mxnet/contrib/onnx/_import/import_onnx.py
##
@@ -0,0 +1,166 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# coding: utf-8
+# pylint: disable=invalid-name,too-many-locals,no-self-use
+""" Support import export formats."""
+from __future__ import absolute_import as _abs
+from import symbol
+from import ndarray as nd
+from .import_helper import _convert_map, _pad_sequence_fix
+
+def _convert_operator(op_name, attrs, inputs, convert_map=None):
+"""Convert from onnx operator to mxnet operator.
+The converter must specify conversions explicitly for incompatible name,
and
+apply handlers to operator attributes.
+
+Parameters
+--
+op_name : str
+Operator name, such as Convolution, FullyConnected
+attrs : dict
+Dict of operator attributes
+inputs: list
+list of inputs to the operator
+convert_map : dict
+Dict of name : callable, where name is the op's name that
+require conversion to mxnet, callable are functions which
+take attrs and return (new_op_name, new_attrs, inputs)
+
+Returns
+---
+(op_name, attrs)
+Converted (op_name, attrs) for mxnet.
+"""
+convert_map = convert_map if convert_map else _convert_map
+if op_name in convert_map:
+op_name, attrs, inputs = convert_map[op_name](op_name, attrs, inputs)
+else:
+raise NotImplementedError("Operator {} not
implemented.".format(op_name))
+op = getattr(symbol, op_name, None)
+if not op:
+raise RuntimeError("Unable to map op_name {} to sym".format(op_name))
+return op, attrs, inputs
+
+class GraphProto(object): # pylint: disable=too-few-public-methods
+"""A helper class for handling mxnet symbol copying from pb2.GraphProto.
+Definition: https://github.com/onnx/onnx/blob/master/onnx/onnx.proto
+"""
+def __init__(self):
+self._nodes = {}
+self._params = {}
+self._renames = {}
+self._num_input = 0
+self._num_param = 0
+
+def from_onnx(self, graph):
+"""Construct symbol from onnx graph.
+The inputs from onnx graph is vague, only providing "1", "2"...
+For convenience, we rename the `real` input names to "input_0",
+"input_1"... And renaming parameters to "param_0", "param_1"...
+
+Parameters
+--
+graph : onnx protobuf object
+The loaded onnx graph
+
+Returns
+---
+sym :symbol.Symbol
+The returned mxnet symbol
+params : dict
+A dict of name: nd.array pairs, used as pretrained weights
+"""
+# parse network inputs, aka parameters
+for init_tensor in graph.initializer:
+if not init_tensor.name.strip():
+raise ValueError("Tensor's name is required.")
+self._params[init_tensor.name] = self._parse_array(init_tensor)
+
+# converting GraphProto message
+for i in graph.input:
+if i.name in self._params:
+# i is a param instead of input
+name_param = 'param_{}'.format(self._num_param)
+self._num_param += 1
+self._params[name_param] = self._params.pop(i.name)
+self._nodes[name_param] = symbol.Variable(name=name_param,
Review comment:
The param values have the dtype information, and it is being used in the
_parse_array method of the GraphProto class.
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] anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet URL: https://github.com/apache/incubator-mxnet/pull/9963#discussion_r173571169 ## File path: tests/ci_build/install/ubuntu_install_onnx.sh ## @@ -0,0 +1,26 @@ +#!/usr/bin/env bash + +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +set -e +set -x + +# install libraries for onnx's python package on ubuntu +apt-get install -y libprotobuf-dev protobuf-compiler + +pip2 install pytest==3.4.0 pytest-cov==2.5.1 protobuf==3.0.0 onnx==1.0.1 Pillow==5.0.0 tabulate==0.7.5 +pip3 install pytest==3.4.0 pytest-cov==2.5.1 protobuf==3.0.0 onnx==1.0.1 Pillow==5.0.0 tabulate==0.7.5 Review comment: @marcoabreu in this file instead of pip installing onnx version 1.0.1 would it be okay to do the following - ```bash git clone --recursive https://github.com/onnx/onnx.git cd onnx git checkout 7e205b66190f4376c64741ba7705dc23e9fbf225 python setup.py install ``` This will allow us to pull in more recent changes from the onnx repo, which will be needed for running a lot of our tests. For example, if we add a new operator later then we will also add corresponding tests on the onnx repo which will need to be run on the CI pipeline. And we cannot always rely on onnx pip package release, which could be outdated. Even in the current CI run, some tests are probably failing because the onnx 1.0.1 release is not up to date. Would this change be okay in this script or do you have any other suggestions? 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] anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx
Module to import onnx models into mxnet
URL: https://github.com/apache/incubator-mxnet/pull/9963#discussion_r173528424
##
File path: example/onnx/test_super_resolution.py
##
@@ -0,0 +1,112 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Testing super_resolution model conversion"""
+from __future__ import absolute_import as _abs
+from __future__ import print_function
+from collections import namedtuple
+import logging
+import numpy as np
+from PIL import Image
+import mxnet as mx
+from mxnet.test_utils import download
+import mxnet.contrib.onnx as onnx_mxnet
+
+# set up logger
+logging.basicConfig()
+LOGGER = logging.getLogger()
+LOGGER.setLevel(logging.INFO)
+
+def download_onnx_model():
+"""Download the onnx model"""
+model_url =
'https://s3.amazonaws.com/onnx-mxnet/examples/super_resolution.onnx'
+download(model_url, 'super_resolution.onnx')
+
+def import_onnx():
+"""Import the onnx model into mxnet"""
+LOGGER.info("Converting onnx format to mxnet's symbol and params...")
+sym, params = onnx_mxnet.import_model('super_resolution.onnx')
Review comment:
No, it currently does not. I will add the verification. Can you please point
me to some examples where this is done, for reference.
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] anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet URL: https://github.com/apache/incubator-mxnet/pull/9963#discussion_r173526997 ## File path: tests/python-pytest/onnx/test_onnx.py ## @@ -0,0 +1,86 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Tests for individual operators""" Review comment: I will move it. This file actually contains tests which currently don't exist on the ONNX repo. Once we make a couple of PRs on ONNX repo and they get merged, this file will get EOLed. 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] anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx
Module to import onnx models into mxnet
URL: https://github.com/apache/incubator-mxnet/pull/9963#discussion_r173519044
##
File path: example/onnx/test_super_resolution.py
##
@@ -0,0 +1,84 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Testing super_resolution model conversion"""
+from __future__ import absolute_import as _abs
+from __future__ import print_function
+from collections import namedtuple
+import logging
+import numpy as np
+from PIL import Image
+import mxnet as mx
+from mxnet.test_utils import download
+import mxnet.contrib.onnx as onnx_mxnet
+
+# set up logger
+logging.basicConfig()
+LOGGER = logging.getLogger()
+LOGGER.setLevel(logging.INFO)
+
+def download_onnx_model():
+"""Download the onnx model"""
+model_url =
'https://s3.amazonaws.com/onnx-mxnet/examples/super_resolution.onnx'
+download(model_url, 'super_resolution.onnx')
+
+def import_onnx():
+"""Import the onnx model into mxnet"""
+LOGGER.info("Converting onnx format to mxnet's symbol and params...")
+sym, params = onnx_mxnet.import_model('super_resolution.onnx')
+assert sym is not None
+assert params is not None
+return sym, params
+
+def get_test_image():
+"""Download and process the test image"""
+# Load test image
+input_image_dim = 224
+img_url =
'https://s3.amazonaws.com/onnx-mxnet/examples/super_res_input.jpg'
+download(img_url, 'super_res_input.jpg')
+img = Image.open('super_res_input.jpg').resize((input_image_dim,
input_image_dim))
+img_ycbcr = img.convert("YCbCr")
+img_y, img_cb, img_cr = img_ycbcr.split()
+input_image = np.array(img_y)[np.newaxis, np.newaxis, :, :]
+return input_image, img_cb, img_cr
+
+def perform_inference((sym, params), (input_img, img_cb, img_cr)):
+"""Perform inference on image using mxnet"""
+# create module
+mod = mx.mod.Module(symbol=sym, data_names=['input_0'], label_names=None)
+mod.bind(for_training=False, data_shapes=[('input_0', input_img.shape)])
+mod.set_params(arg_params=params, aux_params=None)
+
+# run inference
+batch = namedtuple('Batch', ['data'])
+mod.forward(batch([mx.nd.array(input_img)]))
+
+# Save the result
+img_out_y = Image.fromarray(np.uint8(mod.get_outputs()[0][0][0].
+ asnumpy().clip(0, 255)), mode='L')
+
+result_img = Image.merge(
+"YCbCr", [img_out_y,
+ img_cb.resize(img_out_y.size, Image.BICUBIC),
+ img_cr.resize(img_out_y.size, Image.BICUBIC)]).convert("RGB")
+output_img_dim = 672
+assert result_img.size == (output_img_dim, output_img_dim)
Review comment:
the model is supposed to convert a low resolution image into a high
resolution image with specific dimensions, so I have asserted for the size of
the result image. What else will I have to assert for?
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] anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet URL: https://github.com/apache/incubator-mxnet/pull/9963#discussion_r173516274 ## File path: tests/python-pytest/onnx/test_onnx.py ## @@ -0,0 +1,86 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Tests for individual operators""" + +from __future__ import absolute_import +import sys +import os +import unittest +import numpy as np +import numpy.testing as npt +from onnx import helper +import backend as mxnet_backend +CURR_PATH = os.path.dirname(os.path.abspath(os.path.expanduser(__file__))) +sys.path.insert(0, os.path.join(CURR_PATH, '../../python/unittest')) Review comment: It is for the withseed decorator. 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] anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx
Module to import onnx models into mxnet
URL: https://github.com/apache/incubator-mxnet/pull/9963#discussion_r173516139
##
File path: Jenkinsfile
##
@@ -649,7 +649,20 @@ try {
}
stage('Integration Test') {
-parallel 'Python GPU': {
+ parallel 'Onnx CPU': {
+ node('mxnetlinux-cpu') {
+ws('workspace/it-onnx-cpu') {
+ init_git()
+ unpack_lib('cpu')
+ timeout(time: max_time, unit: 'MINUTES') {
+sh "${docker_run} cpu --dockerbinary docker PYTHONPATH=./python/
pytest tests/python-pytest/onnx/onnx_backend_test.py"
+sh "${docker_run} cpu --dockerbinary docker PYTHONPATH=./python/
pytest tests/python-pytest/onnx/test_onnx.py"
Review comment:
@marcoabreu do you mean that either both should be ("test_onnx" and
"test_onnx_backend") or ("onnx_backend_test" and "onnx_test")?
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] anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx
Module to import onnx models into mxnet
URL: https://github.com/apache/incubator-mxnet/pull/9963#discussion_r173511064
##
File path: example/onnx/test_super_resolution.py
##
@@ -0,0 +1,84 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Testing super_resolution model conversion"""
+from __future__ import absolute_import as _abs
+from __future__ import print_function
+from collections import namedtuple
+import logging
+import numpy as np
+from PIL import Image
+import mxnet as mx
+from mxnet.test_utils import download
+import mxnet.contrib.onnx as onnx_mxnet
+
+# set up logger
+logging.basicConfig()
+LOGGER = logging.getLogger()
+LOGGER.setLevel(logging.INFO)
+
+def download_onnx_model():
+"""Download the onnx model"""
+model_url =
'https://s3.amazonaws.com/onnx-mxnet/examples/super_resolution.onnx'
+download(model_url, 'super_resolution.onnx')
+
+def import_onnx():
+"""Import the onnx model into mxnet"""
+LOGGER.info("Converting onnx format to mxnet's symbol and params...")
+sym, params = onnx_mxnet.import_model('super_resolution.onnx')
+assert sym is not None
+assert params is not None
+return sym, params
+
+def get_test_image():
+"""Download and process the test image"""
+# Load test image
+input_image_dim = 224
+img_url =
'https://s3.amazonaws.com/onnx-mxnet/examples/super_res_input.jpg'
+download(img_url, 'super_res_input.jpg')
+img = Image.open('super_res_input.jpg').resize((input_image_dim,
input_image_dim))
+img_ycbcr = img.convert("YCbCr")
+img_y, img_cb, img_cr = img_ycbcr.split()
+input_image = np.array(img_y)[np.newaxis, np.newaxis, :, :]
+return input_image, img_cb, img_cr
+
+def perform_inference((sym, params), (input_img, img_cb, img_cr)):
+"""Perform inference on image using mxnet"""
+# create module
+mod = mx.mod.Module(symbol=sym, data_names=['input_0'], label_names=None)
+mod.bind(for_training=False, data_shapes=[('input_0', input_img.shape)])
+mod.set_params(arg_params=params, aux_params=None)
+
+# run inference
+batch = namedtuple('Batch', ['data'])
+mod.forward(batch([mx.nd.array(input_img)]))
+
+# Save the result
+img_out_y = Image.fromarray(np.uint8(mod.get_outputs()[0][0][0].
+ asnumpy().clip(0, 255)), mode='L')
+
+result_img = Image.merge(
+"YCbCr", [img_out_y,
+ img_cb.resize(img_out_y.size, Image.BICUBIC),
+ img_cr.resize(img_out_y.size, Image.BICUBIC)]).convert("RGB")
+output_img_dim = 672
+assert result_img.size == (output_img_dim, output_img_dim)
Review comment:
@marcoabreu added more asserts in the import_onnx() method where the actual
model import is happening.
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] anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet URL: https://github.com/apache/incubator-mxnet/pull/9963#discussion_r173336298 ## File path: python/mxnet/contrib/__init__.py ## @@ -28,5 +28,5 @@ from . import tensorboard from . import text - +from . import onnx Review comment: No, there is no namespace clash. The import module in onnx folder is name '_import'. 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] anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet URL: https://github.com/apache/incubator-mxnet/pull/9963#discussion_r173335708 ## File path: example/onnx/test_super_resolution.py ## @@ -0,0 +1,62 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Testing super_resolution model conversion""" Review comment: it has been added 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] anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet URL: https://github.com/apache/incubator-mxnet/pull/9963#discussion_r173257102 ## File path: python/mxnet/contrib/__init__.py ## @@ -28,5 +28,5 @@ from . import tensorboard from . import text - +from . import onnx Review comment: This import is for mxnet.contrib.onnx package which will ship as part of the code and not the 'onnx' package which we are installing in CI setup process. So package will always be present. Following is the folder structure of the new mxnet.contrib.onnx module that is being added in this PR.  Please let me know if this wrong, and if I have to change something. 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] anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet URL: https://github.com/apache/incubator-mxnet/pull/9963#discussion_r173257102 ## File path: python/mxnet/contrib/__init__.py ## @@ -28,5 +28,5 @@ from . import tensorboard from . import text - +from . import onnx Review comment: This import is for mxnet.contrib.onnx package which will ship as part of the code and not the 'onnx' package which we are installing in CI setup process. So package will always be present. Following is the folder structure of the new mxnet.contrib.onnx module that is being added in this PR.  Please let me know if this wrong, and I have change something. 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] anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet URL: https://github.com/apache/incubator-mxnet/pull/9963#discussion_r173257102 ## File path: python/mxnet/contrib/__init__.py ## @@ -28,5 +28,5 @@ from . import tensorboard from . import text - +from . import onnx Review comment: This import is for mxnet.contrib.onnx package which will ship as part of the code and not the 'onnx' package which we are installing in CI setup process. So package will always be present. Following is the folder structure of the new mxnet.contrib.onnx module that is being added in this PR. mxnet |-- contrib | |-- onnx | |-- _import |-- gluon |-- ... .. Please let me know if this wrong, and I have change something. 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] anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx
Module to import onnx models into mxnet
URL: https://github.com/apache/incubator-mxnet/pull/9963#discussion_r173253700
##
File path: example/onnx/test_super_resolution.py
##
@@ -0,0 +1,62 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Testing super_resolution model conversion"""
+# pylint: disable=invalid-name
+from __future__ import absolute_import as _abs
+from __future__ import print_function
+from collections import namedtuple
+import mxnet as mx
+from mxnet.test_utils import download
+import mxnet.contrib.onnx as onnx_mxnet
+import numpy as np
+from PIL import Image
+
+model_url =
'https://s3.amazonaws.com/onnx-mxnet/examples/super_resolution.onnx'
+
+download(model_url, 'super_resolution.onnx')
+
+print("Converting onnx format to mxnet's symbol and params...")
Review comment:
okay. I will make this change.
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] anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx
Module to import onnx models into mxnet
URL: https://github.com/apache/incubator-mxnet/pull/9963#discussion_r173253623
##
File path: Jenkinsfile
##
@@ -649,7 +649,20 @@ try {
}
stage('Integration Test') {
-parallel 'Python GPU': {
+ parallel 'Python CPU': {
+ node('mxnetlinux-cpu') {
+ws('workspace/it-python-cpu') {
Review comment:
will fix 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] anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx
Module to import onnx models into mxnet
URL: https://github.com/apache/incubator-mxnet/pull/9963#discussion_r173252880
##
File path: Jenkinsfile
##
@@ -649,7 +649,20 @@ try {
}
stage('Integration Test') {
-parallel 'Python GPU': {
+ parallel 'Python CPU': {
+ node('mxnetlinux-cpu') {
+ws('workspace/it-python-cpu') {
+ init_git()
+ unpack_lib('cpu')
+ timeout(time: max_time, unit: 'MINUTES') {
+sh "${docker_run} cpu --dockerbinary docker PYTHONPATH=./python/
python tests/python/integrationtest/onnx_backend_test.py"
+sh "${docker_run} cpu --dockerbinary docker PYTHONPATH=./python/
python tests/python/integrationtest/test_layers.py"
Review comment:
onnx_backend_test uses pytest.
I will shift the whole folder from "tests/python/integrationtest" to
"tests/integrationtests". I will run all onnx related tests using pytest. And
make corresponding changes in Jenkinsfile. Will that be alright?
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] anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet URL: https://github.com/apache/incubator-mxnet/pull/9963#discussion_r173200345 ## File path: tests/python/integrationtest/test_layers.py ## @@ -0,0 +1,92 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Tests for individual operators""" +# pylint: disable=import-error,no-self-use + +from __future__ import absolute_import +import sys +import os +import unittest +import numpy as np +import numpy.testing as npt +from onnx import helper +import backend as mxnet_backend +curr_path = os.path.dirname(os.path.abspath(os.path.expanduser(__file__))) +sys.path.insert(0, os.path.join(curr_path, '../unittest')) +from common import with_seed + +class TestLayers(unittest.TestCase): +"""Tests for different layers comparing output with numpy operators. +Temporary file until we have a corresponding test in onnx-backend_test +for these operators.""" + +@with_seed(0) Review comment: will use with_seed() instead of the default 0 value. 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] anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx
Module to import onnx models into mxnet
URL: https://github.com/apache/incubator-mxnet/pull/9963#discussion_r173194696
##
File path: Jenkinsfile
##
@@ -649,7 +649,20 @@ try {
}
stage('Integration Test') {
-parallel 'Python GPU': {
+ parallel 'Python CPU': {
+ node('mxnetlinux-cpu') {
+ws('workspace/it-python-cpu') {
+ init_git()
+ unpack_lib('cpu')
+ timeout(time: max_time, unit: 'MINUTES') {
+sh "${docker_run} cpu --dockerbinary docker PYTHONPATH=./python/
python tests/python/integrationtest/onnx_backend_test.py"
+sh "${docker_run} cpu --dockerbinary docker PYTHONPATH=./python/
python tests/python/integrationtest/test_layers.py"
Review comment:
test_layers is running nosetest and I will change the command, but
onnx_backend_test is not using nosetest
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] anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
anirudhacharya commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet URL: https://github.com/apache/incubator-mxnet/pull/9963#discussion_r173182657 ## File path: python/mxnet/contrib/__init__.py ## @@ -28,5 +28,5 @@ from . import tensorboard from . import text - +from . import onnx Review comment: This is the import for Mxnet's onnx package, which is why I have given the relative path. 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
