Add ONNX support #142

interesaaat · 2020-06-16T17:43:19Z

This PR tries to fix the coverage problem of #86.

refactoring of convert creation of ir_converter dir containing the converters for the irs add onnxml tree models to supporteed

some refactoring of the other tree converters

Fix ONNXML operator mapping returning None instead of exception Some renaming Fix a couple of bugs on tree_ensembles

Add ONNX to converter API

Add check that only onnx backend can be target for onnx models Some renaming

Working on classification tasks

Fix setup for reordering trees in multiclass tasks Fix bug for onnxml

Some import clean up

Test only HB files

codecov-commenter · 2020-06-16T21:20:14Z

Codecov Report

Merging #142 into master will decrease coverage by 1.21%.
The diff coverage is 90.53%.

@@            Coverage Diff             @@
##           master     #142      +/-   ##
==========================================
- Coverage   94.54%   93.33%   -1.22%     
==========================================
  Files          20       24       +4     
  Lines        1211     1545     +334     
  Branches      202      285      +83     
==========================================
+ Hits         1145     1442     +297     
- Misses         40       63      +23     
- Partials       26       40      +14

Flag	Coverage Δ
#unittests	`93.33% <90.53%> (-1.22%)`	⬇️

Impacted Files	Coverage Δ
hummingbird/ml/convert.py	`83.65% <80.00%> (-7.12%)`	⬇️
hummingbird/ml/_utils.py	`81.42% <84.21%> (+1.03%)`	⬆️
hummingbird/ml/ir_converters/linked_node.py	`90.17% <90.17%> (ø)`
...bird/ml/operator_converters/onnx_tree_ensembles.py	`91.81% <91.81%> (ø)`
hummingbird/ml/ir_converters/topology.py	`92.00% <92.00%> (ø)`
hummingbird/ml/supported.py	`92.18% <96.55%> (+2.44%)`	⬆️
hummingbird/ml/__init__.py	`100.00% <100.00%> (ø)`
hummingbird/ml/ir_converters/__init__.py	`100.00% <100.00%> (ø)`
hummingbird/ml/operator_converters/__init__.py	`100.00% <100.00%> (ø)`
...ummingbird/ml/operator_converters/_gbdt_commons.py	`100.00% <100.00%> (ø)`
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2e56f88...6620ab0. Read the comment docs.

.github/workflows/pythonapp.yml

ksaur

initial comments, more tomorrow ;)

ksaur · 2020-06-17T00:31:21Z

.github/workflows/pythonapp.yml

@@ -49,7 +49,7 @@ jobs:
        export LDFLAGS="$LDFLAGS -Wl,-rpath,/usr/local/opt/libomp/lib -L/usr/local/opt/libomp/lib -lomp"
    - name: Install extra dependencies
      run: |
-        pip install .[extra]
+        pip install  --no-cache-dir .[extra,onnx]


i think i determined that we don't need --no-cache-dir here. was it giving some error?

Nope, I just copied from the version we had before. Let me remove it.

ksaur · 2020-06-17T00:36:35Z

tests/test_onnxml_converter_lightgbm.py

+        not (onnx_ml_tools_installed() and onnx_installed()), reason="ONNXML test require ONNX, ORT and ONNXMLTOOLS"
+    )
+    def test_lightgbm_pytorch(self):
+        X = [[0, 1], [1, 1], [2, 0]]


Does datasize impact anything we need to test?

I think I have a mix of test using random numbers and fixed arrays for that reason. The fixed array tests are taken from the LightGBM convert tests.

ksaur · 2020-06-17T00:42:34Z

hummingbird/ml/convert.py

+    """
+    Function returning whether the input model is an ONNX model or not.
+    """
+    return "onnx" in model.__module__ and "graph" in dir(model)


maybe something like

>>> type(onnx_ml_model).__name__ 'ModelProto'

or

>>> isinstance(onnx_ml_model, onnx.onnx_ONNX_REL_1_6_ml_pb2.ModelProto) True

