From 796ddeb2cba0f67ac723426906aed1f0163eb353 Mon Sep 17 00:00:00 2001 From: Scott McKay Date: Tue, 8 Sep 2020 17:43:42 +1000 Subject: [PATCH] Remove serialization of outer scope value info in ORT format model (#5077) * Remove serialization of outer scope node arg info in ORT format model. We don't currently need it in a minimal build as only SessionState calls Graph::IsConstantInitializer and it doesn't search outer scope. If we do need it in the future the information can be calculated at runtime (small binary size cost to do so). Motivation: ORT format model was 32% bigger for a BERT model with multiple levels of subgraph and a lot of nodes due to this. Size is about 5% larger of the original ONNX model with the change. ORT format has type/shape info for all nodes, and this model has 2000 nodes so this seems reasonable. Added example code to dump ORT format model to json. Fixed misc bug in python test script around handling float and non-float expected output. --- include/onnxruntime/core/graph/graph.h | 7 ++++ onnxruntime/core/flatbuffers/ort.fbs | 2 -- onnxruntime/core/flatbuffers/ort.fbs.h | 23 +++--------- .../core/framework/session_state_utils.cc | 2 ++ onnxruntime/core/graph/graph.cc | 22 ------------ .../test/framework/ort_model_only_test.cc | 35 ++++++++++++++++--- tools/python/ort_test_dir_utils.py | 2 +- 7 files changed, 45 insertions(+), 48 deletions(-) diff --git a/include/onnxruntime/core/graph/graph.h b/include/onnxruntime/core/graph/graph.h index d4a910428a..94eb55d06d 100644 --- a/include/onnxruntime/core/graph/graph.h +++ b/include/onnxruntime/core/graph/graph.h @@ -596,6 +596,7 @@ class Graph { cannot be overridden at runtime. If the initializer is not found or is not constant, a nullptr is returned. @param check_outer_scope If true and the graph is a subgraph, check ancestor graph/s for 'name' if not found in 'graph'. + @remarks check_outer_scope of true is not supported in a minimal build */ const ONNX_NAMESPACE::TensorProto* GetConstantInitializer(const std::string& name, bool check_outer_scope) const; @@ -992,7 +993,13 @@ class Graph { /** Returns true if the name is for a value that is coming from outer scope */ bool IsOuterScopeValue(const std::string& name) const { +#if !defined(ORT_MINIMAL_BUILD) return resolve_context_.outer_scope_node_args.find(name) != resolve_context_.outer_scope_node_args.cend(); +#else + // we shouldn't have code that calls this in a minimal build + ORT_UNUSED_PARAMETER(name); + ORT_THROW("Internal error. Outer scope value lookup is not currently supported in a minimal build."); +#endif } #if !defined(ORT_MINIMAL_BUILD) diff --git a/onnxruntime/core/flatbuffers/ort.fbs b/onnxruntime/core/flatbuffers/ort.fbs index f18620f3f4..a8d2e08e5f 100644 --- a/onnxruntime/core/flatbuffers/ort.fbs +++ b/onnxruntime/core/flatbuffers/ort.fbs @@ -185,8 +185,6 @@ table Graph{ inputs:[string]; outputs:[string]; - - outer_scope_node_args:[string]; } table Model { diff --git a/onnxruntime/core/flatbuffers/ort.fbs.h b/onnxruntime/core/flatbuffers/ort.fbs.h index d64d2d3c6f..0a553f47ec 100644 --- a/onnxruntime/core/flatbuffers/ort.fbs.h +++ b/onnxruntime/core/flatbuffers/ort.fbs.h @@ -1567,8 +1567,7 @@ struct Graph FLATBUFFERS_FINAL_CLASS : private flatbuffers::Table { VT_MAX_NODE_INDEX = 10, VT_NODE_EDGES = 12, VT_INPUTS = 14, - VT_OUTPUTS = 16, - VT_OUTER_SCOPE_NODE_ARGS = 18 + VT_OUTPUTS = 16 }; const flatbuffers::Vector> *initializers() const { return GetPointer> *>(VT_INITIALIZERS); @@ -1591,9 +1590,6 @@ struct Graph FLATBUFFERS_FINAL_CLASS : private flatbuffers::Table { const flatbuffers::Vector> *outputs() const { return GetPointer> *>(VT_OUTPUTS); } - const flatbuffers::Vector> *outer_scope_node_args() const { - return GetPointer> *>(VT_OUTER_SCOPE_NODE_ARGS); - } bool Verify(flatbuffers::Verifier &verifier) const { return VerifyTableStart(verifier) && VerifyOffset(verifier, VT_INITIALIZERS) && @@ -1615,9 +1611,6 @@ struct Graph FLATBUFFERS_FINAL_CLASS : private flatbuffers::Table { VerifyOffset(verifier, VT_OUTPUTS) && verifier.VerifyVector(outputs()) && verifier.VerifyVectorOfStrings(outputs()) && - VerifyOffset(verifier, VT_OUTER_SCOPE_NODE_ARGS) && - verifier.VerifyVector(outer_scope_node_args()) && - verifier.VerifyVectorOfStrings(outer_scope_node_args()) && verifier.EndTable(); } }; @@ -1647,9 +1640,6 @@ struct GraphBuilder { void add_outputs(flatbuffers::Offset>> outputs) { fbb_.AddOffset(Graph::VT_OUTPUTS, outputs); } - void add_outer_scope_node_args(flatbuffers::Offset>> outer_scope_node_args) { - fbb_.AddOffset(Graph::VT_OUTER_SCOPE_NODE_ARGS, outer_scope_node_args); - } explicit GraphBuilder(flatbuffers::FlatBufferBuilder &_fbb) : fbb_(_fbb) { start_ = fbb_.StartTable(); @@ -1669,10 +1659,8 @@ inline flatbuffers::Offset CreateGraph( uint32_t max_node_index = 0, flatbuffers::Offset>> node_edges = 0, flatbuffers::Offset>> inputs = 0, - flatbuffers::Offset>> outputs = 0, - flatbuffers::Offset>> outer_scope_node_args = 0) { + flatbuffers::Offset>> outputs = 0) { GraphBuilder builder_(_fbb); - builder_.add_outer_scope_node_args(outer_scope_node_args); builder_.add_outputs(outputs); builder_.add_inputs(inputs); builder_.add_node_edges(node_edges); @@ -1691,15 +1679,13 @@ inline flatbuffers::Offset CreateGraphDirect( uint32_t max_node_index = 0, const std::vector> *node_edges = nullptr, const std::vector> *inputs = nullptr, - const std::vector> *outputs = nullptr, - const std::vector> *outer_scope_node_args = nullptr) { + const std::vector> *outputs = nullptr) { auto initializers__ = initializers ? _fbb.CreateVector>(*initializers) : 0; auto node_args__ = node_args ? _fbb.CreateVector>(*node_args) : 0; auto nodes__ = nodes ? _fbb.CreateVector>(*nodes) : 0; auto node_edges__ = node_edges ? _fbb.CreateVector>(*node_edges) : 0; auto inputs__ = inputs ? _fbb.CreateVector>(*inputs) : 0; auto outputs__ = outputs ? _fbb.CreateVector>(*outputs) : 0; - auto outer_scope_node_args__ = outer_scope_node_args ? _fbb.CreateVector>(*outer_scope_node_args) : 0; return onnxruntime::experimental::fbs::CreateGraph( _fbb, initializers__, @@ -1708,8 +1694,7 @@ inline flatbuffers::Offset CreateGraphDirect( max_node_index, node_edges__, inputs__, - outputs__, - outer_scope_node_args__); + outputs__); } struct Model FLATBUFFERS_FINAL_CLASS : private flatbuffers::Table { diff --git a/onnxruntime/core/framework/session_state_utils.cc b/onnxruntime/core/framework/session_state_utils.cc index 14560e49e0..2ae0b80b04 100644 --- a/onnxruntime/core/framework/session_state_utils.cc +++ b/onnxruntime/core/framework/session_state_utils.cc @@ -145,6 +145,8 @@ common::Status SaveInitializedTensors( return Status(st.Category(), st.Code(), oss.str()); } + // any outer scope value is shadowed by a local value and can't override it. + // due to that check_outer_scope is false bool constant = graph.IsConstantInitializer(name, /* check_outer_scope */ false); ORT_RETURN_IF_ERROR(save_tensor_func(ort_value_index, ort_value, deleter, constant)); diff --git a/onnxruntime/core/graph/graph.cc b/onnxruntime/core/graph/graph.cc index f498277796..7466cd2638 100644 --- a/onnxruntime/core/graph/graph.cc +++ b/onnxruntime/core/graph/graph.cc @@ -1199,9 +1199,6 @@ common::Status Graph::SetOuterScopeNodeArgs(const std::unordered_set outer_scope_node_args_vec(resolve_context_.outer_scope_node_args.size()); - std::copy(resolve_context_.outer_scope_node_args.cbegin(), - resolve_context_.outer_scope_node_args.cend(), - outer_scope_node_args_vec.begin()); - auto outer_scope_node_args = builder.CreateVectorOfStrings(outer_scope_node_args_vec); - std::vector> initializers_data; initializers_data.reserve(name_to_initial_tensor_.size()); for (const auto& pair : name_to_initial_tensor_) { @@ -2763,7 +2751,6 @@ common::Status Graph::SaveToOrtFormat(flatbuffers::FlatBufferBuilder& builder, gb.add_node_edges(node_edges); gb.add_inputs(inputs); gb.add_outputs(outputs); - gb.add_outer_scope_node_args(outer_scope_node_args); fbs_graph = gb.Finish(); return Status::OK(); } @@ -3530,15 +3517,6 @@ common::Status Graph::LoadFromOrtFormat(const onnxruntime::experimental::fbs::Gr ORT_RETURN_IF_ERROR(add_node_args(fbs_graph.outputs(), graph_outputs_)); - auto fbs_outer_scope_node_args = fbs_graph.outer_scope_node_args(); - if (fbs_outer_scope_node_args != nullptr) { - resolve_context_.outer_scope_node_args.reserve(fbs_outer_scope_node_args->size()); - for (const auto node_arg_name : *fbs_outer_scope_node_args) { - ORT_RETURN_IF(nullptr == node_arg_name, "node_arg_name is null. Invalid ORT format model."); - resolve_context_.outer_scope_node_args.insert(node_arg_name->str()); - } - } - return Status::OK(); } diff --git a/onnxruntime/test/framework/ort_model_only_test.cc b/onnxruntime/test/framework/ort_model_only_test.cc index 2495803085..fccd92e4f0 100644 --- a/onnxruntime/test/framework/ort_model_only_test.cc +++ b/onnxruntime/test/framework/ort_model_only_test.cc @@ -9,6 +9,10 @@ #include "test_utils.h" #include "test/util/include/asserts.h" +#include "core/flatbuffers/ort.fbs.h" +#include "flatbuffers/idl.h" +#include "flatbuffers/util.h" + #include "gtest/gtest.h" using namespace std; @@ -44,7 +48,7 @@ struct OrtModelTestInfo { std::vector> configs; }; -void RunOrtModel(const OrtModelTestInfo& test_info) { +static void RunOrtModel(const OrtModelTestInfo& test_info) { SessionOptions so; so.session_logid = test_info.logid; for (const auto& config : test_info.configs) @@ -125,8 +129,8 @@ static void CompareValueInfos(const ValueInfoProto& left, const ValueInfoProto& CompareTypeProtos(left.type(), right.type()); } -void CompareGraphAndSessionState(const InferenceSessionGetGraphWrapper& session_object_1, - const InferenceSessionGetGraphWrapper& session_object_2) { +static void CompareGraphAndSessionState(const InferenceSessionGetGraphWrapper& session_object_1, + const InferenceSessionGetGraphWrapper& session_object_2) { const auto& graph_1 = session_object_1.GetGraph(); const auto& graph_2 = session_object_2.GetGraph(); @@ -178,7 +182,7 @@ void CompareGraphAndSessionState(const InferenceSessionGetGraphWrapper& session_ } } -void SaveAndCompareModels(const std::string& onnx_file, const std::basic_string& ort_file) { +static void SaveAndCompareModels(const std::string& onnx_file, const std::basic_string& ort_file) { SessionOptions so; so.session_logid = "SerializeToOrtFormat"; so.optimized_model_filepath = ort_file; @@ -204,10 +208,33 @@ void SaveAndCompareModels(const std::string& onnx_file, const std::basic_string< CompareGraphAndSessionState(session_object, session_object2); } +/* +static void DumpOrtModelAsJson(const std::string& model_uri) { + std::string ort_repo_root("path to your ORT repo root"); + std::string ort_flatbuffers_dir(ort_repo_root + "onnxruntime/core/flatbuffers/"); + std::string schemafile(ort_flatbuffers_dir + "ort.fbs"); + std::string jsonfile; + + ORT_ENFORCE(flatbuffers::LoadFile(schemafile.c_str(), false, &schemafile)); + flatbuffers::Parser parser; + const char* include_directories[] = {ort_flatbuffers_dir.c_str(), nullptr}; + + ORT_ENFORCE(parser.Parse(schemafile.c_str(), include_directories)); + + std::string flatbuffer; + std::string json; + flatbuffers::LoadFile(model_uri.c_str(), true, &flatbuffer); + flatbuffers::GenerateText(parser, flatbuffer.data(), &json); + std::ofstream(model_uri + ".json") << json; +} +*/ + TEST(OrtModelOnlyTests, SerializeToOrtFormat) { const std::basic_string ort_file = ORT_TSTR("ort_github_issue_4031.onnx.ort"); SaveAndCompareModels("testdata/ort_github_issue_4031.onnx", ort_file); + // DumpOrtModelAsJson(ToMBString(ort_file)); + OrtModelTestInfo test_info; test_info.model_filename = ort_file; test_info.logid = "SerializeToOrtFormat"; diff --git a/tools/python/ort_test_dir_utils.py b/tools/python/ort_test_dir_utils.py index f363367029..29ff497e97 100644 --- a/tools/python/ort_test_dir_utils.py +++ b/tools/python/ort_test_dir_utils.py @@ -220,7 +220,7 @@ def run_test_dir(model_or_dir): expected = expected_outputs[output_names[idx]] actual = run_outputs[idx] - if isinstance(expected[0], np.floating): + if expected.dtype.char in np.typecodes['AllFloat']: if not np.isclose(expected, actual, rtol=1.e-3, atol=1.e-3).all(): print(f'Mismatch for {output_names[idx]}:\nExpected:{expected}\nGot:{actual}') failed = True