From df866f7248459af230c467d9377903c4c9d3d257 Mon Sep 17 00:00:00 2001 From: Zhang Qinghua Date: Sat, 6 Mar 2021 12:03:29 +0800 Subject: [PATCH] Add TopoSort Rhs First attribute for special CNode, such as Depend CNode with isolated nodes. --- mindspore/ccsrc/frontend/optimizer/opt.cc | 6 +++--- .../ccsrc/pipeline/jit/parse/function_block.cc | 10 ++++++++-- mindspore/ccsrc/utils/utils.h | 1 + mindspore/core/ir/graph_utils.cc | 13 +++++++++++-- tests/st/ops/cpu/test_dot_op.py | 4 ++-- tests/ut/python/nn/test_nn_embedding.py | 2 +- tests/ut/python/nn/test_ssim.py | 8 ++++---- 7 files changed, 30 insertions(+), 14 deletions(-) diff --git a/mindspore/ccsrc/frontend/optimizer/opt.cc b/mindspore/ccsrc/frontend/optimizer/opt.cc index db25c9810f..3d6a8719c4 100644 --- a/mindspore/ccsrc/frontend/optimizer/opt.cc +++ b/mindspore/ccsrc/frontend/optimizer/opt.cc @@ -228,13 +228,13 @@ bool SubstitutionList::ApplySubstitutionToIR(const OptimizerPtr &optimizer, cons bool change = false; auto res = DoTransform(optimizer, node, substitution); if (res != nullptr) { + if (is_once_) { + return true; + } change = true; changes = true; node = res; } - if (change && is_once_) { - return true; - } UpdateTransformingList(optimizer, node, &todo, change, seen); } diff --git a/mindspore/ccsrc/pipeline/jit/parse/function_block.cc b/mindspore/ccsrc/pipeline/jit/parse/function_block.cc index 971872b023..657d1e4017 100644 --- a/mindspore/ccsrc/pipeline/jit/parse/function_block.cc +++ b/mindspore/ccsrc/pipeline/jit/parse/function_block.cc @@ -17,14 +17,17 @@ */ #include "pipeline/jit/parse/function_block.h" + #include #include + +#include "pybind11/pybind11.h" #include "pipeline/jit/parse/resolve.h" #include "pipeline/jit/parse/parse.h" #include "frontend/operator/ops.h" #include "utils/info.h" #include "debug/trace.h" -#include "pybind11/pybind11.h" +#include "utils/utils.h" namespace mindspore { namespace py = pybind11; @@ -435,7 +438,10 @@ void FunctionBlock::AttachIsolatedNodesBeforeReturn() { old_output = NewValueNode(kNone); } AnfNodePtr stop_grad_node = func_graph()->NewCNode({NewValueNode(prim::kPrimStopGradient), state}); - AnfNodePtr depend_node = func_graph()->NewCNode({NewValueNode(prim::kPrimDepend), old_output, stop_grad_node}); + CNodePtr depend_node = func_graph()->NewCNode({NewValueNode(prim::kPrimDepend), old_output, stop_grad_node}); + // We add this attribute for @constexpr use scene, since we must infer them before other nodes. + // That means isolated nodes will be evaluated first. It's not complete, but works in most scenes. + depend_node->AddAttr(kAttrTopoSortRhsFirst, MakeValue(true)); MS_LOG(INFO) << "Attached for side-effect nodes, depend_node: " << depend_node->DebugString() << ", state: " << state->DebugString(2); func_graph()->set_output(depend_node, true); diff --git a/mindspore/ccsrc/utils/utils.h b/mindspore/ccsrc/utils/utils.h index a2d3aef38a..37804d3848 100644 --- a/mindspore/ccsrc/utils/utils.h +++ b/mindspore/ccsrc/utils/utils.h @@ -397,6 +397,7 @@ constexpr auto kAttrRecompute = "recompute"; constexpr auto kAttrNeedCseAfterRecompute = "need_cse_after_recompute"; constexpr auto kAttrParallelDimInfo = "parallel_dim_info"; constexpr auto kAttrStitch = "stitch"; +constexpr auto kAttrTopoSortRhsFirst = "topo_sort_rhs_first"; // attr value constexpr auto kValueTargetSwitch = "target_switch"; diff --git a/mindspore/core/ir/graph_utils.cc b/mindspore/core/ir/graph_utils.cc index e7df960a0e..67b5050b32 100644 --- a/mindspore/core/ir/graph_utils.cc +++ b/mindspore/core/ir/graph_utils.cc @@ -32,6 +32,7 @@ #include "ir/func_graph.h" #include "utils/log_adapter.h" #include "utils/ms_context.h" +#include "mindspore/ccsrc/utils/utils.h" namespace mindspore { std::vector TopoSort(const AnfNodePtr &root, const SuccFunc &succ, const IncludeFunc &include) { @@ -170,8 +171,16 @@ std::vector BroadFirstSearchGraphUsed(FuncGraphPtr root) { static void PushSuccessors(const CNodePtr &cnode, std::vector *vecs) { auto &inputs = cnode->inputs(); vecs->reserve(vecs->size() + inputs.size()); - // To keep evaluate order from left to right, we push inputs in reversed order. - vecs->insert(vecs->end(), inputs.rbegin(), inputs.rend()); + + // To keep sort order from left to right in default, if kAttrTopoSortRhsFirst not set. + auto attr_sort_rhs_first = cnode->GetAttr(kAttrTopoSortRhsFirst); + auto sort_rhs_first = + attr_sort_rhs_first != nullptr && attr_sort_rhs_first->isa() && GetValue(attr_sort_rhs_first); + if (sort_rhs_first) { + vecs->insert(vecs->end(), inputs.cbegin(), inputs.cend()); + } else { + vecs->insert(vecs->end(), inputs.crbegin(), inputs.crend()); + } } std::vector SuccDeeper(const AnfNodePtr &node) { diff --git a/tests/st/ops/cpu/test_dot_op.py b/tests/st/ops/cpu/test_dot_op.py index c2855baa9d..1efb5a8200 100644 --- a/tests/st/ops/cpu/test_dot_op.py +++ b/tests/st/ops/cpu/test_dot_op.py @@ -174,8 +174,8 @@ def test_dot_008(): network = NetDot() try: network(x2_tensor, x1_tensor) - except IndexError as e: - assert IndexError == type(e) + except ValueError as e: + assert ValueError == type(e) @pytest.mark.level0 diff --git a/tests/ut/python/nn/test_nn_embedding.py b/tests/ut/python/nn/test_nn_embedding.py index 4e583a1249..8a8dcef841 100755 --- a/tests/ut/python/nn/test_nn_embedding.py +++ b/tests/ut/python/nn/test_nn_embedding.py @@ -82,7 +82,7 @@ def test_check_multifield_embedding_false_type_field_id(): @non_graph_engine def test_check_multifield_embedding_false_input_shape(): - with pytest.raises(IndexError): + with pytest.raises(ValueError): compile_multi_field_embedding((8,), (8, 200), (8, 200), dtype.int16, dtype.float32, dtype.int16) diff --git a/tests/ut/python/nn/test_ssim.py b/tests/ut/python/nn/test_ssim.py index 8b7e441014..8e6866cb59 100644 --- a/tests/ut/python/nn/test_ssim.py +++ b/tests/ut/python/nn/test_ssim.py @@ -84,7 +84,7 @@ def test_ssim_different_shape(): img1 = Tensor(np.random.random(shape_1)) img2 = Tensor(np.random.random(shape_2)) net = SSIMNet() - with pytest.raises(ValueError): + with pytest.raises(TypeError): _executor.compile(net, img1, img2) @@ -108,9 +108,9 @@ def test_ssim_invalid_5d_input(): invalid_img2 = Tensor(np.random.random(invalid_shape)) net = SSIMNet() - with pytest.raises(ValueError): + with pytest.raises(TypeError): _executor.compile(net, invalid_img1, img2) - with pytest.raises(ValueError): + with pytest.raises(TypeError): _executor.compile(net, img1, invalid_img2) - with pytest.raises(ValueError): + with pytest.raises(TypeError): _executor.compile(net, invalid_img1, invalid_img2)