[Quant] Support lowering of channel shuffle in FX #83731

Xia-Weiwen · 2022-08-19T08:24:03Z

Description

Support lowering of channel shuffle in FX by adding its module and functional op to is_copy_node list in torch/ao/quantization/fx/_lower_to_native_backend.py

Validation

UTs added to test

correctness of quantized ChannelShuffle module.
FX lowering of ChannelShuffle module and functional channel_shuffle.

cc @jerryzh168 @jianyuh @raghuramank100 @jamesr66a @vkuzo @jgong5 @leslie-fang-intel @mingfeima @XiaobingSuper @sanchitintel @ashokei @jingxu10 @ezyang @SherlockNoMad @soumith @EikanWang @wenzhe-nrv

facebook-github-bot · 2022-08-19T08:24:09Z

🔗 Helpful links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/83731
✖️ Python docs build was skipped
✖️ C++ docs build was skipped
❓Need help or want to give feedback on the CI? Visit our office hours

✅ No Failures (0 Pending)

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

Expand to see more

💚 💚 Looks good so far! There are no failures yet. 💚 💚

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

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

Click here to manually regenerate this comment.

vadimkantorov · 2022-08-19T10:55:12Z

torch/nn/quantized/modules/channelshuffle.py

+import torch
+
+class ChannelShuffle(torch.nn.ChannelShuffle):
+    def __init__(self, groups: int):


is this constructor needed? since it just passes through the arguments

the base-derived constructor would be sufficient?

and maybe same question about forward impl

Thanks. They are removed.

Xia-Weiwen · 2022-08-24T00:35:19Z

@vadimkantorov Please review again. Thanks.

vadimkantorov · 2022-08-24T07:23:59Z

@Xia-Weiwen I'm not a member of PyTorch team, so a review from someone else is required :)

Xia-Weiwen · 2022-08-24T07:25:18Z

@Xia-Weiwen I'm not a member of PyTorch team, so a review from someone else is required :)

I see. Thank you anyway.

Xia-Weiwen · 2022-08-24T07:26:45Z

Hi @jerryzh168 Could you please review this PR? Or could you ask someone else to review? Thanks!

jerryzh168 · 2022-08-24T15:59:12Z

torch/ao/quantization/fx/_lower_to_native_backend.py

        torch.nn.BatchNorm2d,
        torch.nn.BatchNorm3d,
+        torch.nn.ChannelShuffle,


why do we have two channel shuffle here?

Sorry for the typo. Now they are removed.

jerryzh168 · 2022-08-24T16:00:20Z

torch/nn/quantized/modules/channelshuffle.py

@@ -0,0 +1,16 @@
+import torch
+
+class ChannelShuffle(torch.nn.ChannelShuffle):


if the float version works for quantized inputs as well, I don't think we will need to define an extra quantized module

it should be a part of this list in that case I think: https://github.com/pytorch/pytorch/blob/master/torch/ao/quantization/fx/_lower_to_native_backend.py#L122

OK. I have removed them.

jerryzh168 · 2022-08-24T16:02:21Z

torch/ao/quantization/backend_config/_common_operator_config_utils.py

