Support Exports to Multiple ONNX Opset #19294

lara-hdr · 2019-04-15T22:57:30Z

Support exporting multiple ONNX opsets (more specifically opset 10 for now), following the proposal in https://gist.github.com/spandantiwari/99700e60919c43bd167838038d20f353.
And add support for custom ops (merge with #18297).

This PR will be followed by another PR containing the changes related to testing the ops for different opsets.

lara-hdr · 2019-04-17T18:21:35Z

@houseroad could you take a look at this? thanks :)

test/onnx/test_operators.py

houseroad

This is a great effort for supporting multiple onnx opset. Could you fix the failing tests first? I will do a full review later.

houseroad · 2019-04-19T05:08:02Z

tests are still failing

lara-hdr · 2019-04-22T17:00:28Z

@houseroad, looking into the failure of test_dist_broadcast_coalesced_gloo, the other tests are passing

facebook-github-bot

@houseroad has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

houseroad · 2019-04-23T06:17:52Z

rebase to resolve the conflict?

facebook-github-bot

@houseroad has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

…a-hdr/pytorch into lahaidar/multiple_onnx_opset

houseroad

by default, the exporter should just export the model to the default opset.
If user would like to export to a different opset, it can just specify the target opset using the parameter (we already have it).

We should not ask the users to call sym_registry.register_version('', 9)

test/onnx/expect/TestOperators.test_topk.expect

test/onnx/expect/TestOperators.test_topk_opset10.expect

test/onnx/test_operators.py

torch/onnx/symbolic_helper.py

torch/onnx/symbolic_registry.py

torch/onnx/utils.py

lara-hdr · 2019-04-30T18:52:00Z

The user does not call sym_registry.register_version('', 9).
sym_registry.register_version is already called in the export functions in utils.py with the specified opset.
So calling torch.onnx.export() is sufficient, and will register the specified function (or default version in case none is specified).

test/onnx/test_verify.py

test/onnx/test_utility_funs.py

…aidar/multiple_onnx_opset

lara-hdr · 2019-05-08T23:11:47Z

torch/onnx/symbolic_opset10.py

+
+# Blacklist operators for this opset version.
+# These operators have been updated in ONNX but not re-implemented here.
+# It is very important to blacklist these operators to avoid exporting


@houseroad, this is the code for the blacklisting.
_black_list_in_opset() is defined in symbolic_helper.py.
These lines will redefine the ops in black_listed_operators to raise a warning.

I am adding a test test_blacklisted_ops() in test_verify to test this, in #20036.

https://github.com/onnx/onnx/blob/master/onnx/defs/operator_sets.h#L540

You can find the full list of changed ops in opset 10 here

adding maxpool, averagepool and dropout.
The other ops are not in opset 9.

houseroad · 2019-05-09T19:01:29Z

Lint the code?

torch/onnx/symbolic_registry.py

facebook-github-bot

@houseroad has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

add a comment about _registry

facebook-github-bot

@houseroad has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

houseroad · 2019-05-09T20:57:08Z

The failed onnx test is legit: https://circleci.com/api/v1.1/project/github/pytorch/pytorch/1650207/output/104/0?file=true

You may want to add the max_pool in this pr as well.

torch/onnx/symbolic_opset10.py

facebook-github-bot

@houseroad has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot

@houseroad has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

houseroad

Looks good to me. Thanks a lot of this great effort!

lara-hdr · 2019-05-10T20:09:26Z

Thank you @houseroad!

Summary: Support exporting multiple ONNX opsets (more specifically opset 10 for now), following the proposal in https://gist.github.com/spandantiwari/99700e60919c43bd167838038d20f353. And add support for custom ops (merge with pytorch/pytorch#18297). This PR will be followed by another PR containing the changes related to testing the ops for different opsets. Pull Request resolved: pytorch/pytorch#19294 Reviewed By: zrphercule Differential Revision: D15043951 Pulled By: houseroad fbshipit-source-id: d336fc35b8827145639137bc348ae07e3c14bb1c

facebook-github-bot · 2019-05-11T04:02:43Z

@houseroad merged this pull request in f4d9bfa.

Summary: Refactor tests for #19294. Pull Request resolved: #20036 Reviewed By: zrphercule Differential Revision: D16016593 Pulled By: houseroad fbshipit-source-id: eaae324e347679acf3d0ac1c14be03919f54496e

Lara Haidar-Ahmad added 2 commits April 15, 2019 15:27

Support Multiple ONNX Opset

cb92812

missing import

809c795

facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Apr 15, 2019

add register in trace; fix lint

4c9fc50

houseroad reviewed Apr 18, 2019

View reviewed changes

test/onnx/test_operators.py Outdated Show resolved Hide resolved

