fuse quantize+transpose and transpose+dequantize #49509

paulinagacek · 2023-01-03T10:50:43Z

PR types

Others

PR changes

Others

Describe

U2++ model often has chains of quantize followed by transpose and transpose followed by dequantize operators.
As quantize, dequantize and transpose are all reorders we could extend transpose to do quantize or dequantize operation.

This PR aims at adding a fuse pass which makes below fuses:

fp32 -> quantize -> transpose2 fp32 -> transpose2 (output int8)
int8 -> transpose2 -> dequantize int8 -> transpose2 (output fp32)

Performance improvement (faster by 0.08s):
In BERT model avg. inference time per batch:

without fuse - 28.24ms
with fuse - 28.16ms

paddle-bot · 2023-01-03T10:50:47Z

你的PR提交成功，感谢你对开源项目的贡献!
请关注后续CI自动化测试结果，详情请参考Paddle-CI手册。
Your PR has been submitted. Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

… fuse_quant_tranpose

paulinagacek · 2023-01-27T12:25:23Z

@wozna @sfraczek could you review please?

sfraczek

Great work 👍 . I just have a few minor suggestions.

sfraczek · 2023-01-30T08:46:20Z

paddle/fluid/framework/ir/mkldnn/quant_transpose2_dequant_onednn_fuse_pass.h

@@ -0,0 +1,37 @@
+// Copyright (c) 2022 PaddlePaddle Authors. All Rights Reserved.


sfraczek · 2023-01-30T08:46:29Z

paddle/fluid/framework/ir/mkldnn/quant_transpose2_dequant_onednn_fuse_pass.cc

@@ -0,0 +1,173 @@
+// Copyright (c) 2022 PaddlePaddle Authors. All Rights Reserved.


sfraczek · 2023-01-30T08:46:43Z

...n/paddle/fluid/tests/unittests/ir/inference/test_onednn_quant_transpose_dequant_fuse_pass.py

@@ -0,0 +1,121 @@
+# Copyright (c) 2021 PaddlePaddle Authors. All Rights Reserved.


sfraczek · 2023-01-30T08:48:47Z

paddle/fluid/framework/ir/mkldnn/quant_transpose2_dequant_onednn_fuse_pass.cc

