[ONNX] RNN scripting (#57564) #58691

BowenBao · 2021-05-20T20:20:49Z

Stack from ghstack:

[ONNX] Enable support for roll() op. (#58389) #58697 [ONNX] Enable support for roll() op. ([ONNX] Enable support for roll() op. #58389)
[ONNX] handle aten::_set_item on Dict in convertInplaceOpsAndTrackAlias (#58317) #58696 [ONNX] handle aten::_set_item on Dict in convertInplaceOpsAndTrackAlias ([ONNX] handle aten::_set_item on Dict in convertInplaceOpsAndTrackAlias #58317)
[ONNX] use consistent quoting for string literals (#57757) #58695 [ONNX] use consistent quoting for string literals ([ONNX] use consistent quoting for string literals #57757)
[ONNX] Improve lower tuples and handle control flow (#57650) #58694 [ONNX] Improve lower tuples and handle control flow ([ONNX] Improve lower tuples and handle control flow #57650)
[ONNX] Update special post process for SequenceInsert after SequenceEmpty (#56965) #58693 [ONNX] Update special post process for SequenceInsert after SequenceEmpty ([ONNX] Update special post process for SequenceInsert after SequenceEmpty #56965)
Support symbolic for conv_tbc (#58359) #58692 Support symbolic for conv_tbc (Support symbolic for conv_tbc #58359)
[ONNX] RNN scripting (#57564) #58691 [ONNX] RNN scripting ([ONNX] RNN scripting #57564)
[ONNX] Update instance_norm2 symbolic to handle track_running_stats=True (#55051) #58690 [ONNX] Update instance_norm2 symbolic to handle track_running_stats=True ([ONNX] Update instance_norm2 symbolic to handle track_running_stats=True #55051)

Note the first commit in this PR has its own pull request here since it seemed self-contained:
#57082

[ONNX] simplify batch_first logic in RNN tests
[ONNX] support GRU with packed input in scripting mode

This required two changes:

Add as_tensor to symbolic_opset9.py
Change torch::jit::pushPackingPastRnn to recognize and properly
replace another use of the batch_sizes output of prim::PackPadded.
Previously the code assumed that the first use was as input to the
RNN operator. However in some cases, it is also used to compute
max_batch_size. For example in this code:
https://github.com/pytorch/pytorch/blob/febff45/torch/nn/modules/rnn.py#L815-L815

With these changes the GRU tests now pass in scripting mode for opset
version >= 11.

Co-authored-by: Gary Miguel garymiguel@microsoft.com

Differential Revision: D28714805

Note the first commit in this PR has its own pull request here since it seemed self-contained: #57082 * [ONNX] simplify batch_first logic in RNN tests * [ONNX] support GRU with packed input in scripting mode This required two changes: * Add as_tensor to symbolic_opset9.py * Change torch::jit::pushPackingPastRnn to recognize and properly replace another use of the batch_sizes output of prim::PackPadded. Previously the code assumed that the first use was as input to the RNN operator. However in some cases, it is also used to compute max_batch_size. For example in this code: https://github.com/pytorch/pytorch/blob/febff45/torch/nn/modules/rnn.py#L815-L815 With these changes the GRU tests now pass in scripting mode for opset version >= 11. Co-authored-by: Gary Miguel <garymiguel@microsoft.com> [ghstack-poisoned]

facebook-github-bot · 2021-05-20T20:20:56Z

💊 CI failures summary and remediations

As of commit dcfccab (more details on the Dr. CI page):

7/7 failures possibly* introduced in this PR
- 1/7 non-scanned failure(s)

6 failures not recognized by patterns:

Job	Step	Action
^{Lint / mypy}	^{Run autogen}	🔁 rerun
^{Linux CI (pytorch-linux-xenial-py3.6-gcc5.4) / calculate-docker-image}	^{Checkout PyTorch}	🔁 rerun
^{Windows CI (pytorch-win-vs2019-cpu-py3) / test}	^{Setup Python3}	🔁 rerun
^{clang-format / clang-format}	^{Post Fetch PyTorch}	🔁 rerun
^{Windows CI (pytorch-win-vs2019-cpu-py3) / render_test_results}	^{Download PyTorch Test Reports}	🔁 rerun
^{Lint / quick-checks}	^{C++ docs check}	🔁 rerun

This comment was automatically generated by Dr. CI (expand for details).

Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

Note the first commit in this PR has its own pull request here since it seemed self-contained: #57082 * [ONNX] simplify batch_first logic in RNN tests * [ONNX] support GRU with packed input in scripting mode This required two changes: * Add as_tensor to symbolic_opset9.py * Change torch::jit::pushPackingPastRnn to recognize and properly replace another use of the batch_sizes output of prim::PackPadded. Previously the code assumed that the first use was as input to the RNN operator. However in some cases, it is also used to compute max_batch_size. For example in this code: https://github.com/pytorch/pytorch/blob/febff45/torch/nn/modules/rnn.py#L815-L815 With these changes the GRU tests now pass in scripting mode for opset version >= 11. Co-authored-by: Gary Miguel <garymiguel@microsoft.com> [ghstack-poisoned]

SplitInfinity · 2021-05-22T03:33:33Z

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

SplitInfinity · 2021-05-26T17:29:59Z

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

Note the first commit in this PR has its own pull request here since it seemed self-contained: #57082 * [ONNX] simplify batch_first logic in RNN tests * [ONNX] support GRU with packed input in scripting mode This required two changes: * Add as_tensor to symbolic_opset9.py * Change torch::jit::pushPackingPastRnn to recognize and properly replace another use of the batch_sizes output of prim::PackPadded. Previously the code assumed that the first use was as input to the RNN operator. However in some cases, it is also used to compute max_batch_size. For example in this code: https://github.com/pytorch/pytorch/blob/febff45/torch/nn/modules/rnn.py#L815-L815 With these changes the GRU tests now pass in scripting mode for opset version >= 11. Co-authored-by: Gary Miguel <garymiguel@microsoft.com> Differential Revision: [D28714805](https://our.internmc.facebook.com/intern/diff/D28714805) [ghstack-poisoned]

SplitInfinity · 2021-05-26T21:27:35Z

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

facebook-github-bot · 2021-05-27T19:07:02Z

@SplitInfinity merged this pull request in d101816.

Summary: Pull Request resolved: pytorch#58691 Note the first commit in this PR has its own pull request here since it seemed self-contained: pytorch#57082 * [ONNX] simplify batch_first logic in RNN tests * [ONNX] support GRU with packed input in scripting mode This required two changes: * Add as_tensor to symbolic_opset9.py * Change torch::jit::pushPackingPastRnn to recognize and properly replace another use of the batch_sizes output of prim::PackPadded. Previously the code assumed that the first use was as input to the RNN operator. However in some cases, it is also used to compute max_batch_size. For example in this code: https://github.com/pytorch/pytorch/blob/febff45/torch/nn/modules/rnn.py#L815-L815 With these changes the GRU tests now pass in scripting mode for opset version >= 11. Test Plan: Imported from OSS Reviewed By: driazati Differential Revision: D28714805 Pulled By: SplitInfinity fbshipit-source-id: f19647a04533d9ec76399a8793b3f712ea0337d2 Co-authored-by: Gary Miguel <garymiguel@microsoft.com>

From PR: #58691, Replacing the second input of `Gather` 0 to 1 affects other innocent Nodes. In Issue #91526 onnx::range starts from 0, the 0 is changed by this mechanism, as it's shared by onnx::Gather. This PR intends to create a whole independent Constant 0 for replacement. TODO: test [ghstack-poisoned]

…ole" From PR: #58691, Replacing the second input of `Gather` 0 to 1 affects other innocent Nodes. In Issue #91526 onnx::range starts from 0, the 0 is changed by this mechanism, as it's shared by onnx::Gather. This PR intends to create a whole independent Constant 0 for replacement. NOTE: The PR passes all existing RNN tests locally in case CI doesn't include RNN test. TODO: test [ghstack-poisoned]

From PR: #58691, Replacing the second input of `Gather` 0 to 1 affects other innocent Nodes. In Issue #91526 onnx::range starts from 0, the 0 is changed by this mechanism, as it's shared by onnx::Gather. This PR intends to create a whole independent Constant 0 for replacement. NOTE: The PR passes all existing RNN tests locally in case CI doesn't include RNN test. TODO: test [ghstack-poisoned]

…ole" From PR: #58691, Replacing the second input of `Gather` 0 to 1 affects other innocent Nodes. In Issue #91526 onnx::range starts from 0, the 0 is changed by this mechanism, as it's shared by onnx::Gather. This PR intends to create a whole independent Constant 0 for replacement. NOTE: The PR passes all existing RNN tests locally in case CI doesn't include RNN test. TODO: test [ghstack-poisoned]

From PR: #58691, Replacing the second input of `Gather` 0 to 1 affects other innocent Nodes. In Issue #91526 onnx::range starts from 0, the 0 is changed by this mechanism, as it's shared by onnx::Gather. This PR intends to create a whole independent Constant 0 for replacement. NOTE: The PR passes all existing RNN tests locally in case CI doesn't include RNN test. TODO: test [ghstack-poisoned]

…ole" From PR: #58691, Replacing the second input of `Gather` 0 to 1 affects other innocent Nodes. In Issue #91526 onnx::range starts from 0, the 0 is changed by this mechanism, as it's shared by onnx::Gather. This PR intends to create a whole independent Constant 0 for replacement. NOTE: The PR passes all existing RNN tests locally in case CI doesn't include RNN test. TODO: test [ghstack-poisoned]

From PR: #58691, Replacing the second input of `Gather` 0 to 1 affects other innocent Nodes. In Issue #91526 onnx::range starts from 0, the 0 is changed by this mechanism, as it's shared by onnx::Gather. This PR intends to create a whole independent Constant 0 for replacement. NOTE: The PR passes all existing RNN tests locally in case CI doesn't include RNN test. TODO: test [ghstack-poisoned]

…ole" From PR: #58691, Replacing the second input of `Gather` 0 to 1 affects other innocent Nodes. In Issue #91526 onnx::range starts from 0, the 0 is changed by this mechanism, as it's shared with onnx::Gather. This PR intends to create a whole independent Constant 0 for replacement. NOTE: The PR passes all existing RNN tests locally in case CI doesn't include RNN test. ~~TODO: test~~ [ghstack-poisoned]

From PR: #58691, Replacing the second input of `Gather` 0 to 1 affects other innocent Nodes. In Issue #91526 onnx::range starts from 0, the 0 is changed by this mechanism, as it's shared with onnx::Gather. This PR intends to create a whole independent Constant 0 for replacement. NOTE: The PR passes all existing RNN tests locally in case CI doesn't include RNN test. ~~TODO: test~~ [ghstack-poisoned]

From PR: #58691, Replacing the second input of `Gather` 0 to 1 affects other innocent Nodes. In Issue #91526 onnx::range starts from 0, the 0 is changed by this mechanism, as it's shared with onnx::Gather. This PR intends to create a whole independent Constant 0 for replacement. NOTE: The PR passes all existing RNN tests locally in case CI doesn't include RNN test. ~~TODO: test~~ Pull Request resolved: #93120 Approved by: https://github.com/BowenBao

BowenBao requested review from neginraoof and spandantiwari as code owners May 20, 2021 20:20

facebook-github-bot added cla signed oncall: jit Add this issue/PR to JIT oncall triage queue labels May 20, 2021

pytorchbot added the open source label May 20, 2021

facebook-github-bot closed this in d101816 May 27, 2021

facebook-github-bot added the Merged label May 27, 2021

facebook-github-bot deleted the gh/BowenBao/73/head branch May 31, 2021 14:17

garymm mentioned this pull request Oct 8, 2021

[v.1.10.0] Release Tracker #65438

Closed

titaiwangms mentioned this pull request Jan 27, 2023

[ONNX] Fix Gather replacement in RNN peephole #93120

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ONNX] RNN scripting (#57564) #58691

[ONNX] RNN scripting (#57564) #58691

BowenBao commented May 20, 2021 •

edited

Loading

facebook-github-bot commented May 20, 2021 •

edited

Loading

SplitInfinity commented May 22, 2021

SplitInfinity commented May 26, 2021

SplitInfinity commented May 26, 2021

facebook-github-bot commented May 27, 2021

[ONNX] RNN scripting (#57564) #58691

[ONNX] RNN scripting (#57564) #58691

Conversation

BowenBao commented May 20, 2021 • edited Loading

facebook-github-bot commented May 20, 2021 • edited Loading

💊 CI failures summary and remediations

6 failures not recognized by patterns:

SplitInfinity commented May 22, 2021

SplitInfinity commented May 26, 2021

SplitInfinity commented May 26, 2021

facebook-github-bot commented May 27, 2021

BowenBao commented May 20, 2021 •

edited

Loading

facebook-github-bot commented May 20, 2021 •

edited

Loading