[GitHub] marcoabreu commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
marcoabreu 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_r174539726
##
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:
Oh right, good point. We should rather fail if the file is not the one we
are expecting.
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] marcoabreu commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
marcoabreu 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_r174321757 ## File path: python/mxnet/contrib/onnx/_import/import_model.py ## @@ -0,0 +1,46 @@ +# 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 +"""import function""" +# pylint: disable=no-member + +from .import_onnx import GraphProto + +def import_model(model_file): +"""Imports the supplied ONNX model file into MXNet symbol and parameters. +:parameters model_file +-- +model_file : ONNX model file name + +:returns (sym, params) Review comment: The rendered page is available at http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-9963/54/index.html 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] marcoabreu commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
marcoabreu 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_r174206700
##
File path: tests/python-pytest/onnx/onnx_test.py
##
@@ -0,0 +1,133 @@
+# 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
+This module contains operator tests which currently do not exist on
+ONNX backend test framework. Once we have PRs on the ONNX repo and get
+those PRs merged, this file will get EOL'ed.
+"""
+from __future__ import absolute_import
+import sys
+import os
+import unittest
+import logging
+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'))
+from common import with_seed
+
+@with_seed()
+def test_reduce_max():
+"""Test for ReduceMax operator"""
+node_def = helper.make_node("ReduceMax", ["input1"], ["output"], axes=[1,
0], keepdims=1)
+input1 = np.random.ranf([3, 10]).astype("float32")
+output = mxnet_backend.run_node(node_def, [input1])[0]
+numpy_op = np.max(input1, axis=(1, 0), keepdims=True)
+npt.assert_almost_equal(output, numpy_op)
+
+@with_seed()
+def test_reduce_mean():
+"""Test for ReduceMean operator"""
+node_def = helper.make_node("ReduceMean", ["input1"], ["output"], axes=[1,
0], keepdims=1)
+input1 = np.random.ranf([3, 10]).astype("float32")
+output = mxnet_backend.run_node(node_def, [input1])[0]
+numpy_op = np.mean(input1, axis=(1, 0), keepdims=True)
+npt.assert_almost_equal(output, numpy_op, decimal=5)
+
+@with_seed()
+def test_reduce_min():
+"""Test for ReduceMin operator"""
+node_def = helper.make_node("ReduceMin", ["input1"], ["output"], axes=[1,
0], keepdims=1)
+input1 = np.random.ranf([3, 10]).astype("float32")
+output = mxnet_backend.run_node(node_def, [input1])[0]
+numpy_op = np.min(input1, axis=(1, 0), keepdims=True)
+npt.assert_almost_equal(output, numpy_op)
+
+@with_seed()
+def test_reduce_sum():
+"""Test for ReduceSum operator"""
+node_def = helper.make_node("ReduceSum", ["input1"], ["output"], axes=[1,
0], keepdims=1)
+input1 = np.random.ranf([3, 10]).astype("float32")
+output = mxnet_backend.run_node(node_def, [input1])[0]
+numpy_op = np.sum(input1, axis=(1, 0), keepdims=True)
+npt.assert_almost_equal(output, numpy_op, decimal=5)
+
+@with_seed()
+def test_reduce_prod():
+"""Test for ReduceProd operator"""
+node_def = helper.make_node("ReduceProd", ["input1"], ["output"], axes=[1,
0], keepdims=1)
+input1 = np.random.ranf([3, 10]).astype("float32")
+output = mxnet_backend.run_node(node_def, [input1])[0]
+numpy_op = np.prod(input1, axis=(1, 0), keepdims=True)
+npt.assert_almost_equal(output, numpy_op, decimal=5)
+
+@with_seed()
+def test_squeeze():
+"""Test for Squeeze operator"""
+node_def = helper.make_node("Squeeze", ["input1"], ["output"], axes=[1, 3])
+input1 = np.random.ranf([3, 1, 2, 1, 4]).astype("float32")
+output = mxnet_backend.run_node(node_def, [input1])[0]
+npt.assert_almost_equal(output, np.squeeze(input1, axis=[1, 3]))
+
+def test_super_resolution_example():
+"""Test the super resolution example in the example/onnx folder"""
+sys.path.insert(0, os.path.join(CURR_PATH, '../../../example/onnx/'))
+import super_resolution
+
+sym, params = super_resolution.import_onnx()
+assert sym is not None
+assert params is not None
+
+inputs = sym.list_inputs()
+assert len(inputs) == 9
+for i, input_param in enumerate(['param_7', 'param_5', 'param_3',
'param_1',
+ 'input_0', 'param_0', 'param_2',
'param_4', 'param_6']):
+assert inputs[i] == input_param
+
+assert len(sym.list_outputs()) == 1
+assert sym.list_outputs()[0] == 'reshape5_output'
+
+attrs_keys = sym.attr_dict().keys()
+assert len(attrs_keys) == 19
+for i, key_item in enumerate(['reshape4', 'param_5', 'param_4', 'param_7',
+ 'param_6', 'param_1',
[GitHub] marcoabreu commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
marcoabreu 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_r174206407
##
File path: tests/python-pytest/onnx/onnx_test.py
##
@@ -0,0 +1,133 @@
+# 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
+This module contains operator tests which currently do not exist on
+ONNX backend test framework. Once we have PRs on the ONNX repo and get
+those PRs merged, this file will get EOL'ed.
+"""
+from __future__ import absolute_import
+import sys
+import os
+import unittest
+import logging
+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'))
+from common import with_seed
+
+@with_seed()
+def test_reduce_max():
+"""Test for ReduceMax operator"""
+node_def = helper.make_node("ReduceMax", ["input1"], ["output"], axes=[1,
0], keepdims=1)
+input1 = np.random.ranf([3, 10]).astype("float32")
+output = mxnet_backend.run_node(node_def, [input1])[0]
+numpy_op = np.max(input1, axis=(1, 0), keepdims=True)
+npt.assert_almost_equal(output, numpy_op)
+
+@with_seed()
+def test_reduce_mean():
+"""Test for ReduceMean operator"""
+node_def = helper.make_node("ReduceMean", ["input1"], ["output"], axes=[1,
0], keepdims=1)
+input1 = np.random.ranf([3, 10]).astype("float32")
+output = mxnet_backend.run_node(node_def, [input1])[0]
+numpy_op = np.mean(input1, axis=(1, 0), keepdims=True)
+npt.assert_almost_equal(output, numpy_op, decimal=5)
+
+@with_seed()
+def test_reduce_min():
+"""Test for ReduceMin operator"""
+node_def = helper.make_node("ReduceMin", ["input1"], ["output"], axes=[1,
0], keepdims=1)
+input1 = np.random.ranf([3, 10]).astype("float32")
+output = mxnet_backend.run_node(node_def, [input1])[0]
+numpy_op = np.min(input1, axis=(1, 0), keepdims=True)
+npt.assert_almost_equal(output, numpy_op)
+
+@with_seed()
+def test_reduce_sum():
+"""Test for ReduceSum operator"""
+node_def = helper.make_node("ReduceSum", ["input1"], ["output"], axes=[1,
0], keepdims=1)
+input1 = np.random.ranf([3, 10]).astype("float32")
+output = mxnet_backend.run_node(node_def, [input1])[0]
+numpy_op = np.sum(input1, axis=(1, 0), keepdims=True)
+npt.assert_almost_equal(output, numpy_op, decimal=5)
+
+@with_seed()
+def test_reduce_prod():
+"""Test for ReduceProd operator"""
+node_def = helper.make_node("ReduceProd", ["input1"], ["output"], axes=[1,
0], keepdims=1)
+input1 = np.random.ranf([3, 10]).astype("float32")
+output = mxnet_backend.run_node(node_def, [input1])[0]
+numpy_op = np.prod(input1, axis=(1, 0), keepdims=True)
+npt.assert_almost_equal(output, numpy_op, decimal=5)
+
+@with_seed()
+def test_squeeze():
+"""Test for Squeeze operator"""
+node_def = helper.make_node("Squeeze", ["input1"], ["output"], axes=[1, 3])
+input1 = np.random.ranf([3, 1, 2, 1, 4]).astype("float32")
+output = mxnet_backend.run_node(node_def, [input1])[0]
+npt.assert_almost_equal(output, np.squeeze(input1, axis=[1, 3]))
+
+def test_super_resolution_example():
+"""Test the super resolution example in the example/onnx folder"""
+sys.path.insert(0, os.path.join(CURR_PATH, '../../../example/onnx/'))
+import super_resolution
+
+sym, params = super_resolution.import_onnx()
+assert sym is not None
+assert params is not None
+
+inputs = sym.list_inputs()
+assert len(inputs) == 9
+for i, input_param in enumerate(['param_7', 'param_5', 'param_3',
'param_1',
+ 'input_0', 'param_0', 'param_2',
'param_4', 'param_6']):
+assert inputs[i] == input_param
+
+assert len(sym.list_outputs()) == 1
+assert sym.list_outputs()[0] == 'reshape5_output'
+
+attrs_keys = sym.attr_dict().keys()
+assert len(attrs_keys) == 19
+for i, key_item in enumerate(['reshape4', 'param_5', 'param_4', 'param_7',
+ 'param_6', 'param_1',
[GitHub] marcoabreu commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
marcoabreu 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_r174191440 ## File path: tests/python-pytest/onnx/onnx_test.py ## @@ -0,0 +1,138 @@ +# 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 +This module contains operator tests which currently do not exist on +ONNX backend test framework. Once we have PRs on the ONNX repo and get +those PRs merged, this file will get EOL'ed. +""" +from __future__ import absolute_import +import sys +import os +import unittest +import logging +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')) +from common import with_seed + +# set up logger +logging.basicConfig() +LOGGER = logging.getLogger() +LOGGER.setLevel(logging.INFO) Review comment: Please remove 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] marcoabreu commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
marcoabreu 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_r173758757
##
File path: tests/python-pytest/onnx/onnx_backend_test.py
##
@@ -0,0 +1,116 @@
+# 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.
+
+"""ONNX test backend wrapper"""
+from __future__ import absolute_import
+from __future__ import division
+from __future__ import print_function
+from __future__ import unicode_literals
+
+import unittest
+try:
+import onnx.backend.test
+except ImportError:
+raise ImportError("Onnx and protobuf need to be installed")
+
+import backend as mxnet_backend
+
+# This is a pytest magic variable to load extra plugins
+pytest_plugins = "onnx.backend.test.report",
+
+BACKEND_TEST = onnx.backend.test.BackendTest(mxnet_backend, __name__)
+
+IMPLEMENTED_OPERATORS = [
Review comment:
Are you going to activate all tests?
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] marcoabreu commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
marcoabreu 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_r173758169
##
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:
I would (don't know if it's feasible) compare the similarity between a
simply upscaled picture and the produced picture. If it differs too much, this
could indicate that something went wrong and we didn't actually create an
upscale picture but just scrambled bits in picture format.
But when I'm thinking about it... This is an example that is supposed to be
used by users as a reference, right? I think we should rather write a test on
top of the examples instead of overloading it with verification logic as it
could cause issues differentiating the necessary pieces from verification logic.
What do you think?
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] marcoabreu commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
marcoabreu 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_r173756982
##
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:
I would prefer tests being introduced at the same time as a new feature. How
much is missing?
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] marcoabreu commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
marcoabreu 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_r173756565 ## File path: ci/docker/Dockerfile.build.ubuntu_cpu ## @@ -46,6 +46,8 @@ COPY install/ubuntu_docs.sh /work/ RUN /work/ubuntu_docs.sh COPY install/ubuntu_adduser.sh /work/ RUN /work/ubuntu_adduser.sh +COPY install/ubuntu_onnx.sh /work/ Review comment: Please put this script before 'adduser' 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] marcoabreu commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
marcoabreu 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_r173756140
##
File path: Jenkinsfile
##
@@ -560,7 +560,18 @@ try {
}
stage('Integration Test') {
-parallel 'Python GPU': {
+ parallel 'Onnx CPU': {
Review comment:
Indenting
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] marcoabreu commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
marcoabreu 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_r173601458 ## 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: Yes, it's fine as long as a commit id or version tag is set. Ideally, we'd use a stable tag instead of a commit hash, but considering the recent and active development as well as number of changes at ONNX, it's okay. It would be good to have a stable version for reference because this will allow us that we verified compatibility between MXNet Version 1.X and ONNX 1.X. 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] marcoabreu commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
marcoabreu 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_r173601178 ## 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: Ah I see. If the plan is to integrate them into ONNX, then it's fine. Please just consider nosetests for tests that are specifically for the MXNet side 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] marcoabreu commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
marcoabreu 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_r173601044
##
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:
https://github.com/apache/incubator-mxnet/blob/master/python/mxnet/gluon/data/vision/datasets.py#L152
We're not adopting this behaviour everywhere, but it would be good if we
could :)
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] marcoabreu commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
marcoabreu 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_r173518305 ## 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: This is not using the onnx tests, right? In that case, please move them to the other folder and use nosetests since that framework is the default for now 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] marcoabreu commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
marcoabreu 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_r173517817
##
File path: Jenkinsfile
##
@@ -649,7 +649,20 @@ try {
}
stage('Integration Test') {
-parallel 'Python GPU': {
+ parallel 'Onnx CPU': {
Review comment:
Indentation
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] marcoabreu commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
marcoabreu 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_r173517341
##
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:
Thanks. But please also verify the output
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] marcoabreu commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
marcoabreu 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_r173517132
##
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:
Is the version (or hash) of this file verified somewhere? To prevent it
being updated without us noticing
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] marcoabreu commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
marcoabreu 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_r173515957 ## 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: What is this 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] marcoabreu commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
marcoabreu 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_r173515010 ## File path: python/mxnet/contrib/__init__.py ## @@ -28,5 +28,5 @@ from . import tensorboard from . import text - +from . import onnx Review comment: Ok, thanks 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] marcoabreu commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
marcoabreu 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_r173401052
##
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:
@eric-haibin-lin Does this assert match? I don't think it assesses the task
properly.
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] marcoabreu commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
marcoabreu 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_r173400362
##
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/integrationtests/onnx_backend_test.py"
Review comment:
Change 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
[GitHub] marcoabreu commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
marcoabreu 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_r173329728
##
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:
please create a folder called ``tests/python-pytest/onnx`` instead. Right,
then you'll have to make the appropriate changes to the dockerfiles
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] marcoabreu commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
marcoabreu 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_r173329488 ## File path: python/mxnet/contrib/__init__.py ## @@ -28,5 +28,5 @@ from . import tensorboard from . import text - +from . import onnx Review comment: Ah I understand, thanks for elaborating. Could there be a possible namespace clash? 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] marcoabreu commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
marcoabreu 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_r173225245
##
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:
If they're running nosetests, then that's the wrong command to execute them.
For non-nose-tests please create a parallel directory to tests/python
because all tests in tests/python are nosetests.
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] marcoabreu commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
marcoabreu 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_r173224818 ## File path: python/mxnet/contrib/__init__.py ## @@ -28,5 +28,5 @@ from . import tensorboard from . import text - +from . import onnx Review comment: What happens if the onnx-package is not present? 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] marcoabreu commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
marcoabreu 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_r173112215
##
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:
Are these two not running 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] marcoabreu commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
marcoabreu 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_r173111759 ## File path: tests/python/integrationtest/onnx_backend_test.py ## @@ -0,0 +1,102 @@ +# 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. + +"""ONNX test backend wrapper""" +# pylint: disable=invalid-name,import-error,wrong-import-position Review comment: Please provide reason to disable Pylint 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] marcoabreu commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
marcoabreu 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_r173111256 ## 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: Please don't use a default seed. (applies to all with_seeds) 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] marcoabreu commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
marcoabreu 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_r173110843
##
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"
+sh "${docker_run} cpu --dockerbinary docker PYTHONPATH=./python/
python example/onnx/test_super_resolution.py"
Review comment:
That example does not have any asserts and thus has no effect
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] marcoabreu commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
marcoabreu 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_r173110983 ## File path: python/mxnet/contrib/__init__.py ## @@ -28,5 +28,5 @@ from . import tensorboard from . import text - +from . import onnx Review comment: What happens if the onnx-package is not present? 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] marcoabreu commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
marcoabreu 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_r173111567 ## 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""" Review comment: Is this test specifically for ONNX? In that case please name it test_onnx.py, otherwise please explain the requirements to run this 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] marcoabreu commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
marcoabreu 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_r173111441 ## 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 Review comment: Please provide reason for disabling pylint here 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] marcoabreu commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
marcoabreu 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_r173110177
##
File path: Jenkinsfile
##
@@ -649,7 +649,20 @@ try {
}
stage('Integration Test') {
-parallel 'Python GPU': {
+ parallel 'Python CPU': {
Review comment:
Please use the right name. Also, indenting is wrong.
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] marcoabreu commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
marcoabreu 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_r173110264
##
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:
wrong workspace name (should be onnx)
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] marcoabreu commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
marcoabreu 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_r173110442
##
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:
Could you use logging instead of print?
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] marcoabreu commented on a change in pull request #9963: [MXNET-34] Onnx Module to import onnx models into mxnet
marcoabreu 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_r173110919 ## 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: Please add asserts if this should be a 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