+    Graph *graph) const {
+  PADDLE_ENFORCE_NOT_NULL(
+      graph, platform::errors::InvalidArgument("Graph cannot be nullptr."));
+  FusePassBase::Init("quant_transpose2_dequant_onednn_fuse_pass", graph);


I would suggest putting this string in a private variable named name_scope. You can put it in header in the class declaration so it's accessible in both functions.

sfraczek · 2023-01-30T09:55:01Z

paddle/fluid/framework/ir/mkldnn/quant_transpose2_dequant_onednn_fuse_pass.cc

+    if (!transpose2_op->Op()->HasAttr("use_mkldnn") ||
+        (transpose2_op->Op()->HasAttr("use_mkldnn") &&
+         !(PADDLE_GET_CONST(bool,
+                            transpose2_op->Op()->GetAttr("use_mkldnn"))))) {


When first condition gets evaluated to false, only then the second condition is evaluated so you can safely remove the second check for existance of attribute use_mkldnn and just get the value.

sfraczek · 2023-01-30T13:45:36Z

paddle/fluid/framework/ir/graph_pattern_detector.cc

+                       ->assert_is_op_input("quantize", "Input");
+  auto *quant_op = pattern->NewNode(quant_op_repr())->assert_is_op("quantize");
+  auto *quant_out = pattern->NewNode(quant_out_repr())
+                        ->AsOutput()


I think this should be AsIntermediate() because it's removed in pass.

sfraczek · 2023-01-30T13:46:26Z

paddle/fluid/framework/ir/graph_pattern_detector.cc

+  auto *transpose2_op =
+      pattern->NewNode(transpose2_op_repr())->assert_is_op("transpose2");
+  auto dequant_in = pattern->NewNode(dequant_in_repr())
+                        ->assert_is_op_input("dequantize", "Input");


Here it's also removed so I think it should be AsIntermediate()

sfraczek · 2023-01-30T14:01:30Z

...n/paddle/fluid/tests/unittests/ir/inference/test_onednn_quant_transpose_dequant_fuse_pass.py

+            inputs={'Input': ['transpose2_output_2']},
+            outputs={'Output': ['dequantize_output']},
+            attrs={
+                'is_negative_input': False,


I don't know if dequantize uses this attribute ?

You're right, it does not

Anyway, as we discussed, let's leave it because some other places in code do read or write this attribute.

sfraczek · 2023-01-30T14:01:57Z

...n/paddle/fluid/tests/unittests/ir/inference/test_onednn_quant_transpose_dequant_fuse_pass.py

+            inputs={'Input': ['input_data']},
+            outputs={'Output': ['quantize_output']},
+            attrs={
+                'is_negative_input': False,


Could/should this also be sampled from True or False? If yes, please make sure generate_input function is providing data in accordance to the value.

sfraczek · 2023-01-30T14:08:36Z

...n/paddle/fluid/tests/unittests/ir/inference/test_onednn_quant_transpose_dequant_fuse_pass.py

+    def sample_predictor_configs(self, program_config):
+        config = self.create_inference_config(
+            use_mkldnn=True,
+            passes=['quant_transpose2_dequant_onednn_fuse_pass'],
+        )
+        yield config, ['transpose2', 'transpose2'], (1e-5, 1e-5)
+
+    def test(self):
+        self.run_and_statis(
+            quant=False, passes=['quant_transpose2_dequant_onednn_fuse_pass']
+        )


I don't see passes being defined in sample_predictor_configs in other tests. Maybe there it is not required?

without it test does not see the pass

sfraczek

I found a few more

sfraczek · 2023-01-30T14:25:48Z

paddle/fluid/framework/ir/mkldnn/quant_transpose2_dequant_onednn_fuse_pass.cc

+
+  int found_patterns_count = 0;
+  auto handler = [&](const GraphPatternDetector::subgraph_t &subgraph,
+                     Graph *g) {


@Silv3S is our expert on this: He noticed lack of the below check and a need to add in constructor of the class some calls to AddOpCompat like in other passes.

Suggested change

Graph *g) {

Graph *g) {

if (!IsCompat(subgraph, g)) {

LOG(WARNING) << "Pass in op compat failed.";

return;

}

With this check I have an error: ValueError: (InvalidArgument) At least one OpCompat instance should be added

Have you added something like this? https://github.com/PaddlePaddle/Paddle/pull/43519/files#diff-56e4db16c655c73ec8154c0e6530c3a5bde9d180170e4308efd1ff827771520eR97

sfraczek · 2023-01-30T14:28:40Z

paddle/fluid/framework/ir/mkldnn/quant_transpose2_dequant_onednn_fuse_pass.cc

+REGISTER_PASS_CAPABILITY(quant_transpose2_dequant_onednn_fuse_pass)
+    .AddCombination(
+        paddle::framework::compatible::OpVersionComparatorCombination().GE(
+            "transpose2", 0));


@Silv3S should this be .EQ("transpose2", 0);? Because there is only version 0 and we don't know what will be in the future?
Also there should be similar line for quantize op but .LE("quantize", 2)) because it works regardless if shift attr or bfloat is present? (See the bottom of quantize_op.cc)

I think it's more elegant to add .EQ rather than .GE, because we can't be sure that newer version will be compatible with this pass

sfraczek · 2023-01-30T14:34:41Z

paddle/phi/kernels/onednn/transpose_kernel.cc

+  if (output_data_type == "bf16") {
+    out_dtype = DataType::BFLOAT16;


I think we have no test for this case here if we have performed a fuse and the fuse has set bf16

@sfraczek So what would you suugest?

To add a unit test or to add enforce it's not bf16 until this feature is validated for bf16.

sfraczek · 2023-01-30T14:37:52Z

paddle/fluid/framework/ir/mkldnn/quant_transpose2_dequant_onednn_fuse_pass.cc

+    bool is_bfloat =
+        quant_op->Op()->HasAttr("bfloat16")
+            ? PADDLE_GET_CONST(bool, quant_op->Op()->GetAttr("bfloat16"))
+            : false;


I wonder if we don't have to deal with an atribute output_format from quantize_op.cc?

This attribute is only set but never used, so I skipped it

wozna · 2023-01-30T16:27:21Z

paddle/fluid/framework/ir/graph_pattern_detector.cc

+                       ->AsInput()
+                       ->assert_is_op_input("quantize", "Input");
+  auto *quant_op = pattern->NewNode(quant_op_repr())->assert_is_op("quantize");
+  auto *quant_out = pattern->NewNode(quant_out_repr())


I think you should check if quant_out has only one output, because we want to make sure that there is only one connection between quantize and transpose so we can safely remove quantize.

wozna · 2023-01-30T16:28:23Z

paddle/fluid/framework/ir/graph_pattern_detector.cc

+}
+
+PDNode *patterns::Transpose2Dequant::operator()() {
+  auto *transpose2_op =


Same here, check if dequant_in has only one output.

shouldn't we check if dequant_in has only one input?

… fuse_quant_tranpose

sfraczek · 2023-02-02T16:12:48Z

paddle/fluid/framework/ir/graph_pattern_detector.cc

-      pattern->NewNode(dequant_op_repr())->assert_is_op("dequantize");
+  auto dequant_op = pattern->NewNode(dequant_op_repr())
+                        ->assert_is_op("dequantize")
+                        ->AsIntermediate();


I think AsIntermediate has no effect on ops, only vars. I meant that you add this to the dequant_in above if it's removed.

sfraczek · 2023-02-02T16:20:29Z

paddle/fluid/framework/ir/graph_pattern_detector.cc

-  auto *quant_op = pattern->NewNode(quant_op_repr())->assert_is_op("quantize");
+  auto *quant_op = pattern->NewNode(quant_op_repr())
+                       ->assert_is_op("quantize")
+                       ->AsIntermediate();


Same here, ops don't seem to have roles. I think they only concern links. However, I don't know what all the code is doing in this file so I may be wrong but that's what we've been doing :).

If you found out that we should add AsIntermediate to removed nodes, it would be great and we could fix all patterns.

@sfraczek From what I've found in code AsIntermediate is used only during subgraph validation - adding AsIntermediate prevents node from being used in multiple overlapping patterns or linked with external nodes.

Can you tell if it is only for vars or ops too?

… fuse_quant_tranpose

sfraczek · 2023-02-03T09:10:03Z

paddle/fluid/framework/ir/graph_pattern_detector.cc

+                        ->AsInput()
+                        ->AsIntermediate()


AsIntermediate() overwrites the result of AsInput()

You're right thanks!

… fuse_quant_tranpose

sfraczek

LGTM :)

wozna

LGTM

paddle-bot · 2023-02-08T13:04:18Z

你的PR已合入Paddle库，请关注后续测试结果。
Your PR has been merged into the repository. An official integration test will be conducted later. Stay tuned.

paddle-bot bot added contributor External developers status: proposed labels Jan 3, 2023

Silv3S added the Intel label Jan 3, 2023

paddle-bot bot removed the status: proposed label Jan 3, 2023

onecatcn assigned yeliang2258 Jan 7, 2023

paulinagacek added 12 commits January 9, 2023 09:24

QuantTranpose pattern is being found by pass

8232bbe

quant + transpose fuse

dd593de

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

f9b3813

… fuse_quant_tranpose

code style changes

ff9d0ba

UT written, reorder fixed

b54b5f6

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

db9f824

… fuse_quant_tranpose

Dequantize + transpose2 fuse added

6dfdf2d

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

5ad2b62

… fuse_quant_tranpose

pass name changed

8ed8af0

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

340634b

… fuse_quant_tranpose

UT added & shift corrected

b6d7048

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

0192d21

… fuse_quant_tranpose

paulinagacek marked this pull request as ready for review January 16, 2023 15:37

paulinagacek added 2 commits January 16, 2023 17:37

got rid of redundancy

ac0667d

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

bbe8fad

… fuse_quant_tranpose

jczaja self-requested a review January 30, 2023 10:11

$sfraczek$

sfraczek reviewed Jan 30, 2023

View reviewed changes

wozna reviewed Jan 30, 2023

View reviewed changes

paulinagacek added 2 commits February 1, 2023 16:17

review changes

b25be03

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

7e4018f

… fuse_quant_tranpose

$sfraczek$

sfraczek reviewed Feb 2, 2023

View reviewed changes

AsIntermediate corrected

f005c98

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

fb3aa34

… fuse_quant_tranpose

$sfraczek$

sfraczek reviewed Feb 3, 2023

View reviewed changes

paulinagacek added 2 commits February 3, 2023 11:09

compat added

cd54e09

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

92e714b

… fuse_quant_tranpose

onecatcn assigned weishengying and unassigned yeliang2258 Feb 7, 2023

paulinagacek requested review from sfraczek and removed request for jczaja February 7, 2023 08:39

$sfraczek$

sfraczek approved these changes Feb 7, 2023

View reviewed changes

wozna approved these changes Feb 7, 2023

View reviewed changes

jczaja merged commit 197a4ff into PaddlePaddle:develop Feb 8, 2023

paddle-bot bot added the status: accepted label Feb 8, 2023

Silv3S mentioned this pull request Feb 16, 2023

Extract fused_transpose op dedicated for oneDNN fuse passes #50021

Merged

		@@ -0,0 +1,37 @@
		// Copyright (c) 2022 PaddlePaddle Authors. All Rights Reserved.

		@@ -0,0 +1,173 @@
		// Copyright (c) 2022 PaddlePaddle Authors. All Rights Reserved.

		@@ -0,0 +1,121 @@
		# Copyright (c) 2021 PaddlePaddle Authors. All Rights Reserved.

		if (output_data_type == "bf16") {
		out_dtype = DataType::BFLOAT16;

fuse quantize+transpose and transpose+dequantize #49509

fuse quantize+transpose and transpose+dequantize #49509

Conversation

paulinagacek commented Jan 3, 2023 • edited Loading

PR types

PR changes

Describe

paddle-bot bot commented Jan 3, 2023

paulinagacek commented Jan 27, 2023

sfraczek left a 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

sfraczek Jan 30, 2023 • edited Loading

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

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

sfraczek left a comment • edited Loading

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

paulinagacek Feb 1, 2023 • edited Loading

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

Choose a reason for hiding this comment

sfraczek Feb 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sfraczek Feb 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sfraczek left a comment

Choose a reason for hiding this comment

wozna left a comment

Choose a reason for hiding this comment

paddle-bot bot commented Feb 8, 2023

paulinagacek commented Jan 3, 2023 •

edited

Loading

$@sfraczek$ sfraczek left a comment

$@sfraczek$ sfraczek Jan 30, 2023 •

edited

Loading

$@sfraczek$ sfraczek left a comment •

edited

Loading

paulinagacek Feb 1, 2023 •

edited

Loading

$@sfraczek$ sfraczek Feb 2, 2023 •

edited

Loading

$@sfraczek$ sfraczek Feb 3, 2023 •

edited

Loading

$@sfraczek$ sfraczek left a comment