houseroad reviewed Apr 18, 2019

View reviewed changes

Lara Haidar-Ahmad added 3 commits April 18, 2019 11:05

fix test failures

6b25531

resolve conflicts

d32b139

missing change for last merge

b2ae0c4

ezyang added the module: onnx Related to torch.onnx label Apr 19, 2019

lara-hdr and others added 3 commits April 19, 2019 13:46

fix merge test_utility_funs.py

891f63a

fix test_operators.py failures

6984d40

update test_verify

2170fa2

facebook-github-bot reviewed Apr 23, 2019

View reviewed changes

resolve conflict

909867e

facebook-github-bot reviewed Apr 23, 2019

View reviewed changes

resolve conflicts

710ba98

BowenBao mentioned this pull request Apr 26, 2019

[ONNX] Proposing custom ops support in ONNX exporter #18297

Closed

Lara added 3 commits April 29, 2019 16:03

update new schema tests to not rely on expect files

47c4f55

Merge branch 'lahaidar/multiple_onnx_opset' of https://github.com/lar…

6a3ff63

…a-hdr/pytorch into lahaidar/multiple_onnx_opset

resolve conflicts

98707c0

houseroad requested changes Apr 30, 2019

View reviewed changes

houseroad reviewed Apr 30, 2019

View reviewed changes

test/onnx/test_verify.py Outdated Show resolved Hide resolved

houseroad reviewed Apr 30, 2019

View reviewed changes

test/onnx/test_utility_funs.py Outdated Show resolved Hide resolved

new tests in new file and minor fixes

0ecb9dc

lara-hdr and others added 4 commits May 7, 2019 16:39

rename test fct to fix fixture error

75fbf83

black list updated ops and add fallback test

a876134

Merge branch 'master' of https://github.com/lara-hdr/pytorch into lah…

9a58dba

…aidar/multiple_onnx_opset

Merge branch 'master' of https://github.com/lara-hdr/pytorch into lah…

7b7d6ad

…aidar/multiple_onnx_opset

lara-hdr commented May 8, 2019

View reviewed changes

Update symbolic_helper.py

499fe71

houseroad reviewed May 9, 2019

View reviewed changes

torch/onnx/symbolic_registry.py Show resolved Hide resolved

facebook-github-bot reviewed May 9, 2019

View reviewed changes

lara-hdr and others added 2 commits May 9, 2019 12:27

Update symbolic_registry.py

9668ac1

add a comment about _registry

add maxpool avgpool and dropout to blacklist for opset 10

b7afe05

facebook-github-bot reviewed May 9, 2019

View reviewed changes

add maxpool op to opset 10

00c1592

lara-hdr commented May 10, 2019

View reviewed changes

torch/onnx/symbolic_opset10.py Show resolved Hide resolved

facebook-github-bot reviewed May 10, 2019

View reviewed changes

Update symbolic_opset10.py

2f01871

pytorchbot added module: docs Related to our documentation, both in docs/ and docblocks module: internals Related to internal abstractions in c10 and ATen labels May 10, 2019

facebook-github-bot reviewed May 10, 2019

View reviewed changes

houseroad approved these changes May 10, 2019

View reviewed changes

facebook-github-bot closed this in f4d9bfa May 11, 2019

facebook-github-bot added the merged label May 11, 2019

ezyang added the open source label Jun 24, 2019

ezyang mentioned this pull request Jul 10, 2019

ValueError: only one element tensors can be converted to Python scalars #22674

Closed

Support Exports to Multiple ONNX Opset #19294

Support Exports to Multiple ONNX Opset #19294

Uh oh!

Conversation

lara-hdr commented Apr 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lara-hdr commented Apr 17, 2019

Uh oh!

Uh oh!

houseroad left a comment

Choose a reason for hiding this comment

Uh oh!

houseroad commented Apr 19, 2019

Uh oh!

lara-hdr commented Apr 22, 2019

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

houseroad commented Apr 23, 2019

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

houseroad left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lara-hdr commented Apr 30, 2019

Uh oh!

Uh oh!

Uh oh!

lara-hdr May 8, 2019

Choose a reason for hiding this comment

Uh oh!

houseroad May 9, 2019

Choose a reason for hiding this comment

Uh oh!

lara-hdr May 9, 2019

Choose a reason for hiding this comment

Uh oh!

houseroad commented May 9, 2019

Uh oh!

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

houseroad commented May 9, 2019

Uh oh!

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

houseroad left a comment

Choose a reason for hiding this comment

Uh oh!

lara-hdr commented May 10, 2019

Uh oh!

facebook-github-bot commented May 11, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

lara-hdr commented Apr 15, 2019 •

edited

Loading