Fix tracing slice/select with dynamic inputs #26549

neginraoof · 2019-09-20T17:26:41Z

Fix Slice/Select trace arguments. This PR stashes arguments to functions in order to avoid tracing them as constants.
This PR depends on a fix for select op in PR:
#25273

…oof/traceSlice

…d by ORT anymore.

colesbury · 2019-09-20T18:36:29Z

torch/csrc/autograd/python_variable_indexing.cpp

-static Variable applySelect(const Variable& self, int64_t dim, int64_t index, int64_t real_dim=0) {
-  if (index == 0 && dim == 0 && self.dim() == 0) {
+static Variable applySelect(const Variable& self, int64_t dim, PyObject* index, int64_t real_dim=0) {
+  auto& var = THPVariable_Unpack(index);


I don't think index is necessarily a Variable (Tensor) here. It might be a Python number for example.

Yes, updated this

…oof/traceSlice

…torch into neraoof/traceSlice

neginraoof · 2019-09-25T07:53:20Z

cc @houseroad for review

…oof/traceSlice

facebook-github-bot

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

neginraoof · 2019-09-27T17:52:58Z

@pytorchbot rebase this please

test/onnx/test_pytorch_onnx_onnxruntime.py

houseroad · 2019-10-16T16:57:43Z

@pytorchbot seems not working, @neginraoof could you manually rebase this PR?

…torch into neraoof/traceSlice

…oof/traceSlice

neginraoof · 2019-10-16T22:12:12Z

@houseroad this is rebased.

houseroad · 2019-10-16T22:22:46Z

I am wondering why still so many test cases failing... Did you rebase to the current master? Our monitor shows all CI passing.

…oof/traceSlice

neginraoof · 2019-10-17T01:51:30Z

@houseroad This is ready. There was a related failure in test_jit that I missed. I updated the jit test to skip opset 9 tests for DynamicSlice. (This is the op that's no longer supported with any backends)

neginraoof · 2019-10-18T19:25:32Z

@pytorchbot retest this please

facebook-github-bot

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

…oof/traceSlice

neginraoof · 2019-10-21T17:30:43Z

@pytorchbot retest this please

neginraoof · 2019-10-21T17:53:36Z

Failure for data_workers_test.py does not look relevant. Will look into that.

facebook-github-bot

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

…torch into neraoof/traceSlice # Conflicts: # torch/onnx/symbolic_opset9.py

…oof/traceSlice

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 · 2019-10-23T01:09:32Z

@houseroad merged this pull request in d2eb08d.

Summary: Fix Slice/Select trace arguments. This PR stashes arguments to functions in order to avoid tracing them as constants. This PR depends on a fix for select op in PR: pytorch#25273 Pull Request resolved: pytorch#26549 Reviewed By: hl475 Differential Revision: D17623851 Pulled By: houseroad fbshipit-source-id: ae314004266688d2c25c5bada2dcedbfc4f39c5b

neginraoof added 2 commits September 20, 2019 09:49

fix torch slice and select trace for dynamic inputs

d38fd61

added index and jit tests

1e7332e

neginraoof requested a review from apaszke as a code owner September 20, 2019 17:26

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: onnx Related to torch.onnx module: pybind Related to our Python bindings / interactions with other Python libraries labels Sep 20, 2019

ezyang added the open source label Sep 20, 2019

neginraoof added 2 commits September 20, 2019 11:11

Merge branch 'master' of https://github.com/pytorch/pytorch into nera…

f3d8417

…oof/traceSlice

avoid export of ONNX DynamicSlice in opset 9. This op is not supporte…

36e09b3

…d by ORT anymore.

colesbury reviewed Sep 20, 2019

View reviewed changes

neginraoof changed the title ~~Neraoof/trace slice~~ Fix tracing slice/select with dynamic inputs Sep 22, 2019

neginraoof and others added 9 commits September 22, 2019 15:28

Merge branch 'master' of https://github.com/pytorch/pytorch into nera…

5ea4fff

…oof/traceSlice

fix for flake8 and comment

13fa24c

Merge branch 'master' of https://github.com/pytorch/pytorch into nera…

94da9e2

…oof/traceSlice

Removed dynamic slice for opset 9

5c5df80

test dynamic slice opset for 10

2557307

removed dynamic slice tests for opset9

5b99da0

Merge branch 'neraoof/traceSlice' of https://github.com/neginraoof/py…

ff37f0a

…torch into neraoof/traceSlice

fixed expect graph

c7c6bcf

skip dynamic slice test for opset 9

71e946c

Merge branch 'master' of https://github.com/pytorch/pytorch into nera…

b6097f1

…oof/traceSlice

yf225 added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Sep 25, 2019

facebook-github-bot reviewed Sep 27, 2019

View reviewed changes

Merge remote-tracking branch 'origin/master' into HEAD

e82cac9

BowenBao reviewed Oct 1, 2019

View reviewed changes

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

neginraoof added 2 commits October 1, 2019 11:09

clean up

4b73952

update pytorch version in expect file

7e4da17

neginraoof added 2 commits October 16, 2019 10:06

Merge branch 'neraoof/traceSlice' of https://github.com/neginraoof/py…

df6f786

…torch into neraoof/traceSlice

Merge branch 'master' of https://github.com/pytorch/pytorch into nera…

8e72f64

…oof/traceSlice

neginraoof added 2 commits October 16, 2019 16:20

Merge branch 'master' of https://github.com/pytorch/pytorch into nera…

351d462

…oof/traceSlice

skip dynamic slice for opset9, no longer supported

265c31f

facebook-github-bot reviewed Oct 19, 2019

View reviewed changes

neginraoof added 4 commits October 20, 2019 19:34

Merge branch 'master' of https://github.com/pytorch/pytorch into nera…

c311305

…oof/traceSlice

clean up

1aebcdf

fix for comments

f3609d4

clean removed expect file

34f4f96

facebook-github-bot reviewed Oct 22, 2019

View reviewed changes

neginraoof added 4 commits October 22, 2019 09:06

dyn slice for aten_fallback

54be515

Merge branch 'neraoof/traceSlice' of https://github.com/neginraoof/py…

f8122a6

…torch into neraoof/traceSlice # Conflicts: # torch/onnx/symbolic_opset9.py

clean up

e06adaf

Merge branch 'master' of https://github.com/pytorch/pytorch into nera…

946742f

…oof/traceSlice

facebook-github-bot reviewed Oct 22, 2019

View reviewed changes

facebook-github-bot closed this in d2eb08d Oct 23, 2019

facebook-github-bot added the merged label Oct 23, 2019

neginraoof mentioned this pull request Oct 25, 2019

[ONNX] incorrect export of opset10 slice with dynamic_axis #28611

Closed

mruberry added the Merged label Oct 28, 2020

Fix tracing slice/select with dynamic inputs #26549

Fix tracing slice/select with dynamic inputs #26549

Uh oh!

Conversation

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

Uh oh!

Uh oh!

colesbury Sep 20, 2019

Choose a reason for hiding this comment

Uh oh!

neginraoof Sep 25, 2019

Choose a reason for hiding this comment

Uh oh!

neginraoof commented Sep 25, 2019

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

neginraoof commented Sep 27, 2019

Uh oh!

Uh oh!

houseroad commented Oct 16, 2019

Uh oh!

neginraoof commented Oct 16, 2019

Uh oh!

houseroad commented Oct 16, 2019

Uh oh!

neginraoof commented Oct 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

neginraoof commented Oct 18, 2019

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

neginraoof commented Oct 21, 2019

Uh oh!

neginraoof commented Oct 21, 2019

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!

facebook-github-bot commented Oct 23, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

neginraoof commented Sep 20, 2019 •

edited

Loading

neginraoof commented Oct 17, 2019 •

edited

Loading