From 367361fc74c8c749f1b857e79ced15f7d94b5f1a Mon Sep 17 00:00:00 2001 From: Hector Li Date: Mon, 18 Nov 2019 10:50:44 -0800 Subject: [PATCH] Fix the issue in matmul_add_fusion (#2407) Fix the issue in matmul_add_fusion If Muatmul + Add has shape [K] * [K, N], reset it to [1, K] * [K, N] will make the output shape to [1, N] will also requires a reshape on the output. Fix: just remove the shape reset to not fuse it. Add a negative test case for matmul+add fusion --- .../core/optimizer/matmul_add_fusion.cc | 9 -------- .../test/optimizer/graph_transform_test.cc | 21 ++++++++++++++++++ .../matmul_add_fusion/3Input/neg_model.onnx | Bin 0 -> 194 bytes 3 files changed, 21 insertions(+), 9 deletions(-) create mode 100644 onnxruntime/test/testdata/transform/matmul_add_fusion/3Input/neg_model.onnx diff --git a/onnxruntime/core/optimizer/matmul_add_fusion.cc b/onnxruntime/core/optimizer/matmul_add_fusion.cc index 2241244f6c..194ddc8beb 100644 --- a/onnxruntime/core/optimizer/matmul_add_fusion.cc +++ b/onnxruntime/core/optimizer/matmul_add_fusion.cc @@ -64,15 +64,6 @@ Status MatMulAddFusion::ApplyImpl(Graph& graph, bool& modified, int graph_level, continue; } - if (1 == matmul_a_shape->dim_size() && 2 == matmul_b_shape->dim_size()) { - // MatMul has shape [K] * [K, N], reset it to [1, K] * [K, N], so that it can work for Gemm - auto mutable_matmul_a_shape = const_cast(matmul_a_shape); - auto dim_0 = mutable_matmul_a_shape->mutable_dim(0); - auto dim_1 = (const_cast(matmul_a_shape))->add_dim(); - (*dim_1) = (*dim_0); - dim_0->set_dim_value(1); - } - if (2 != matmul_a_shape->dim_size() || 2 != matmul_b_shape->dim_size()) { // Gemm only support Matrix continue; diff --git a/onnxruntime/test/optimizer/graph_transform_test.cc b/onnxruntime/test/optimizer/graph_transform_test.cc index 93d40cbd9a..17c478eab5 100644 --- a/onnxruntime/test/optimizer/graph_transform_test.cc +++ b/onnxruntime/test/optimizer/graph_transform_test.cc @@ -542,6 +542,27 @@ TEST(GraphTransformationTests, MatMulAddFusion_three_input) { ASSERT_TRUE(op_to_count["Gemm"] == 1); } +// Matmul+Add with shape [k]*[k,N]+[N], won't do the fusion +// We can do the fusion by changing shape to [1,k]*[k,N]+[1,N], then add a reshape [1,N]=>[N] +// This will bring extra cost. And there's only very limited gain to fuse Matmul+Add to Gemm +// Since the basic implementation is almost same +TEST(GraphTransformationTests, MatMulAddFusion_negitive_case) { + auto model_uri = MODEL_FOLDER "matmul_add_fusion/3Input/neg_model.onnx"; + + std::shared_ptr p_model; + ASSERT_TRUE(Model::Load(model_uri, p_model, nullptr, DefaultLoggingManager().DefaultLogger()).IsOK()); + Graph& graph = p_model->MainGraph(); + + onnxruntime::GraphTransformerManager graph_transformation_mgr{5}; + graph_transformation_mgr.Register(onnxruntime::make_unique(), TransformerLevel::Level1); + ASSERT_TRUE(graph_transformation_mgr.ApplyTransformers(graph, TransformerLevel::Level1, DefaultLoggingManager().DefaultLogger()).IsOK()); + + std::map op_to_count = CountOpsInGraph(graph); + ASSERT_TRUE(op_to_count["MatMul"] == 1); + ASSERT_TRUE(op_to_count["Add"] == 1); + ASSERT_TRUE(op_to_count["Gemm"] == 0); +} + #ifndef DISABLE_CONTRIB_OPS TEST(GraphTransformationTests, Gemm_Relu_three_input) { auto model_uri = MODEL_FOLDER "matmul_add_fusion/3Input/gemm_relu.onnx"; diff --git a/onnxruntime/test/testdata/transform/matmul_add_fusion/3Input/neg_model.onnx b/onnxruntime/test/testdata/transform/matmul_add_fusion/3Input/neg_model.onnx new file mode 100644 index 0000000000000000000000000000000000000000..fa8c1dccbaef2ade1e7ce974aece1447d5c74ccc GIT binary patch literal 194 zcmd;J72?Uy%d5~$tw_u*$Vs(Y$;c(e#puYz=p@7xY$(O%n^@vonxh1zxkR{tqFjv5 zLX0j_%#JB3N+3c=IybQ-w=^d{F(oCwI5RIjCpA7LGdDgrKP5FMiXUj45EmB*2cr-R v7ZV3dlrYF}J}w@h00&3_h?zmEoRL&9CjkXqkOi^?KvpA*II(as2yg=c6m}tV literal 0 HcmV?d00001