Merged PR 1970: Refactor code

1. Remove "using namespace onnx" from header files
2.  onnx_test_runner: combine input/output data into a single file for each dataset.
3.  onnx_test_runner: change the way how finished event was sent, to fix a crash on exit bug.
4.  cmake: Cleanup include_directories, change it to target_include_directories, so that it only make effect on special targets.
5. cmake: remove lotusIR_graph_obj and lotus_common_obj target
6. cmake: perfer to use prebuilt zlib library
7. cloudtest: disable "LotusModelTest.resnet50.debug" and "LotusModelTest.vgg19.debug". They run too long.
8. cloudtest: add 6 models. coreml_SVC_sklearn_load_wine,coreml_SVC_sklearn_load_breast_cancer,coreml_SVC_OpenML_1464_blood_transfusion,coreml_SVC_OpenML_312_scene,coreml_SVR_sklearn_load_boston,coreml_SVR_sklearn_load_diabetes
9. logging: move the implementation of OStreamSink to a ".cc" file, so that users of it has no dependency on the "date" library.

Related work items: #137

Author: snnn

cmake/CMakeLists.txt

@@ -91,6 +91,10 @@ if(WIN32)
   #Always enable exception handling, even for Windows ARM
   SET (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /EHsc")
 endif()
+
+# External dependencies
+list(APPEND CMAKE_MODULE_PATH ${PROJECT_SOURCE_DIR}/external)
+
 #Here we support three build mode:
 #1. (recommended)lotus_USE_PREBUILT_PB is set (ONNX_CUSTOM_PROTOC_EXECUTABLE should also be set)
 #   We will not build protobuf, will use a prebuilt binary instead. This mode can also support cross-compiling
@@ -108,9 +112,20 @@ if(lotus_USE_PREBUILT_PB)
     _PROTOBUF_INSTALL_PREFIX
     ${_PROTOBUF_INSTALL_PREFIX}/..
     REALPATH)
-  include(${_PROTOBUF_INSTALL_PREFIX}/cmake/protobuf-config.cmake)
+  if(WIN32)
+    include(${_PROTOBUF_INSTALL_PREFIX}/cmake/protobuf-config.cmake)
+  else()
+    include(${_PROTOBUF_INSTALL_PREFIX}/lib64/cmake/protobuf/protobuf-config.cmake)
+  endif()
   include(protobuf_function.cmake)
 else()
+  # zlib
+  find_package(ZLIB)
+  if (NOT ZLIB_FOUND)
+    include(zlib)
+    set_target_properties(zlib_copy_headers_to_destination PROPERTIES FOLDER "External/zlib")
+    set_target_properties(zlib_create_destination_dir PROPERTIES FOLDER "External/zlib")
+  endif()
   # use protobuf as a submodule
   add_subdirectory(${PROJECT_SOURCE_DIR}/external/protobuf/cmake EXCLUDE_FROM_ALL)
   set_target_properties(libprotobuf PROPERTIES FOLDER "External/Protobuf")
@@ -125,8 +140,6 @@ endif()
 get_target_property(PROTOBUF_INCLUDE_DIRS protobuf::libprotobuf
     INTERFACE_INCLUDE_DIRECTORIES)
 
-set(googletest_INCLUDE_DIRS ${PROJECT_SOURCE_DIR}/external/googletest/googletest/include ${PROJECT_SOURCE_DIR}/external/googletest/googlemock/include)
-
 if (lotus_USE_CUDA AND "${lotus_CUDNN_HOME}" STREQUAL "")
   message(FATAL_ERROR "lotus_CUDNN_HOME required for lotus_USE_CUDA")
 endif()
@@ -155,10 +168,6 @@ if (lotus_SPLIT_UNIT_TEST_PROJECTS)
   set(SingleUnitTestProject OFF)
 endif()
 
-
-
-# External dependencies
-list(APPEND CMAKE_MODULE_PATH ${PROJECT_SOURCE_DIR}/external)
 set(LOTUS_ROOT ${PROJECT_SOURCE_DIR}/../lotus)
 
 include(date)
@@ -174,11 +183,6 @@ include(eigen)
 set(lotus_EXTERNAL_DEPENDENCIES eigen)
 set(lotus_EXTERNAL_LIBRARIES protobuf::libprotobuf)
 
-# zlib
-include(zlib)
-set_target_properties(zlib_copy_headers_to_destination PROPERTIES FOLDER "External/zlib")
-set_target_properties(zlib_create_destination_dir PROPERTIES FOLDER "External/zlib")
-
 # gtest and gmock
 add_subdirectory(${PROJECT_SOURCE_DIR}/external/googletest EXCLUDE_FROM_ALL)
 set_target_properties(gmock PROPERTIES FOLDER "External/GTest")