@@ -353,6 +353,7 @@ def _get_default_op_configs(dtype_configs: List[DTypeConfig]) -> List[BackendPat
        torch.nn.LayerNorm,
        torch.nn.Dropout,
        torch.nn.PReLU,
+        torch.nn.ChannelShuffle,


should add this to _get_share_qprams_op_backend_config I think

I have added channel shuffle to copy node list. Is this still needed?

jerryzh168

channel shuffle should be categorized as copy node I think, so we don't need to add new quantized modules and configs and lowering need to change as well, please see inline comments for changes

Xia-Weiwen · 2022-08-25T06:23:39Z

channel shuffle should be categorized as copy node I think, so we don't need to add new quantized modules and configs and lowering need to change as well, please see inline comments for changes

Thanks for reviewing. I have removed most of previous changes. Now I only add it to the is_copy_node list and add two unit tests. Please review again. Thanks.

jerryzh168 · 2022-10-27T18:01:40Z

test/quantization/fx/test_quantize_fx.py

+        models = [M1().eval(), M2().eval(), M3().eval()]
+        # torch.channel_shuffle is torch.nn.functional.channel_shuffle
+        expected_nodes = [
+            ns.call_module(torch.nn.ChannelShuffle),
+            ns.call_function(torch.channel_shuffle),
+            ns.call_function(torch.channel_shuffle)
+        ]


nit: might be better to just write a list of tuple of (model, expected_node) so that it's clearer

OK. It's done. Please take a look.

jerryzh168

looks good, please add expected channel shuffle op to node_occurrence as well

Xia-Weiwen · 2022-10-31T05:12:13Z

Hi @jerryzh168 I changed per your comments. All checks have passed. Do you have more comments? Thanks.

Xia-Weiwen · 2022-11-08T10:00:25Z

Hi @jerryzh168 Do you think it's ok to land this?

jerryzh168

LG, please add a comment in torch/ao/quantization/fx/_lower_to_native_backend.py about why we only need torch.channel_shuffle and not F.channel_shuffle

Xia-Weiwen · 2022-11-09T05:04:35Z

LG, please add a comment in torch/ao/quantization/fx/_lower_to_native_backend.py about why we only need torch.channel_shuffle and not F.channel_shuffle

Thanks. I will add that comment and land it after all CI checks pass.

…tQuantizeFx.test_channel_shuffle_lowering

…wer_to_native_backend.py

Xia-Weiwen · 2022-11-09T08:06:21Z

@pytorchbot merge

pytorchmergebot · 2022-11-09T08:08:06Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

## Description Support lowering of channel shuffle in FX by adding its module and functional op to `is_copy_node` list in `torch/ao/quantization/fx/_lower_to_native_backend.py` ## Validation UTs added to test - correctness of quantized `ChannelShuffle` module. - FX lowering of `ChannelShuffle` module and functional `channel_shuffle`. Pull Request resolved: pytorch#83731 Approved by: https://github.com/jerryzh168

Xia-Weiwen requested review from albanD, jbschlosser and saketh-are as code owners August 19, 2022 08:24

facebook-github-bot added the cla signed label Aug 19, 2022

facebook-github-bot added the fx label Aug 19, 2022

pytorchbot added the open source label Aug 19, 2022

Xia-Weiwen force-pushed the quantized_channel_shuffle branch from 051843b to 845a466 Compare August 19, 2022 08:39

vadimkantorov reviewed Aug 19, 2022

View reviewed changes

mikaylagawarecki added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Aug 19, 2022

Xia-Weiwen requested review from vadimkantorov and removed request for saketh-are, albanD and jbschlosser August 22, 2022 00:52

Xia-Weiwen force-pushed the quantized_channel_shuffle branch 2 times, most recently from 7cbcfc8 to 22533a5 Compare August 23, 2022 07:49

yanbing-j added intel priority matters to intel architecture from performance wise intel This tag is for PR from Intel labels Aug 24, 2022

jerryzh168 reviewed Aug 24, 2022

View reviewed changes

jerryzh168 requested changes Aug 24, 2022

View reviewed changes

Xia-Weiwen force-pushed the quantized_channel_shuffle branch from 21caf8c to 8371853 Compare August 25, 2022 06:21

Xia-Weiwen requested review from jerryzh168 and removed request for vadimkantorov August 25, 2022 06:26

Xia-Weiwen requested a review from jgong5 October 21, 2022 07:01

Xia-Weiwen force-pushed the quantized_channel_shuffle branch 2 times, most recently from ec6c344 to 874872f Compare October 27, 2022 07:34

jerryzh168 reviewed Oct 27, 2022

View reviewed changes

jerryzh168 approved these changes Oct 27, 2022

View reviewed changes

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Oct 27, 2022

Xia-Weiwen requested a review from jerryzh168 October 28, 2022 02:08

Xia-Weiwen force-pushed the quantized_channel_shuffle branch from 2fc90cc to 0bae9de Compare October 29, 2022 05:19

Xia-Weiwen force-pushed the quantized_channel_shuffle branch from 0bae9de to a6a6e30 Compare November 4, 2022 00:24

github-actions bot added the oncall: quantization Quantization support in PyTorch label Nov 4, 2022

Xia-Weiwen force-pushed the quantized_channel_shuffle branch from a6a6e30 to 2a92764 Compare November 7, 2022 05:41

jerryzh168 approved these changes Nov 9, 2022

View reviewed changes

Xia-Weiwen added 6 commits November 9, 2022 13:06

Support lowering of channel shuffle in FX

1c4a1c6

Remove unnecessary code

ffe13c9

Check occurrences of functional channel_shuffle and its module in Tes…

a9d3830

…tQuantizeFx.test_channel_shuffle_lowering

Modify the loop in test_channel_shuffle_lowering

d31b146

Update test case in test_quantize_fx.py

ce7e667

Fix lint

9f66a0f

Xia-Weiwen force-pushed the quantized_channel_shuffle branch from 2a92764 to 9f66a0f Compare November 9, 2022 05:08

Add comment for torch.channel_shuffle in torch/ao/quantization/fx/_lo…

5080c58

…wer_to_native_backend.py

pytorchmergebot added the Merged label Nov 9, 2022

pytorchmergebot closed this in f6192b7 Nov 9, 2022

Xia-Weiwen deleted the quantized_channel_shuffle branch December 1, 2022 05:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Quant] Support lowering of channel shuffle in FX #83731

