Skip to content

Conversation

@supriyar
Copy link
Contributor

@supriyar supriyar commented Dec 3, 2019

Stack from ghstack:

Summary:
Caffe2 expects quantized ops to be in NHWC format while pytorch inputs are in NCHW.
Add a jit pass to insert permutes to convert from nchw2nhwc before each conv op and add nhwc2nchw permute after the conv op.
Using graph rewriter to find consecutive redundant permutes and remove them from the graph

Test Plan:
python test/onnx/test_pytorch_onnx_caffe2_quantized.py TestQuantizedOps

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: D18790518

Summary:
Caffe2 expects quantized ops to be in NHWC format while pytorch inputs are in NCHW.
Add a jit pass to insert permutes to convert from nchw2nhwc before each conv op and add nhwc2nchw permute after the conv op.
Using graph rewriter to find consecutive redundant permutes and remove them from the graph

Test Plan:
python test/onnx/test_pytorch_onnx_caffe2_quantized.py TestQuantizedOps

Reviewers:

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
@supriyar supriyar requested a review from apaszke as a code owner December 3, 2019 19:46
@facebook-github-bot facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Dec 3, 2019
…onv ops"

Summary:
Caffe2 expects quantized ops to be in NHWC format while pytorch inputs are in NCHW.
Add a jit pass to insert permutes to convert from nchw2nhwc before each conv op and add nhwc2nchw permute after the conv op.
Using graph rewriter to find consecutive redundant permutes and remove them from the graph

Test Plan:
python test/onnx/test_pytorch_onnx_caffe2_quantized.py TestQuantizedOps

Reviewers:

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
supriyar added a commit that referenced this pull request Dec 3, 2019
Summary:
Caffe2 expects quantized ops to be in NHWC format while pytorch inputs are in NCHW.
Add a jit pass to insert permutes to convert from nchw2nhwc before each conv op and add nhwc2nchw permute after the conv op.
Using graph rewriter to find consecutive redundant permutes and remove them from the graph

Test Plan:
python test/onnx/test_pytorch_onnx_caffe2_quantized.py TestQuantizedOps

Reviewers:

Subscribers:

Tasks:

Tags:

ghstack-source-id: 889aef9
Pull Request resolved: #30679
@supriyar supriyar requested a review from houseroad December 3, 2019 21:24
…onv ops"

Summary:
Caffe2 expects quantized ops to be in NHWC format while pytorch inputs are in NCHW.
Add a jit pass to insert permutes to convert from nchw2nhwc before each conv op and add nhwc2nchw permute after the conv op.
Using graph rewriter to find consecutive redundant permutes and remove them from the graph