i guess for the second one, the version isn't known. but we could check just the first part

Yes the version can change. Should I go with the first one?

yah, i think the first one is cleanest, although i don't feel strongly :)

ksaur · 2020-06-17T00:44:37Z

.github/workflows/pythonapp.yml

@@ -61,7 +61,7 @@ jobs:
        pytest
    - name: Coverage
      run: |
-        coverage run -a -m pytest tests
+        coverage run -a -m pytest tests --ignore=tests/test_no_extra_install.py


and this part should no longer be needed due to the skips, although it doesn't hurt. (maybe you just did a revert on this file?)

My doubt was whether coverage will get messed up since we generate the coverage when some dependencies are off and then again when they are on, and we run the same tests.

hummingbird/ml/convert.py

hummingbird/ml/ir_converters/linked_node.py

ksaur

next batch....

hummingbird/ml/convert.py

ksaur · 2020-06-17T16:47:14Z

hummingbird/ml/convert.py

        extra_config: Extra configurations to be used by the individual operator converters.
                      The set of supported extra configurations can be found at `hummingbird.ml.supported`

    Examples:
-        >>> pytorch_model = convert(sklearn_model,`pytorch`)
+        >>> pytorch_model = convert(sklearn_model,`torch`)


We still need some comment somewhere (not necessarily here) about how these are equivalent, and that this is a non-breaking change. I'm being picky because folks have >100 forks of HB, and I don't want to break anything or confuse anyone like last time w/pypi! (our blog, etc has this as 'pytorch' so i just want to make sure we communicate this change and that it is backward compat.)

We can add an item on this in the next release description.

hummingbird/ml/ir_converters/linked_node.py

hummingbird/ml/operator_converters/_gbdt_commons.py

hummingbird/ml/operator_converters/onnx_tree_ensembles.py

ksaur · 2020-06-17T17:15:57Z

hummingbird/ml/operator_converters/onnx_tree_ensembles.py

+    classes = post_transform = None
+
+    for attr in model.origin.attribute:
+        if attr.name == "nodes_falsenodeids":


Maybe put a comment about how these strings are from https://xadupre.github.io/skl2onnx/onnx_ops.html or something

Good point! Let me add a link.

hummingbird/ml/operator_converters/skl_decision_tree.py

hummingbird/ml/supported.py

ksaur

LGTM, waiting to merge #145 first.

interesaaat · 2020-06-18T21:28:04Z

If the tests pass it should be good to go. They just bumped PyTorch to 1.5.1. There is a bug in the ONNX exporter for GEMM: I thought they fixed it by with 1.5.1 it's still failing. I enabled tree_trav by default.

* add onnx_installed in utils refactoring of convert creation of ir_converter dir containing the converters for the irs add onnxml tree models to supported * add documentation * add tree ensemble converters some refactoring of the other tree converters * Tree ensemble regressor now points to gbdt implementation * Add check that only onnx backend can be target for onnx models * Add check in covert for supported input model format \ backend Working on classification tasks * Add example notebook for ONNX * rename pytorch into torch

interesaaat added 30 commits May 11, 2020 23:11

add onnx_installed in utils

31650ca

refactoring of convert creation of ir_converter dir containing the converters for the irs add onnxml tree models to supporteed

add documentation

9e14329

add tree ensemble converters

83a0abb

some refactoring of the other tree converters

Fix few bugs, still not working

4e8a5ad

Fix wrong convert parameters

b131933

Fix ONNXML operator mapping returning None instead of exception Some renaming Fix a couple of bugs on tree_ensembles

Tree ensemble regressor now points to gbdt implementation

8f83736

Merge remote-tracking branch 'origin/master' into mainterl/onnxml

69f8cac

Add ONNX to converter API

Add back all regression tests

a01e3c8

Add check that only onnx backend can be target for onnx models Some renaming

Add check in covert for supported input model format \ backend

f751204

Working on classification tasks

onnx converter was editing the model and make it unreadable afterwards

694f786

testing multiclass

69a0fb0

