[WIP] Serde Module for Import/Export of models between Onnx and Mxnet #9892

anirudhacharya · 2018-02-27T01:05:34Z

Description

ImportExport module based on this design doc - https://cwiki.apache.org/confluence/display/MXNET/Proposal%3A+ImportExport+module.
This PR is a work in progress, we are just sending out this skeleton code to get views on the api specification, general code structure, and any additional dependencies it may pull into the project. As this module is also being discussed currently in the dev list, it is open to change in the design and specification of the project.

Changes

Import from Onnx.
Export to Onnx.

Comments

I have currently added this project under contrib, as it is used for experimental features which can later be moved elsewhere.

@spidydev @Roshrini @lupesko @szha

piiswrong · 2018-02-27T01:26:04Z

python/mxnet/contrib/serde/_export/__init__.py

+import onnx
+
+def export_model(sym, params):
+    pass


Will delete this module for now.

Is this the determined API for exporting MXNet models to onnx format? Have you considered whether we can borrow the interface of this function in Pytorch?
https://github.com/pytorch/pytorch/blob/master/torch/onnx/__init__.py#L43

piiswrong · 2018-02-27T01:26:12Z

python/mxnet/contrib/__init__.py

@@ -28,3 +28,4 @@
 from . import tensorboard

 from . import text
+from . import serde


https://github.com/dmlc/

Will change the module name from serde to onnx.

+1
Serde can come in later as a facade on top of onnx and coreml

piiswrong · 2018-02-27T01:27:33Z

python/mxnet/contrib/serde/_import/backend.py

+# and then return the output.
+
+class MXNetBackend(Backend):
+    """MXNet backend for ONNX"""


what's this?

This class is used by the onnx backend test framework here - https://github.com/anirudhacharya/incubator-mxnet/blob/serde/python/mxnet/contrib/serde/_import/tests/onnx_backend_test.py

For testing the import functionality, we intend to use ONNX's backend test framework as described here - https://github.com/onnx/onnx/blob/master/docs/OnnxBackendTest.md
This is done to ensure that tests will be shared across different frameworks and that our module will be sync with onnx's standards and definition for various operators. This class will be used by the test framework to run operators on mxnet backend.

piiswrong · 2018-02-27T01:35:28Z

python/mxnet/contrib/serde/_import/common.py

+from __future__ import absolute_import as _abs
+from ....base import string_types
+
+class Renamer(object):


This class looks unnecessary

piiswrong · 2018-02-27T01:41:47Z

python/mxnet/contrib/serde/_import/tests/onnx_backend_test.py

+from onnx_mxnet import backend as mxnet_backend
+
+# This is a pytest magic variable to load extra plugins
+pytest_plugins = 'onnx.backend.test.report'


tests should be put in tests/python/unittests

will change it.

piiswrong · 2018-02-27T01:44:18Z

python/mxnet/contrib/serde/_import/import_onnx.py

+                slice_op = symbol.slice_axis(slice_op, axis=axis, begin=begin[i], end=end[i])
+        return slice_op
+
+    def _fix_squeeze(self, inputs, new_attr):


why not do this in attribute converter?

piiswrong · 2018-02-27T01:44:42Z

python/mxnet/contrib/serde/_import/import_onnx.py

+
+    def run_node(self, node, device='CPU'): # pylint: disable=unused-argument
+        """Construct symbol from individual node.
+        Mainly using this function for unittests"""


test utility should be put into test code

Will refactor this code along with Renamer and Attribute Converter class.

* temporary fix to update the verionsioning of 1.1.0 that is skipped during build process * updated build_doc.sh * testing new updates of build_doc.sh * fixed comments and syntax * removed test data and comments

* changed url references from dmlc to apache/incubator-mxnet * refactored build all docs * Created dockerfile and modified scripts to support it. * add license header * used license header tool this time * updated scripts to take arguments for flexible use

* TVM bridge support. Support wrap TVM compiled function as a NDArray function. * Testcases and CI to include TVM as dependency * address review comments * Add more comments, change to constexpr * change to log warn * update comment on the type code

* Update website version dropdown on master * Update

* opt register only once * lint pass * opt register only once lint pass * add test_optimizer.cpp * fix warning * typo

* add infer_type for regression ops * trigger CI

* Accelerate the calculation of F1 * cache the mid results * trigger CI

…ork (a trial solution to issue#9866) (apache#9867) * Enable the reporting of cross-entropy or nll loss value during training * Set the default value of loss as a '' to avoid a Python runtime issue when loss argument is not set * Applying the Xavier with "uniform" type to initialize weight when network is VGG

…xnet into onnx

lupesko

Looks good!
Still needs some work...

lupesko · 2018-02-28T20:50:02Z

python/mxnet/contrib/__init__.py

@@ -28,3 +28,4 @@
 from . import tensorboard

 from . import text
+from . import serde


+1
Serde can come in later as a facade on top of onnx and coreml

lupesko · 2018-02-28T20:50:39Z

python/mxnet/contrib/serde/_import/__init__.py

+import onnx
+from .import_onnx import GraphProto
+
+def import_model(model_file):


Propose to rename to import_onnx_model

It will be used as follows -

sym, params = mxnet.contrib.onnx._import.import_model("super_resolution.onnx")

So onnx is already present in the package path.
Adding onnx in the method call could be redundant. It will get used like this

sym, params = mxnet.contrib.onnx._import.import_onnx_model("super_resolution.onnx")

I am not sure if we would want that.

Is import_model a user level API? If so, why is it registered under a hidden module _import?

lupesko · 2018-02-28T20:51:07Z

python/mxnet/contrib/serde/_import/backend.py

@@ -0,0 +1,131 @@
+# Copyright 2017 Amazon.com, Inc. or its affiliates. All Rights Reserved.


lupesko · 2018-02-28T20:51:17Z

python/mxnet/contrib/serde/_import/__init__.py

@@ -0,0 +1,26 @@
+# coding: utf-8


Please add copyright headers

lupesko · 2018-02-28T20:51:34Z

python/mxnet/contrib/serde/_import/backend.py

+# coding: utf-8
+# pylint: disable=too-many-locals,invalid-name
+"""backend wrapper for onnx test infrastructure"""
+from collections import namedtuple


nit: newline

lupesko · 2018-02-28T20:51:59Z

python/mxnet/contrib/serde/_import/backend_rep.py

@@ -0,0 +1,70 @@
+# Copyright 2017 Amazon.com, Inc. or its affiliates. All Rights Reserved.


2018, repeats in more files, please update.

I will update all the license and copyright statements.

lupesko · 2018-02-28T20:53:34Z

python/setup.py

@@ -28,7 +28,7 @@
 else:
    from setuptools import setup
    from setuptools.extension import Extension
-    kwargs = {'install_requires': ['numpy<=1.13.3,>=1.8.2', 'requests==2.18.4', 'graphviz==0.8.1'], 'zip_safe': False}
+    kwargs = {'install_requires': ['numpy<=1.13.3,>=1.8.2', 'requests==2.18.4', 'graphviz==0.8.1', 'onnx>=1.0.1'], 'zip_safe': False}


Let's make ONNX a runtime dependency, in the future we can update it to be an install time dependency.

- Change package name to onnx. - remove Renamer class - Add apache license to files. - refactor test files to unittests folder. - remove export folder. Signed-off-by: Acharya <[email protected]>

anirudhacharya · 2018-03-01T22:31:05Z

will raise another PR. closing this PR

#9963

Acharya added 2 commits February 26, 2018 01:00

import module

b662a69

fixing dependencies

1b0c516

anirudhacharya requested a review from szha as a code owner February 27, 2018 01:05

piiswrong suggested changes Feb 27, 2018

View reviewed changes

thinksanky and others added 12 commits February 27, 2018 11:50

Updated build_doc.sh to build on the new release tag found (apache#9896)

ada45fb

* temporary fix to update the verionsioning of 1.1.0 that is skipped during build process * updated build_doc.sh * testing new updates of build_doc.sh * fixed comments and syntax * removed test data and comments

Make sure build_doc only builds target version (apache#9900)

dc39343

Update website version dropdown on master (apache#9901)

6005ad5

* Update website version dropdown on master * Update

fix optimizer bug in CPP-Package (apache#9809)

7a0509d

* opt register only once * lint pass * opt register only once lint pass * add test_optimizer.cpp * fix warning * typo

add infer_type for regression ops, fix issues#9847 (apache#9848)

34af930

* add infer_type for regression ops * trigger CI

[Metric] Accelerate the calculation of F1 (apache#9833)

b8ae967

* Accelerate the calculation of F1 * cache the mid results * trigger CI

updated version to 1.1.0 (apache#9895)

9761f21

workaround for install page display issue (apache#9902)

48749a5

Merge branch 'serde' of https://github.com/anirudhacharya/incubator-m…

9c48caa

…xnet into onnx

lupesko suggested changes Feb 28, 2018

View reviewed changes

Addressing comments and refactoring test code.

6f71287

- Change package name to onnx. - remove Renamer class - Add apache license to files. - refactor test files to unittests folder. - remove export folder. Signed-off-by: Acharya <[email protected]>

anirudhacharya requested review from cjolivier01 and marcoabreu as code owners March 1, 2018 22:28

anirudhacharya closed this Mar 1, 2018

anirudhacharya deleted the serde branch March 1, 2018 23:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Serde Module for Import/Export of models between Onnx and Mxnet #9892

[WIP] Serde Module for Import/Export of models between Onnx and Mxnet #9892

anirudhacharya commented Feb 27, 2018

piiswrong Feb 27, 2018

anirudhacharya Feb 28, 2018

reminisce Mar 1, 2018

piiswrong Feb 27, 2018

anirudhacharya Feb 28, 2018

lupesko Feb 28, 2018

piiswrong Feb 27, 2018

anirudhacharya Feb 28, 2018

piiswrong Feb 27, 2018

piiswrong Feb 27, 2018

anirudhacharya Feb 28, 2018

piiswrong Feb 27, 2018

piiswrong Feb 27, 2018

anirudhacharya Feb 28, 2018

lupesko left a comment

lupesko Feb 28, 2018

lupesko Feb 28, 2018

anirudhacharya Feb 28, 2018

reminisce Mar 1, 2018

lupesko Feb 28, 2018

lupesko Feb 28, 2018

lupesko Feb 28, 2018

lupesko Feb 28, 2018

anirudhacharya Feb 28, 2018

lupesko Feb 28, 2018

anirudhacharya Feb 28, 2018

anirudhacharya commented Mar 1, 2018 •

edited

Loading

		@@ -0,0 +1,131 @@
		# Copyright 2017 Amazon.com, Inc. or its affiliates. All Rights Reserved.

		@@ -0,0 +1,70 @@
		# Copyright 2017 Amazon.com, Inc. or its affiliates. All Rights Reserved.

[WIP] Serde Module for Import/Export of models between Onnx and Mxnet #9892

[WIP] Serde Module for Import/Export of models between Onnx and Mxnet #9892

Conversation

anirudhacharya commented Feb 27, 2018

Description

Changes

Comments

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lupesko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anirudhacharya commented Mar 1, 2018 • edited Loading

anirudhacharya commented Mar 1, 2018 •

edited

Loading