@@ -247,12 +251,8 @@ endif()
 
 include_directories(
     ${LOTUS_ROOT}
-    ${PROJECT_SOURCE_DIR}/external/onnx
-    ${CMAKE_CURRENT_BINARY_DIR}/external/onnx
     # External dependencies.
-    ${PROTOBUF_INCLUDE_DIRS}
     ${eigen_INCLUDE_DIRS}
-    ${date_INCLUDE_DIR}
     ${gsl_INCLUDE_DIR}
 )
 

cmake/lotusIR_graph.cmake

@@ -7,36 +7,15 @@ file(GLOB_RECURSE lotusIR_defs_src
     "${LOTUS_ROOT}/core/defs/*.cc"
 )
 
-add_library(lotusIR_graph_obj OBJECT ${lotusIR_graph_src} ${lotusIR_defs_src})
-add_dependencies(lotusIR_graph_obj onnx gsl)
+add_library(lotusIR_graph ${lotusIR_graph_src} ${lotusIR_defs_src})
+add_dependencies(lotusIR_graph onnx_proto gsl)
+target_include_directories(lotusIR_graph PUBLIC $<TARGET_PROPERTY:onnx,INTERFACE_INCLUDE_DIRECTORIES>  $<TARGET_PROPERTY:protobuf::libprotobuf,INTERFACE_INCLUDE_DIRECTORIES>)
 
-set_target_properties(lotusIR_graph_obj PROPERTIES FOLDER "Lotus")
-set_target_properties(lotusIR_graph_obj PROPERTIES LINKER_LANGUAGE CXX)
-
-source_group(TREE ${LOTUS_ROOT}/core FILES ${lotusIR_graph_src} ${lotusIR_defs_src})
-
-if (WIN32)
-    set(lotusIR_graph_obj_static_library_flags
-        -IGNORE:4221 # LNK4221: This object file does not define any previously undefined public symbols, so it will not be used by any link operation that consumes this library
-    )
-    
-    set_target_properties(lotusIR_graph_obj PROPERTIES
-        STATIC_LIBRARY_FLAGS "${lotusIR_graph_obj_static_library_flags}")
-    
-    target_compile_options(lotusIR_graph_obj PRIVATE
-        /EHsc   # exception handling - C++ may throw, extern "C" will not
-    )
-
-    # Add Code Analysis properties to enable C++ Core checks. Have to do it via a props file include. 
-    set_target_properties(lotusIR_graph_obj PROPERTIES VS_USER_PROPS ${PROJECT_SOURCE_DIR}/EnableVisualStudioCodeAnalysis.props)
-endif()
-
-
-
-add_library(lotusIR_graph $<TARGET_OBJECTS:lotusIR_graph_obj>)
-target_link_libraries(lotusIR_graph PUBLIC lotus_common onnx)
 
 set_target_properties(lotusIR_graph PROPERTIES FOLDER "Lotus")
+set_target_properties(lotusIR_graph PROPERTIES LINKER_LANGUAGE CXX)
+
+source_group(TREE ${LOTUS_ROOT}/core FILES ${lotusIR_graph_src} ${lotusIR_defs_src})
 
 if (WIN32)
     set(lotusIR_graph_static_library_flags
@@ -49,4 +28,7 @@ if (WIN32)
     target_compile_options(lotusIR_graph PRIVATE
         /EHsc   # exception handling - C++ may throw, extern "C" will not
     )
+
+    # Add Code Analysis properties to enable C++ Core checks. Have to do it via a props file include. 
+    set_target_properties(lotusIR_graph PROPERTIES VS_USER_PROPS ${PROJECT_SOURCE_DIR}/EnableVisualStudioCodeAnalysis.props)
 endif()

cmake/lotus_common.cmake

@@ -32,40 +32,17 @@ file(GLOB lotus_common_src ${lotus_common_src_patterns})
 
 source_group(TREE ${LOTUS_ROOT}/core FILES ${lotus_common_src})
 
-add_library(lotus_common_obj OBJECT ${lotus_common_src})
-
+add_library(lotus_common ${lotus_common_src})
+target_include_directories(lotus_common PRIVATE ${date_INCLUDE_DIR})
 # logging uses date. threadpool uses eigen
-add_dependencies(lotus_common_obj date eigen gsl)
+add_dependencies(lotus_common date eigen gsl)
 
-set_target_properties(lotus_common_obj PROPERTIES LINKER_LANGUAGE CXX)
-set_target_properties(lotus_common_obj PROPERTIES FOLDER "Lotus")
+set_target_properties(lotus_common PROPERTIES LINKER_LANGUAGE CXX)
+set_target_properties(lotus_common PROPERTIES FOLDER "Lotus")
 
 if(WIN32)
     # Add Code Analysis properties to enable C++ Core checks. Have to do it via a props file include. 
-    set_target_properties(lotus_common_obj PROPERTIES VS_USER_PROPS ${PROJECT_SOURCE_DIR}/EnableVisualStudioCodeAnalysis.props)
+    set_target_properties(lotus_common PROPERTIES VS_USER_PROPS ${PROJECT_SOURCE_DIR}/EnableVisualStudioCodeAnalysis.props)
 endif()
 
-# add library that tests can link against
-add_library(lotus_common 
-            $<TARGET_OBJECTS:lotus_common_obj>)            
-
-set_target_properties(lotus_common PROPERTIES FOLDER "Lotus")
-
-if (WIN32)
-    target_compile_definitions(lotus_common PRIVATE
-        _SCL_SECURE_NO_WARNINGS
-    )
-
-    set(lotus_common_static_library_flags
-        -IGNORE:4221 # LNK4221: This object file does not define any previously undefined public symbols, so it will not be used by any link operation that consumes this library
-    )
-    
-    set_target_properties(lotus_common PROPERTIES
-        STATIC_LIBRARY_FLAGS "${lotus_common_static_library_flags}"
-    )
-    
-    target_compile_options(lotus_common PRIVATE
-        /EHsc   # exception handling - C++ may throw, extern "C" will not
-    )
-endif()
 

cmake/lotus_framework.cmake

@@ -11,6 +11,7 @@ endif()
 source_group(TREE ${LOTUS_ROOT}/core FILES ${lotus_framework_srcs})
 
 add_library(lotus_framework_obj OBJECT ${lotus_framework_srcs})
+target_include_directories(lotus_framework_obj PUBLIC $<TARGET_PROPERTY:onnx,INTERFACE_INCLUDE_DIRECTORIES> PUBLIC $<TARGET_PROPERTY:protobuf::libprotobuf,INTERFACE_INCLUDE_DIRECTORIES>)
 set_target_properties(lotus_framework_obj PROPERTIES FOLDER "Lotus")
 set_target_properties(lotus_framework_obj PROPERTIES LINKER_LANGUAGE CUDA)
 # need onnx to build to create headers that this project includes
@@ -37,7 +38,7 @@ file(GLOB_RECURSE lotus_util_srcs
 source_group(TREE ${LOTUS_ROOT}/core FILES ${lotus_util_srcs})
 
 add_library(lotus_util_obj OBJECT ${lotus_util_srcs})
-
+target_include_directories(lotus_util_obj PUBLIC $<TARGET_PROPERTY:onnx,INTERFACE_INCLUDE_DIRECTORIES> PUBLIC $<TARGET_PROPERTY:protobuf::libprotobuf,INTERFACE_INCLUDE_DIRECTORIES>)
 set_target_properties(lotus_util_obj PROPERTIES LINKER_LANGUAGE CXX)
 set_target_properties(lotus_util_obj PROPERTIES FOLDER "Lotus")
 add_dependencies(lotus_util_obj onnx gsl ${lotus_EXTERNAL_DEPENDENCIES})

cmake/lotus_providers.cmake

@@ -11,6 +11,7 @@ file(GLOB lotus_providers_common_srcs
 source_group(TREE ${LOTUS_ROOT}/core FILES ${lotus_providers_common_srcs} ${lotus_providers_srcs})
 
 add_library(lotus_providers_obj OBJECT ${lotus_providers_common_srcs} ${lotus_providers_srcs})
+target_include_directories(lotus_providers_obj PRIVATE $<TARGET_PROPERTY:onnx,INTERFACE_INCLUDE_DIRECTORIES> $<TARGET_PROPERTY:protobuf::libprotobuf,INTERFACE_INCLUDE_DIRECTORIES>)
 
 add_dependencies(lotus_providers_obj eigen gsl onnx)
 
@@ -40,7 +41,8 @@ if (lotus_USE_CUDA)
     add_library(lotus_providers_cuda_cc_obj OBJECT ${lotus_providers_cuda_cc_srcs})
     add_dependencies(lotus_providers_cuda_cc_obj eigen gsl onnx)
     set_target_properties(lotus_providers_cuda_cc_obj PROPERTIES FOLDER "Lotus")
-    
+    target_include_directories(lotus_providers_cuda_cc_obj PRIVATE $<TARGET_PROPERTY:onnx,INTERFACE_INCLUDE_DIRECTORIES> $<TARGET_PROPERTY:protobuf::libprotobuf,INTERFACE_INCLUDE_DIRECTORIES>)
+
     add_library(lotus_providers_cuda $<TARGET_OBJECTS:lotus_providers_cuda_cc_obj> ${lotus_providers_cuda_cu_srcs})
     set_target_properties(lotus_providers_cuda PROPERTIES LINKER_LANGUAGE CUDA)
     set_target_properties(lotus_providers_cuda PROPERTIES FOLDER "Lotus")

cmake/lotus_unittests.cmake

@@ -19,7 +19,7 @@ function(AddTest)
   endif(_UT_DEPENDS)
 
   target_link_libraries(${_UT_TARGET} ${_UT_LIBS} ${CMAKE_THREAD_LIBS_INIT} ${lotus_EXTERNAL_LIBRARIES})
-  
+  target_include_directories(${_UT_TARGET} PRIVATE ${date_INCLUDE_DIR})
   if (WIN32)
     #It's cmake bug, cannot add this compile option for cuda compiler
     #(https://gitlab.kitware.com/cmake/cmake/issues/17535)
@@ -260,9 +260,10 @@ endif()
 
 add_library(onnx_test_runner_common ${onnx_test_runner_common_srcs})
 add_dependencies(onnx_test_runner_common lotus_providers lotus_framework lotusIR_graph onnx)
+target_include_directories(onnx_test_runner_common PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/external/onnx/onnx $<TARGET_PROPERTY:onnx,INTERFACE_INCLUDE_DIRECTORIES> $<TARGET_PROPERTY:protobuf::libprotobuf,INTERFACE_INCLUDE_DIRECTORIES>)
 set_target_properties(onnx_test_runner_common PROPERTIES FOLDER "LotusTest")
 
-lotus_protobuf_generate(APPEND_PATH TARGET onnx_test_runner_common)
+lotus_protobuf_generate(APPEND_PATH IMPORT_DIRS ${PROJECT_SOURCE_DIR}/external/onnx/onnx TARGET onnx_test_runner_common)
 
 set(onnx_test_libs 
     ${FS_STDLIB}
@@ -299,7 +300,6 @@ if (lotus_USE_OPENBLAS)
 endif()
 
 add_executable(onnx_test_runner ${onnx_test_runner_src_dir}/main.cc)
-target_include_directories(onnx_test_runner PUBLIC ${lotusIR_graph_header})
 target_link_libraries(onnx_test_runner PRIVATE onnx_test_runner_common ${GETOPT_LIB} ${onnx_test_libs})
 set_target_properties(onnx_test_runner PROPERTIES FOLDER "LotusTest")
 
@@ -311,6 +311,7 @@ if(WIN32)
   if(NOT ${CMAKE_GENERATOR_PLATFORM} MATCHES "ARM")
     add_library(onnx_test_runner_vstest SHARED ${onnx_test_runner_src_dir}/vstest_logger.cc ${onnx_test_runner_src_dir}/vstest_main.cc)
     target_compile_options(onnx_test_runner_vstest PRIVATE ${DISABLED_WARNINGS_FOR_PROTOBUF})
+    target_include_directories(onnx_test_runner_vstest PRIVATE ${date_INCLUDE_DIR}) 
     target_link_libraries(onnx_test_runner_vstest ${onnx_test_libs} onnx_test_runner_common)
     set_target_properties(onnx_test_runner_vstest PROPERTIES FOLDER "LotusTest")
   endif()

lotus/core/common/logging/sinks/ostream_sink.cc

@@ -0,0 +1,30 @@
+#include "ostream_sink.h"
+#include "date/date.h"
+
+namespace Lotus {
+namespace Logging {
+
+void OStreamSink::SendImpl(const Timestamp &timestamp, const std::string &logger_id, const Capture &message) {
+  // operator for formatting of timestamp in ISO8601 format including microseconds
+  using date::operator<<;
+
+  // Two options as there may be multiple calls attempting to write to the same sink at once:
+  // 1) Use mutex to synchronize access to the stream.
+  // 2) Create the message in an ostringstream and output in one call.
+  //
+  // Going with #2 as it should scale better at the cost of creating the message in memory first
+  // before sending to the stream.
+
+  std::ostringstream msg;
+
+  msg << timestamp << " [" << message.SeverityPrefix() << ":" << message.Category() << ":" << logger_id << ", "
+      << message.Location().ToString() << "] " << message.Message();
+
+  (*stream_) << msg.str() << "\n";
+
+  if (flush_) {
+    stream_->flush();
+  }
+}
+}  // namespace Logging
+}  // namespace Lotus

lotus/core/common/logging/sinks/ostream_sink.h

@@ -4,8 +4,6 @@
 #include <sstream>
 #include <string>
 
-#include "date/date.h"
-
 #include "core/common/logging/capture.h"
 #include "core/common/logging/isink.h"
 
@@ -22,28 +20,7 @@ class OStreamSink : public ISink {
   }
 
  public:
-  void SendImpl(const Timestamp &timestamp, const std::string &logger_id, const Capture &message) override {
-    // operator for formatting of timestamp in ISO8601 format including microseconds
-    using date::operator<<;
-
-    // Two options as there may be multiple calls attempting to write to the same sink at once:
-    // 1) Use mutex to synchronize access to the stream.
-    // 2) Create the message in an ostringstream and output in one call.
-    //
-    // Going with #2 as it should scale better at the cost of creating the message in memory first
-    // before sending to the stream.
-
-    std::ostringstream msg;
-
-    msg << timestamp << " [" << message.SeverityPrefix() << ":" << message.Category() << ":" << logger_id << ", "
-        << message.Location().ToString() << "] " << message.Message();
-
-    (*stream_) << msg.str() << "\n";
-
-    if (flush_) {
-      stream_->flush();
-    }
-  }
+  void SendImpl(const Timestamp &timestamp, const std::string &logger_id, const Capture &message) override;
 
  private:
   std::ostream *stream_;

lotus/core/framework/allocation_planner.cc

@@ -8,7 +8,7 @@
 #include "core/framework/session_state.h"
 #include "core/graph/utils.h"
 #include "core/framework/data_types.h"
-
+using namespace onnx;
 namespace Lotus {
 
 std::ostream& operator<<(std::ostream& out, AllocKind alloc_kind) {
@@ -216,14 +216,14 @@ class PlannerImpl {
 
   MLDataType GetMLDataType(const LotusIR::NodeArg& arg) {
     const DataType ptype = arg.Type();
-    const onnx::TypeProto& type_proto = onnx::Utils::DataTypeUtils::ToTypeProto(ptype);
+    const TypeProto& type_proto = Utils::DataTypeUtils::ToTypeProto(ptype);
     return DataTypeImpl::TypeFromProto(type_proto);
   }
 
   /*! \brief Given a tensor-type, return the size of an element of the tensor.
   */
   size_t GetElementSize(const DataType& tensor_type) {
-    const onnx::TypeProto& type_proto = onnx::Utils::DataTypeUtils::ToTypeProto(tensor_type);
+    const TypeProto& type_proto = Utils::DataTypeUtils::ToTypeProto(tensor_type);
     MLDataType ml_data_type = DataTypeImpl::TypeFromProto(type_proto);
     const TensorTypeBase* tensor_type_base = ml_data_type->AsTensorType();
     LOTUS_ENFORCE(nullptr != tensor_type_base);
@@ -492,7 +492,7 @@ class PlannerImpl {
   bool IsNonTensor(const LotusIR::NodeArg& nodearg) {
     // TODO: unclear why we should go through a string-representation of type
     auto ptype = nodearg.Type();
-    auto& type_proto = onnx::Utils::DataTypeUtils::ToTypeProto(ptype);
+    auto& type_proto = Utils::DataTypeUtils::ToTypeProto(ptype);
     return !type_proto.has_tensor_type();
   }
 

lotus/core/framework/allocation_planner.h

@@ -103,12 +103,12 @@ std::ostream& operator<<(std::ostream& out, std::pair<const SequentialExecutionP
 // to do the planning.
 class ISequentialPlannerContext {
  public:
-  virtual const TensorShapeProto* GetShape(const LotusIR::NodeArg& arg) const = 0;
+  virtual const onnx::TensorShapeProto* GetShape(const LotusIR::NodeArg& arg) const = 0;
 };
 
 class SequentialPlannerContext : public ISequentialPlannerContext {
  public:
-  const TensorShapeProto* GetShape(const LotusIR::NodeArg& arg) const override {
+  const onnx::TensorShapeProto* GetShape(const LotusIR::NodeArg& arg) const override {
     return arg.Shape();
   }
 };