[ONNX] Support exporting aten::copy_ and aten::index_put to ONNX opset 11 #26941

BowenBao · 2019-09-27T00:08:42Z

Add more comments and refactor the logic of ReshapeToAdvancedIndexingFormat
Add more description here. Cases that are/aren't supported, and how they are supported.
Need to merge this PR [ONNX] Use deepcopy inputs for ONNX ort test cases #27186 to enable testing inplace operators.

We are now supporting exporting aten::copy_ and aten::index_put to ONNX.
Here's a breakdown of the different cases in PyTorch code.

# Case 1: Scalar Indices
x[0, 1, 2] = data

# Case 2: Slice Indices
x[1:3, :, ::2] = data

# Case 3: Ellipsis Indices
x[..., 0] = data

# Case 4: Tensor Indices
ind1 = torch.tensor([0, 2])
ind2 = torch.tensor([1, 1])
x[ind1, ind2] = data

# Case 5: Mixing all the above cases
ind1 = torch.tensor([0, 2])
ind2 = torch.tensor([1, 1])
x[1:3, ind1, ind2, ..., 3] = data

Limitations:

Tensor indices must be consecutive, and 1-d tensors.

# Supported
ind1 = torch.tensor([0, 2])
ind2 = torch.tensor([1, 1])
x[ind1, ind2] = data

# Not supported
ind1 = torch.tensor([0, 2])
ind2 = torch.tensor([1, 1])
ind3 = torch.tensor([[0], [1]])
x[ind1, :, ind2] = data
x[ind3] = data

Negative indices are not supported.

# Not supported
x[-1] = data

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.

lara-hdr

Thanks for the comments, they make the code easy to follow.
The code is clean and looks good, I left some minor comments.

torch/csrc/jit/passes/onnx/prepare_inplace_ops_for_onnx.cpp

torch/onnx/symbolic_opset11.py

test/onnx/test_pytorch_onnx_onnxruntime.py

BowenBao · 2019-10-31T21:46:14Z

Looking at failed tests after rebasing with master.

BowenBao · 2019-10-31T23:16:25Z

The failed tests are related to this issue #29008. The traced graph produced by tracer is empty, resulting in incorrect ONNX graph.

facebook-github-bot

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

BowenBao · 2019-11-06T18:52:48Z

@pytorchbot retest this please

houseroad · 2019-11-06T18:56:08Z

@pytorchbot stopped working. a manual rebase?

facebook-github-bot

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

houseroad

Shall we just ~~abandon~~rebase this PR?

facebook-github-bot

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

neginraoof · 2019-12-06T19:48:31Z

Looks good

lara-hdr

LGTM thanks!

houseroad

Looks good.

facebook-github-bot · 2019-12-10T11:23:28Z

@houseroad merged this pull request in 63f1b78.

…ytorch#26941) Summary: - [x] Add more comments and refactor the logic of `ReshapeToAdvancedIndexingFormat` - [x] Add more description here. Cases that are/aren't supported, and how they are supported. - [x] Need to merge this PR pytorch#27186 to enable testing inplace operators. We are now supporting exporting aten::copy_ and aten::index_put to ONNX. Here's a breakdown of the different cases in PyTorch code. ``` # Case 1: Scalar Indices x[0, 1, 2] = data # Case 2: Slice Indices x[1:3, :, ::2] = data # Case 3: Ellipsis Indices x[..., 0] = data # Case 4: Tensor Indices ind1 = torch.tensor([0, 2]) ind2 = torch.tensor([1, 1]) x[ind1, ind2] = data # Case 5: Mixing all the above cases ind1 = torch.tensor([0, 2]) ind2 = torch.tensor([1, 1]) x[1:3, ind1, ind2, ..., 3] = data ``` Limitations: Tensor indices must be consecutive, and 1-d tensors. ``` # Supported ind1 = torch.tensor([0, 2]) ind2 = torch.tensor([1, 1]) x[ind1, ind2] = data # Not supported ind1 = torch.tensor([0, 2]) ind2 = torch.tensor([1, 1]) ind3 = torch.tensor([[0], [1]]) x[ind1, :, ind2] = data x[ind3] = data ``` Negative indices are not supported. ``` # Not supported x[-1] = data ``` Pull Request resolved: pytorch#26941 Differential Revision: D17951030 Pulled By: houseroad fbshipit-source-id: 4357777072f53aa0bc4b297aa1ee53457a7f8dec

BowenBao requested a review from apaszke as a code owner September 27, 2019 00:08

pytorchbot added oncall: jit Add this issue/PR to JIT oncall triage queue module: autograd Related to torch.autograd, and the autograd engine in general module: build Build system issues module: onnx Related to torch.onnx labels Sep 27, 2019

