From 2102b8f67cefa288af6f21eb34e68f63631a9b18 Mon Sep 17 00:00:00 2001 From: Scott McKay Date: Mon, 22 Aug 2022 17:20:52 +1000 Subject: [PATCH] Avoid duplicate symbol error between ONNX and ORT for ostream operator<< with TensorShapeProto (#12651) * Remove ostream operator<< definitions for TensorShapeProto and TensorProto as they clash with ONNX definitions in onnx/defs/printer.h/cc. Currently printer.h (unnecessarily) pulls in a number of other ONNX headers which causes naming clashes with parts of ORT. It is also excluded in a minimal build. Instead convert the onnx::TensorShapeProto to onnxruntime::TensorShape so we use the existing ostream operator<< for TensorShape. Make GetTensorShapeFromTensorProto consistent with GetTensorShapeFromTensorShapeProto so both return a TensorShape (as the name implies). --- onnxruntime/core/framework/execution_frame.cc | 6 +-- .../core/framework/session_state_utils.cc | 6 +-- .../core/framework/tensorprotoutils.cc | 17 ++++--- onnxruntime/core/framework/tensorprotoutils.h | 2 +- onnxruntime/core/framework/utils.cc | 44 ------------------- onnxruntime/core/framework/utils.h | 6 --- onnxruntime/core/graph/graph.cc | 29 +++++++----- onnxruntime/core/optimizer/initializer.cc | 3 +- .../transpose_optimizer/optimizer_api_impl.cc | 6 ++- onnxruntime/test/util/compare_ortvalue.cc | 4 +- .../training_api/checkpoint_property.cc | 10 ++--- 11 files changed, 42 insertions(+), 91 deletions(-) diff --git a/onnxruntime/core/framework/execution_frame.cc b/onnxruntime/core/framework/execution_frame.cc index 0dcae9a1c0..7d3a6e06a6 100644 --- a/onnxruntime/core/framework/execution_frame.cc +++ b/onnxruntime/core/framework/execution_frame.cc @@ -825,9 +825,9 @@ void ExecutionFrame::VerifyOutputSizes(int output_index, const Node& node, const } if (!compatible) { - LOGS(session_state_.Logger(), WARNING) << "Expected shape from model of " << *expected_shape - << " does not match actual shape of " << output_shape - << " for output " << output_def->Name(); + LOGS(session_state_.Logger(), WARNING) + << "Expected shape from model of " << utils::GetTensorShapeFromTensorShapeProto(*expected_shape) + << " does not match actual shape of " << output_shape << " for output " << output_def->Name(); } } diff --git a/onnxruntime/core/framework/session_state_utils.cc b/onnxruntime/core/framework/session_state_utils.cc index a322ae2354..0d0015e9bd 100644 --- a/onnxruntime/core/framework/session_state_utils.cc +++ b/onnxruntime/core/framework/session_state_utils.cc @@ -87,9 +87,7 @@ static inline common::Status ExtDataTensorProtoToTensor(const Env& env, // avoided if the Tensor class implements the do-nothing behavior when given a // nullptr for the allocator argument const DataTypeImpl* const type = DataTypeImpl::TensorTypeFromONNXEnum(tensor_proto.data_type())->GetElementType(); - std::vector tensor_shape_vec = utils::GetTensorShapeFromTensorProto(tensor_proto); - TensorShape tensor_shape{tensor_shape_vec}; - + TensorShape tensor_shape = utils::GetTensorShapeFromTensorProto(tensor_proto); tensor = Tensor(type, tensor_shape, ext_data_buf, OrtMemoryInfo(CPU, OrtAllocatorType::OrtDeviceAllocator)); return common::Status::OK(); @@ -106,7 +104,7 @@ static common::Status DeserializeTensorProto(const Env& env, const std::basic_st } // Get shape and type of the tensor, and allocate the empty tensor - TensorShape tensor_shape{utils::GetTensorShapeFromTensorProto(tensor_proto)}; + TensorShape tensor_shape = utils::GetTensorShapeFromTensorProto(tensor_proto); const DataTypeImpl* const type = DataTypeImpl::TensorTypeFromONNXEnum(tensor_proto.data_type())->GetElementType(); std::unique_ptr p_tensor; if (m != nullptr) { diff --git a/onnxruntime/core/framework/tensorprotoutils.cc b/onnxruntime/core/framework/tensorprotoutils.cc index ce384545b8..02a5dcb707 100644 --- a/onnxruntime/core/framework/tensorprotoutils.cc +++ b/onnxruntime/core/framework/tensorprotoutils.cc @@ -494,14 +494,14 @@ TensorShape GetTensorShapeFromTensorShapeProto(const ONNX_NAMESPACE::TensorShape return TensorShape(std::move(tensor_shape_vec)); } -std::vector GetTensorShapeFromTensorProto(const ONNX_NAMESPACE::TensorProto& tensor_proto) { +TensorShape GetTensorShapeFromTensorProto(const ONNX_NAMESPACE::TensorProto& tensor_proto) { const auto& dims = tensor_proto.dims(); std::vector tensor_shape_vec(static_cast(dims.size())); for (int i = 0; i < dims.size(); ++i) { tensor_shape_vec[i] = dims[i]; } - return tensor_shape_vec; + return TensorShape(std::move(tensor_shape_vec)); } struct UnInitializeParam { @@ -650,8 +650,8 @@ Status TensorProtoToTensor(const Env& env, const ORTCHAR_T* model_path, const ONNX_NAMESPACE::TensorProto& tensor_proto, Tensor& tensor) { // Validate tensor compatibility - std::vector tensor_shape_vec = GetTensorShapeFromTensorProto(tensor_proto); - if (gsl::make_span(tensor_shape_vec) != tensor.Shape().GetDims()) { + TensorShape tensor_shape = GetTensorShapeFromTensorProto(tensor_proto); + if (tensor_shape != tensor.Shape()) { return Status(common::ONNXRUNTIME, common::INVALID_ARGUMENT, "TensorProtoToTensor() tensor shape mismatch!"); } const DataTypeImpl* const source_type = DataTypeImpl::TensorTypeFromONNXEnum(tensor_proto.data_type())->GetElementType(); @@ -747,7 +747,7 @@ Status TensorProtoToMLValue(const Env& env, const ORTCHAR_T* model_path, } // Note: We permit an empty tensor_shape_vec, and treat it as a scalar (a tensor of size 1). - TensorShape tensor_shape{GetTensorShapeFromTensorProto(tensor_proto)}; + TensorShape tensor_shape = GetTensorShapeFromTensorProto(tensor_proto); const DataTypeImpl* const type = DataTypeImpl::TensorTypeFromONNXEnum(tensor_proto.data_type())->GetElementType(); std::unique_ptr tensorp = std::make_unique(type, tensor_shape, m.GetBuffer(), m.GetAllocInfo()); if (tensorp->SizeInBytes() > m.GetLen()) { @@ -798,7 +798,7 @@ ONNXTensorElementDataType GetTensorElementType(const ONNX_NAMESPACE::TensorProto ONNX_NAMESPACE::TensorProto TensorToTensorProto(const Tensor& tensor, const std::string& tensor_proto_name) { // Given we are using the raw_data field in the protobuf, this will work only for little-endian format. - if constexpr(endian::native != endian::little) { + if constexpr (endian::native != endian::little) { ORT_THROW("Big endian not supported"); } @@ -1146,10 +1146,9 @@ static void SetIndices(gsl::span gathered_indices, auto* ind_dest = reinterpret_cast(raw_indices.data()); size_t dest_index = 0; for (auto src_index : gathered_indices) { - if constexpr(sizeof(T) == sizeof(int8_t)) { + if constexpr (sizeof(T) == sizeof(int8_t)) { ind_dest[dest_index] = static_cast(src_index); - } - else { + } else { auto* dst = ind_dest + dest_index; T v = static_cast(src_index); memcpy(dst, &v, sizeof(T)); diff --git a/onnxruntime/core/framework/tensorprotoutils.h b/onnxruntime/core/framework/tensorprotoutils.h index 1edfcedc3d..6f1b60dd29 100644 --- a/onnxruntime/core/framework/tensorprotoutils.h +++ b/onnxruntime/core/framework/tensorprotoutils.h @@ -36,7 +36,7 @@ namespace utils { TensorShape GetTensorShapeFromTensorShapeProto(const ONNX_NAMESPACE::TensorShapeProto& tensor_shape_proto); -std::vector GetTensorShapeFromTensorProto(const ONNX_NAMESPACE::TensorProto& tensor_proto); +TensorShape GetTensorShapeFromTensorProto(const ONNX_NAMESPACE::TensorProto& tensor_proto); /** * deserialize a TensorProto into a preallocated memory buffer. diff --git a/onnxruntime/core/framework/utils.cc b/onnxruntime/core/framework/utils.cc index 8059de6c84..ff41c506f8 100644 --- a/onnxruntime/core/framework/utils.cc +++ b/onnxruntime/core/framework/utils.cc @@ -27,50 +27,6 @@ #include "contrib_ops/cpu/aten_ops/aten_op_executor.h" #endif -namespace ONNX_NAMESPACE { -std::ostream& operator<<(std::ostream& out, const TensorShapeProto& shape_proto) { - std::string result; - result.reserve(128); - - result.append("{"); - bool first = true; - for (auto& dim : shape_proto.dim()) { - if (!first) { - result.append(","); - } - - if (onnxruntime::utils::HasDimValue(dim)) - result.append(std::to_string(dim.dim_value())); - else if (onnxruntime::utils::HasDimParam(dim)) - result.append(dim.dim_param()); - - first = false; - } - result.append("}"); - - return (out << result); -} - -std::ostream& operator<<(std::ostream& out, const TensorProto& tensor_proto) { - std::string result; - result.reserve(128); - - result.append("{"); - bool first = true; - for (auto& dim : tensor_proto.dims()) { - if (!first) { - result.append(","); - } - - result.append(std::to_string(dim)); - first = false; - } - result.append("}"); - - return (out << result); -} -} // namespace ONNX_NAMESPACE - namespace onnxruntime { namespace utils { void* DefaultAlloc(size_t size) { diff --git a/onnxruntime/core/framework/utils.h b/onnxruntime/core/framework/utils.h index 84c96757bb..ebe599193d 100644 --- a/onnxruntime/core/framework/utils.h +++ b/onnxruntime/core/framework/utils.h @@ -13,12 +13,6 @@ #ifdef ENABLE_TRAINING #include "core/framework/partial_graph_execution_state.h" #endif -namespace ONNX_NAMESPACE { -class TensorShapeProto; -class TensorProto; -std::ostream& operator<<(std::ostream& out, const TensorShapeProto& shape_proto); -std::ostream& operator<<(std::ostream& out, const TensorProto& tensor_proto); -} // namespace ONNX_NAMESPACE namespace onnxruntime { class ExecutionProviders; diff --git a/onnxruntime/core/graph/graph.cc b/onnxruntime/core/graph/graph.cc index 03078dca09..4b8939f237 100644 --- a/onnxruntime/core/graph/graph.cc +++ b/onnxruntime/core/graph/graph.cc @@ -104,7 +104,8 @@ static Status MergeShapeInfo(const std::string& output_name, // target.shape() was empty. 'assert' just in case that ever changes. assert(utils::HasShape(source) && utils::HasShape(target)); LOGS(logger, WARNING) << "Error merging shape info for output. '" << output_name - << "' source:" << utils::GetShape(source) << " target:" << utils::GetShape(target) + << "' source:" << utils::GetTensorShapeFromTensorShapeProto(utils::GetShape(source)) + << " target:" << utils::GetTensorShapeFromTensorShapeProto(utils::GetShape(target)) << ". Falling back to lenient merge."; if (utils::HasTensorType(source)) { ONNX_NAMESPACE::UnionShapeInfo(utils::GetShape(source), *target.mutable_tensor_type()); @@ -2413,19 +2414,25 @@ common::Status Graph::TypeCheckInputsAndInitializers() { node_arg->SetShape(inferred_shape); } } else { + bool invalid = false; + if (p_existing_shape->dim_size() != tensor_proto->dims_size()) { - return ORT_MAKE_STATUS(ONNXRUNTIME, FAIL, - "Type Error: Shape of initializer ", name, " does not match. ", - *p_existing_shape, " != ", *tensor_proto); + invalid = true; + } else { + for (int i = 0; i < p_existing_shape->dim_size(); ++i) { + auto& d = p_existing_shape->dim(i); + if (utils::HasDimValue(d) && (d.dim_value() != tensor_proto->dims(i))) { + invalid = true; + break; + } + } } - for (int i = 0; i < p_existing_shape->dim_size(); ++i) { - auto& d = p_existing_shape->dim(i); - if (utils::HasDimValue(d) && (d.dim_value() != tensor_proto->dims(i))) { - return ORT_MAKE_STATUS(ONNXRUNTIME, FAIL, - "Type Error: Shape of initializer ", name, " does not match. ", - *p_existing_shape, " != ", *tensor_proto); - } + if (invalid) { + return ORT_MAKE_STATUS(ONNXRUNTIME, FAIL, + "Type Error: Shape of initializer ", name, " does not match. ", + utils::GetTensorShapeFromTensorShapeProto(*p_existing_shape), " != ", + utils::GetTensorShapeFromTensorProto(*tensor_proto)); } } } diff --git a/onnxruntime/core/optimizer/initializer.cc b/onnxruntime/core/optimizer/initializer.cc index 1128fadcd0..726c1ff318 100644 --- a/onnxruntime/core/optimizer/initializer.cc +++ b/onnxruntime/core/optimizer/initializer.cc @@ -36,8 +36,7 @@ Initializer::Initializer(const ONNX_NAMESPACE::TensorProto& tensor_proto, const name_ = tensor_proto.name(); } - auto proto_dims = utils::GetTensorShapeFromTensorProto(tensor_proto); - TensorShape proto_shape(proto_dims); + auto proto_shape = utils::GetTensorShapeFromTensorProto(tensor_proto); // This must be pre-allocated Tensor w(DataTypeImpl::TensorTypeFromONNXEnum(proto_data_type)->GetElementType(), proto_shape, std::make_shared()); diff --git a/onnxruntime/core/optimizer/transpose_optimizer/optimizer_api_impl.cc b/onnxruntime/core/optimizer/transpose_optimizer/optimizer_api_impl.cc index 4c882ff1c3..a002d108f8 100644 --- a/onnxruntime/core/optimizer/transpose_optimizer/optimizer_api_impl.cc +++ b/onnxruntime/core/optimizer/transpose_optimizer/optimizer_api_impl.cc @@ -244,11 +244,13 @@ void ApiValueInfo::UnsqueezeDims(const std::vector& axes) { // std::vector ApiTensor::Shape() const { - return utils::GetTensorShapeFromTensorProto(tensor_proto_); + TensorShape shape = utils::GetTensorShapeFromTensorProto(tensor_proto_); + const auto dims = shape.GetDims(); + return std::vector{dims.cbegin(), dims.cend()}; } size_t ApiTensor::NumElements() const { - int64_t size = TensorShape(utils::GetTensorShapeFromTensorProto(tensor_proto_)).Size(); + int64_t size = utils::GetTensorShapeFromTensorProto(tensor_proto_).Size(); ORT_ENFORCE(size >= 0, "Failed to get size of TensorProto"); return gsl::narrow_cast(size); } diff --git a/onnxruntime/test/util/compare_ortvalue.cc b/onnxruntime/test/util/compare_ortvalue.cc index 8d41f15df9..0baf953574 100644 --- a/onnxruntime/test/util/compare_ortvalue.cc +++ b/onnxruntime/test/util/compare_ortvalue.cc @@ -423,7 +423,7 @@ std::pair CompareOrtValue(const OrtValue& o, const static std::pair CompareTensorOrtValueAndTensorTypeProto(const ONNX_NAMESPACE::TypeProto_Tensor& t, const Ort::Value& o) { // below code doesn't work - //if (((TensorTypeBase*)o.Type())->GetElementType() != DataTypeImpl::ElementTypeFromProto(t.elem_type())) { + // if (((TensorTypeBase*)o.Type())->GetElementType() != DataTypeImpl::ElementTypeFromProto(t.elem_type())) { // return COMPARE_RESULT::TYPE_MISMATCH; //} @@ -445,7 +445,7 @@ static std::pair CompareTensorOrtValueAndTensorType if (tensor_shape_proto.dim_size() == 0) { oss << "(unknown)"; } else { - oss << tensor_shape_proto; + oss << utils::GetTensorShapeFromTensorShapeProto(tensor_shape_proto); } oss << "', real output is "; VectorToString(shape, oss); diff --git a/orttraining/orttraining/training_api/checkpoint_property.cc b/orttraining/orttraining/training_api/checkpoint_property.cc index 59556923b8..6ce573ae3c 100644 --- a/orttraining/orttraining/training_api/checkpoint_property.cc +++ b/orttraining/orttraining/training_api/checkpoint_property.cc @@ -18,16 +18,12 @@ template void ParsePropertyFromTensorProto(const ONNX_NAMESPACE::TensorProto& tensor_proto, std::string& name, PropertyDataType& value) { - std::vector tensor_shape_vec = utils::GetTensorShapeFromTensorProto(tensor_proto); - int64_t expected_num_elements = 1; - for (auto& d : tensor_shape_vec) { - expected_num_elements *= d; - } - ORT_ENFORCE(expected_num_elements == 1, "Only scalar value support for checkpoint property."); + TensorShape tensor_shape = utils::GetTensorShapeFromTensorProto(tensor_proto); + ORT_ENFORCE(tensor_shape.Size() == 1, "Only scalar value is supported for checkpoint property."); Path model_path; InlinedVector data_vector(1); T* p = data_vector.data(); - ORT_THROW_IF_ERROR(utils::UnpackTensor(tensor_proto, model_path, p, expected_num_elements)); + ORT_THROW_IF_ERROR(utils::UnpackTensor(tensor_proto, model_path, p, 1)); name = tensor_proto.name(); value = data_vector[0]; }