Test Plan:
python test/onnx/test_pytorch_onnx_caffe2_quantized.py TestQuantizedOps

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: [D18790518](https://our.internmc.facebook.com/intern/diff/D18790518)

[ghstack-poisoned]
supriyar added a commit that referenced this pull request Dec 3, 2019
Summary:
Caffe2 expects quantized ops to be in NHWC format while pytorch inputs are in NCHW.
Add a jit pass to insert permutes to convert from nchw2nhwc before each conv op and add nhwc2nchw permute after the conv op.
Using graph rewriter to find consecutive redundant permutes and remove them from the graph

Test Plan:
python test/onnx/test_pytorch_onnx_caffe2_quantized.py TestQuantizedOps

Reviewers:

Subscribers:

Tasks:

Tags:

ghstack-source-id: 190567f
Pull Request resolved: #30679
…onv ops"

Summary:
Caffe2 expects quantized ops to be in NHWC format while pytorch inputs are in NCHW.
Add a jit pass to insert permutes to convert from nchw2nhwc before each conv op and add nhwc2nchw permute after the conv op.
Using graph rewriter to find consecutive redundant permutes and remove them from the graph

Test Plan:
python test/onnx/test_pytorch_onnx_caffe2_quantized.py TestQuantizedOps

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: [D18790518](https://our.internmc.facebook.com/intern/diff/D18790518)

[ghstack-poisoned]
supriyar added a commit that referenced this pull request Dec 4, 2019
Summary:
Caffe2 expects quantized ops to be in NHWC format while pytorch inputs are in NCHW.
Add a jit pass to insert permutes to convert from nchw2nhwc before each conv op and add nhwc2nchw permute after the conv op.
Using graph rewriter to find consecutive redundant permutes and remove them from the graph

Test Plan:
python test/onnx/test_pytorch_onnx_caffe2_quantized.py TestQuantizedOps

Reviewers:

Subscribers:

Tasks:

Tags:

ghstack-source-id: 68d1d19
Pull Request resolved: #30679
Copy link
Member

@houseroad houseroad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall the PR is good. Some inline comments to address.

torch._C._jit_pass_onnx_unpack_quantized_weights(graph, params_dict)

# Insert permutes before and after each conv op to ensure correct order.
torch._C._jit_pass_onnx_quantization_insert_permutes(graph, params_dict)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible we can use _jit_pass_custom_pattern_based_rewrite_graph to insert permutes as well?

Copy link
Contributor Author

@supriyar supriyar Dec 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did try that but got an assert of the form

terminate called after throwing an instance of 'c10::Error'
  what():  n->owningGraph() == this && n->inBlockList() INTERNAL ASSERT FAILED at ../torch/csrc/jit/ir.h:1191

I suspect adding new ops to the graph using the rewriter isn't updating all the required dependencies correctly.

op_node->insertInput(0, permute_node->output());

Node* permute_node_after = graph->create(
Symbol::fromQualString("quantized::nhwc2nchw"), {input_node->output()});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why use input_node->output()? shall we use op_node->outputs()[0] here?

…onv ops"

Summary:
Caffe2 expects quantized ops to be in NHWC format while pytorch inputs are in NCHW.
Add a jit pass to insert permutes to convert from nchw2nhwc before each conv op and add nhwc2nchw permute after the conv op.
Using graph rewriter to find consecutive redundant permutes and remove them from the graph

Test Plan:
python test/onnx/test_pytorch_onnx_caffe2_quantized.py TestQuantizedOps

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: [D18790518](https://our.internmc.facebook.com/intern/diff/D18790518)

[ghstack-poisoned]
supriyar added a commit that referenced this pull request Dec 5, 2019
Summary:
Caffe2 expects quantized ops to be in NHWC format while pytorch inputs are in NCHW.
Add a jit pass to insert permutes to convert from nchw2nhwc before each conv op and add nhwc2nchw permute after the conv op.
Using graph rewriter to find consecutive redundant permutes and remove them from the graph

Test Plan:
python test/onnx/test_pytorch_onnx_caffe2_quantized.py TestQuantizedOps

Reviewers:

Subscribers:

Tasks:

Tags:

ghstack-source-id: 22f7a0b
Pull Request resolved: #30679
@supriyar supriyar requested a review from houseroad December 5, 2019 02:20
Copy link
Member

@houseroad houseroad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Ship it.

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in a51c5f5.

@facebook-github-bot facebook-github-bot deleted the gh/supriyar/43/head branch December 10, 2019 15:20
wuhuikx pushed a commit to wuhuikx/pytorch that referenced this pull request Jan 30, 2020
Summary:
Pull Request resolved: pytorch#30679

Caffe2 expects quantized ops to be in NHWC format while pytorch inputs are in NCHW.
Add a jit pass to insert permutes to convert from nchw2nhwc before each conv op and add nhwc2nchw permute after the conv op.
Using graph rewriter to find consecutive redundant permutes and remove them from the graph

Test Plan:
python test/onnx/test_pytorch_onnx_caffe2_quantized.py TestQuantizedOps

Imported from OSS

Differential Revision: D18790518

fbshipit-source-id: 4dd39cf0b31b21f5586c0edfdce2260d4e245112
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Merged oncall: jit Add this issue/PR to JIT oncall triage queue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants