Fix bug in cpu_bfloat16_pass for MKLDNN #48481
Conversation
|
你的PR提交成功，感谢你对开源项目的贡献! |
| @@ -148,8 +148,8 @@ ProgramDesc BuildProgramDescDoubleInput(bool use_mkldnn) { | |||
|
|
|||
| TEST(CpuBfloat16Pass, double_input_ops) { | |||
| bool use_mkldnn = true; | |||
| int quant_op = 4; | |||
| int dequant_op = 3; | |||
| int quant_op = 3; | |||
There was a problem hiding this comment.
Previously, we had a model where one operator output was linked to Matmul X and Y inputs, so we had this unit test in paddle/fluid/framework/ir/mkldnn/cpu_bfloat16_pass_tester.cc.
What was the error that you want to disable bfloat16 for this example? Because maybe you don't need to turn it off, just adjust it to be supported, just like you added an example in the unit test.
There was a problem hiding this comment.
We found such a concat node. I think such a node can be skipped directly, and there is no need to add complicated algorithms to support it.
There was a problem hiding this comment.
Another solution is that we can delete the following code on line 44 in cpu_bfloat16_pass.cc：
if (IsAlreadyLinked(linked_xputs, physical_xput_name)) continue;
There was a problem hiding this comment.
@wozna Which plan do you think is more reasonable?
There was a problem hiding this comment.
@yeliang2258 Ah yes I also noticed such a connection in Picodet int8 and just added support for quantization in this PR #48872. I think the solution with if (IsAlreadyLinked(linked_xputs, physical_xput_name)) continue; can be a better solution.
There was a problem hiding this comment.
Ok, thank you for the suggestion, then I will modify it and delete the relevant logic of if (IsAlreadyLinked(linked_xputs, physical_xput_name)) continue;
|
Since you haven't replied for more than a year, we have closed this issue/pr. |
PR types
Bug fixes
PR changes
Others
Describe
Fix bug in cpu_bfloat16_pass for MKLDNN