Add reorder_tree constant

ad8dd4f

Fix setup for reordering trees in multiclass tasks Fix bug for onnxml

Add example notebook for ONNX

060e156

Some import clean up

Add documentation

b59c1f2

add onnx to action

d16dff1

fix typo in pipeline

6dd2587

add onnxmltools to pipelie

cd46f2a

Fix call to build linked node graph

c5bfeca

setting up onnx requirements

8968cf3

Merge remote-tracking branch 'origin/master' into mainterl/onnxml

e444da8

fix requirements

10c079d

fix to common requirements

b8daf63

Trying to fix the flake8 error

a994524

Merge remote-tracking branch 'origin/master' into mainterl/onnxml

2f32ed8

rename pytorch into torch

b5e4d69

fix onnx requirements

f040d22

Trying with onnx requirements without 3.8

7d6cef0

Trying with different ordering

bd3e7a8

remove unecessary constraint

02e3c99

Fix requirements for onnx

8086942

Test only HB files

interesaaat added 5 commits June 15, 2020 16:19

fix to sort

83ee1fd

Merge remote-tracking branch 'origin/master' into mainterl/onnxml2

0e0b3b5

rename pytorch into torch

4839762

fix typo

26bec7b

try with old coverage code

a5f2ebd

interesaaat added 4 commits June 16, 2020 14:37

trying with different coverage flags

6e72102

add onnx

35014bc

add test when onnx is not installed

6a19588

comment out unused code

49e2aa2

ksaur reviewed Jun 17, 2020

View reviewed changes

.github/workflows/pythonapp.yml Show resolved Hide resolved

ksaur reviewed Jun 17, 2020

View reviewed changes

interesaaat mentioned this pull request Jun 17, 2020

Add ONNX as input and output model format #86

Closed

ksaur reviewed Jun 17, 2020

View reviewed changes

interesaaat added 2 commits June 17, 2020 10:43

Addressing first batch of comments

f7063ba

Address second batch of comments

aff5029

ksaur self-requested a review June 17, 2020 22:51

ksaur approved these changes Jun 17, 2020

View reviewed changes

interesaaat added 4 commits June 18, 2020 14:09

Merge remote-tracking branch 'origin/master' into mainterl/onnxml2

4e637ef

renaming of pytorch into torch

2738940

debug ubuntu fail

98af852

enabling tree_trav by default

e3a4471

change pytest-cov with pytest

6620ab0

ksaur merged commit 4cb113a into master Jun 18, 2020

ksaur deleted the mainterl/onnxml2 branch June 18, 2020 22:12

ksaur mentioned this pull request Jun 18, 2020

Add support for ONNX-ML models as input format #69

Closed

interesaaat mentioned this pull request Jun 19, 2020

LightGBM converter integration with Hummingbird onnx/onnxmltools#403

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ONNX support #142

Add ONNX support #142

interesaaat commented Jun 16, 2020

codecov-commenter commented Jun 16, 2020 •

edited

ksaur left a comment

ksaur Jun 17, 2020

interesaaat Jun 17, 2020

ksaur Jun 17, 2020

interesaaat Jun 17, 2020

ksaur Jun 17, 2020

ksaur Jun 17, 2020

interesaaat Jun 17, 2020

ksaur Jun 17, 2020

ksaur Jun 17, 2020

interesaaat Jun 17, 2020

interesaaat Jun 17, 2020

ksaur left a comment

ksaur Jun 17, 2020

interesaaat Jun 17, 2020

ksaur Jun 17, 2020

interesaaat Jun 17, 2020

ksaur left a comment

interesaaat commented Jun 18, 2020

Add ONNX support #142

Add ONNX support #142

Conversation

interesaaat commented Jun 16, 2020

codecov-commenter commented Jun 16, 2020 • edited

Codecov Report

ksaur 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

ksaur 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

ksaur left a comment

Choose a reason for hiding this comment

interesaaat commented Jun 18, 2020

codecov-commenter commented Jun 16, 2020 •

edited