ezyang added the open source label Sep 27, 2019

BowenBao changed the title ~~[WIP] Support exporting aten::copy_ and aten::index_put to ONNX opset 11~~ Support exporting aten::copy_ and aten::index_put to ONNX opset 11 Oct 4, 2019

jerryzh168 requested a review from houseroad October 5, 2019 01:30

jerryzh168 added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Oct 5, 2019

BowenBao force-pushed the onnx_index_put branch from a782a1b to 990a0e1 Compare October 7, 2019 21:01

BowenBao changed the title ~~Support exporting aten::copy_ and aten::index_put to ONNX opset 11~~ [ONNX] Support exporting aten::copy_ and aten::index_put to ONNX opset 11 Oct 7, 2019

pytorchbot added the module: ci Related to continuous integration label Oct 9, 2019

BowenBao force-pushed the onnx_index_put branch from cf89f4e to d0207c7 Compare October 14, 2019 17:19

facebook-github-bot reviewed Oct 16, 2019

View reviewed changes

houseroad requested review from lara-hdr and spandantiwari October 22, 2019 17:18

facebook-github-bot reviewed Oct 22, 2019

View reviewed changes

lara-hdr suggested changes Oct 23, 2019

View reviewed changes

torch/csrc/jit/passes/onnx/prepare_inplace_ops_for_onnx.cpp Outdated Show resolved Hide resolved

torch/onnx/symbolic_opset11.py Outdated Show resolved Hide resolved

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

BowenBao force-pushed the onnx_index_put branch from 6213678 to d18c6d2 Compare October 31, 2019 18:55

facebook-github-bot reviewed Nov 4, 2019

View reviewed changes

BowenBao force-pushed the onnx_index_put branch from e15f103 to 84c0523 Compare November 6, 2019 17:53

houseroad requested a review from lara-hdr November 6, 2019 18:58

BowenBao force-pushed the onnx_index_put branch from 84c0523 to a6d8571 Compare November 6, 2019 23:46

facebook-github-bot reviewed Nov 7, 2019

View reviewed changes

houseroad reviewed Nov 26, 2019

View reviewed changes

BowenBao force-pushed the onnx_index_put branch from 4fd69f6 to eff8cb4 Compare November 26, 2019 18:04

houseroad requested a review from lara-hdr November 27, 2019 16:10

BowenBao added 13 commits December 2, 2019 11:18

Support exporting aten::copy_ and aten::index_put to ONNX opset 11

fd51913

remove debug prints

6440721

refactor indices conversion code

88f01a1

fix lint

d6abe1c

add warning back

a9caa9c

update ort ci version, enable tests

30257c9

fix lint

f8a1303

add more comments, refactor code around unordered_map emplace

ada35ed

Explicitly set arange dtype to Long, use int max instead of magic number

46599cf

address comments

c136636

add more comments

f67e123

move inplace onnx pass

a2f95cd

fix lint

ff5fdad

BowenBao force-pushed the onnx_index_put branch from 5baa626 to ff5fdad Compare December 2, 2019 19:18

rename variable to avoid test error

10f7baf

facebook-github-bot reviewed Dec 2, 2019

View reviewed changes

lara-hdr approved these changes Dec 6, 2019

View reviewed changes

houseroad approved these changes Dec 7, 2019

View reviewed changes

facebook-github-bot closed this in 63f1b78 Dec 7, 2019

houseroad mentioned this pull request Dec 8, 2019

[v1.4.0 cherrypick] Support exporting aten::copy_ and aten::index_put to ONNX opset 11 #30939

Closed

facebook-github-bot added the merged label Dec 10, 2019

mruberry added the Merged label Oct 28, 2020

masahi mentioned this pull request Jan 8, 2021

Adding aten::unsqueeze_ to PT Frontend apache/tvm#7231

Merged

[ONNX] Support exporting aten::copy_ and aten::index_put to ONNX opset 11 #26941

[ONNX] Support exporting aten::copy_ and aten::index_put to ONNX opset 11 #26941

Uh oh!

Conversation

BowenBao commented Sep 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

lara-hdr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

BowenBao commented Oct 31, 2019

Uh oh!

BowenBao commented Oct 31, 2019

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

BowenBao commented Nov 6, 2019

Uh oh!

houseroad commented Nov 6, 2019

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

houseroad left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

neginraoof commented Dec 6, 2019

Uh oh!

lara-hdr 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!

facebook-github-bot commented Dec 10, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

BowenBao commented Sep 27, 2019 •

edited

Loading

houseroad left a comment •

edited

Loading