[Quant] Support lowering of channel shuffle in FX #83731

Xia-Weiwen commented Aug 19, 2022 •

edited by pytorch-bot bot

facebook-github-bot commented Aug 19, 2022 •

edited

vadimkantorov Aug 19, 2022 •

edited

Xia-Weiwen Aug 22, 2022

Xia-Weiwen commented Aug 24, 2022

vadimkantorov commented Aug 24, 2022 •

edited

Xia-Weiwen commented Aug 24, 2022

Xia-Weiwen commented Aug 24, 2022

jerryzh168 Aug 24, 2022

Xia-Weiwen Aug 25, 2022

jerryzh168 Aug 24, 2022

jerryzh168 Aug 24, 2022

Xia-Weiwen Aug 25, 2022

jerryzh168 Aug 24, 2022

Xia-Weiwen Aug 25, 2022

jerryzh168 left a comment

Xia-Weiwen commented Aug 25, 2022

jerryzh168 Oct 27, 2022

Xia-Weiwen Oct 28, 2022

jerryzh168 left a comment

Xia-Weiwen commented Oct 31, 2022

Xia-Weiwen commented Nov 8, 2022

jerryzh168 left a comment

Xia-Weiwen commented Nov 9, 2022

Xia-Weiwen commented Nov 9, 2022

pytorchmergebot commented Nov 9, 2022

		@@ -0,0 +1,16 @@
		import torch

		class ChannelShuffle(torch.nn.ChannelShuffle):

[Quant] Support lowering of channel shuffle in FX #83731

[Quant] Support lowering of channel shuffle in FX #83731

Conversation

Xia-Weiwen commented Aug 19, 2022 • edited by pytorch-bot bot

Description

Validation

facebook-github-bot commented Aug 19, 2022 • edited

🔗 Helpful links

✅ No Failures (0 Pending)

vadimkantorov Aug 19, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Xia-Weiwen commented Aug 24, 2022

vadimkantorov commented Aug 24, 2022 • edited

Xia-Weiwen commented Aug 24, 2022

Xia-Weiwen commented Aug 24, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerryzh168 left a comment

Choose a reason for hiding this comment

Xia-Weiwen commented Aug 25, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerryzh168 left a comment

Choose a reason for hiding this comment

Xia-Weiwen commented Oct 31, 2022

Xia-Weiwen commented Nov 8, 2022

jerryzh168 left a comment

Choose a reason for hiding this comment

Xia-Weiwen commented Nov 9, 2022

Xia-Weiwen commented Nov 9, 2022

pytorchmergebot commented Nov 9, 2022

Merge started

Xia-Weiwen commented Aug 19, 2022 •

edited by pytorch-bot bot

facebook-github-bot commented Aug 19, 2022 •

edited

vadimkantorov Aug 19, 2022 •

edited

vadimkantorov commented Aug 24, 2022 •

edited