diff --git a/CMakeLists.txt b/CMakeLists.txt index 07d138cb0..6f0bdf860 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -148,9 +148,6 @@ if (USE_OPENMP) find_package(OpenMP REQUIRED) endif (USE_OPENMP) -# core xgboost -add_subdirectory(${xgboost_SOURCE_DIR}/src) - # dmlc-core msvc_use_static_runtime() add_subdirectory(${xgboost_SOURCE_DIR}/dmlc-core) @@ -169,34 +166,13 @@ endif (MSVC) if (ENABLE_ALL_WARNINGS) target_compile_options(dmlc PRIVATE -Wall -Wextra) endif (ENABLE_ALL_WARNINGS) -target_link_libraries(objxgboost PUBLIC dmlc) # rabit add_subdirectory(rabit) -if (RABIT_MOCK) - target_link_libraries(objxgboost PUBLIC rabit_mock_static) - if (MSVC) - target_compile_options(rabit_mock_static PRIVATE - -D_CRT_SECURE_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE) - endif (MSVC) -else() - target_link_libraries(objxgboost PUBLIC rabit) - if (MSVC) - target_compile_options(rabit PRIVATE - -D_CRT_SECURE_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE) - endif (MSVC) -endif(RABIT_MOCK) -foreach(lib rabit rabit_mock_static) - # Explicitly link dmlc to rabit, so that configured header (build_config.h) - # from dmlc is correctly applied to rabit. - if (TARGET ${lib}) - target_link_libraries(${lib} dmlc ${CMAKE_THREAD_LIBS_INIT}) - if (ENABLE_ALL_WARNINGS) - target_compile_options(${lib} PRIVATE -Wall -Wextra) - endif (ENABLE_ALL_WARNINGS) - endif (TARGET ${lib}) -endforeach() +# core xgboost +add_subdirectory(${xgboost_SOURCE_DIR}/src) +target_link_libraries(objxgboost PUBLIC dmlc) # Exports some R specific definitions and objects if (R_LIB) @@ -220,7 +196,7 @@ endif (USE_CUDA) #-- Hide all C++ symbols if (HIDE_CXX_SYMBOLS) - foreach(target objxgboost xgboost dmlc rabit rabit_mock_static) + foreach(target objxgboost xgboost dmlc) set_target_properties(${target} PROPERTIES CXX_VISIBILITY_PRESET hidden) endforeach() endif (HIDE_CXX_SYMBOLS) @@ -292,7 +268,7 @@ install(DIRECTORY ${xgboost_SOURCE_DIR}/include/xgboost # # https://github.com/dmlc/xgboost/issues/6085 if (BUILD_STATIC_LIB) - set(INSTALL_TARGETS xgboost runxgboost objxgboost rabit dmlc) + set(INSTALL_TARGETS xgboost runxgboost objxgboost dmlc) else (BUILD_STATIC_LIB) set(INSTALL_TARGETS xgboost runxgboost) endif (BUILD_STATIC_LIB) diff --git a/jvm-packages/xgboost4j-spark/src/test/scala/ml/dmlc/xgboost4j/scala/spark/FeatureSizeValidatingSuite.scala b/jvm-packages/xgboost4j-spark/src/test/scala/ml/dmlc/xgboost4j/scala/spark/FeatureSizeValidatingSuite.scala index 0bbfb9f78..7e560827b 100644 --- a/jvm-packages/xgboost4j-spark/src/test/scala/ml/dmlc/xgboost4j/scala/spark/FeatureSizeValidatingSuite.scala +++ b/jvm-packages/xgboost4j-spark/src/test/scala/ml/dmlc/xgboost4j/scala/spark/FeatureSizeValidatingSuite.scala @@ -67,7 +67,7 @@ class FeatureSizeValidatingSuite extends FunSuite with PerTest { (id, lp.label, lp.features) }.toDF("id", "label", "features") val xgb = new XGBoostClassifier(paramMap) - intercept[XGBoostError] { + intercept[Exception] { xgb.fit(repartitioned) } } diff --git a/jvm-packages/xgboost4j/src/native/xgboost4j.cpp b/jvm-packages/xgboost4j/src/native/xgboost4j.cpp index b7461a7de..804e3653c 100644 --- a/jvm-packages/xgboost4j/src/native/xgboost4j.cpp +++ b/jvm-packages/xgboost4j/src/native/xgboost4j.cpp @@ -916,7 +916,7 @@ JNIEXPORT jint JNICALL Java_ml_dmlc_xgboost4j_java_XGBoostJNI_RabitTrackerPrint (JNIEnv *jenv, jclass jcls, jstring jmsg) { std::string str(jenv->GetStringUTFChars(jmsg, 0), jenv->GetStringLength(jmsg)); - RabitTrackerPrint(str.c_str()); + JVM_CHECK_CALL(RabitTrackerPrint(str.c_str())); return 0; } diff --git a/python-package/xgboost/rabit.py b/python-package/xgboost/rabit.py index e8abe74df..a54a46f48 100644 --- a/python-package/xgboost/rabit.py +++ b/python-package/xgboost/rabit.py @@ -6,7 +6,7 @@ import ctypes import pickle import numpy as np -from .core import _LIB, c_str, STRING_TYPES +from .core import _LIB, c_str, STRING_TYPES, _check_call def _init_rabit(): @@ -77,7 +77,7 @@ def tracker_print(msg): msg = str(msg) is_dist = _LIB.RabitIsDistributed() if is_dist != 0: - _LIB.RabitTrackerPrint(c_str(msg)) + _check_call(_LIB.RabitTrackerPrint(c_str(msg))) else: sys.stdout.write(msg) sys.stdout.flush() diff --git a/rabit/CMakeLists.txt b/rabit/CMakeLists.txt index eeb056e83..85fa68730 100644 --- a/rabit/CMakeLists.txt +++ b/rabit/CMakeLists.txt @@ -2,71 +2,16 @@ cmake_minimum_required(VERSION 3.3) find_package(Threads REQUIRED) -add_library(rabit src/allreduce_base.cc src/engine.cc src/c_api.cc) -add_library(rabit_mock_static src/allreduce_base.cc src/engine_mock.cc src/c_api.cc) +set(RABIT_SOURCES + ${CMAKE_CURRENT_LIST_DIR}/src/allreduce_base.cc + ${CMAKE_CURRENT_LIST_DIR}/src/c_api.cc) -target_link_libraries(rabit Threads::Threads dmlc) -target_link_libraries(rabit_mock_static Threads::Threads dmlc) +if (RABIT_BUILD_MPI) + list(APPEND RABIT_SOURCES ${CMAKE_CURRENT_LIST_DIR}/src/engine_mpi.cc) +elseif (RABIT_MOCK) + list(APPEND RABIT_SOURCES ${CMAKE_CURRENT_LIST_DIR}/src/engine_mock.cc) +else () + list(APPEND RABIT_SOURCES ${CMAKE_CURRENT_LIST_DIR}/src/engine.cc) +endif () -set(rabit_libs rabit rabit_mock_static) -set_target_properties(rabit rabit_mock_static - PROPERTIES CXX_STANDARD 14 - CXX_STANDARD_REQUIRED ON - POSITION_INDEPENDENT_CODE ON) - -if(RABIT_BUILD_MPI) - find_package(MPI REQUIRED) - if (NOT MPI_CXX_FOUND) - message(FATAL_ERROR "CXX Interface for MPI is required for building MPI backend.") - endif (NOT MPI_CXX_FOUND) - add_library(rabit_mpi src/engine_mpi.cc ${MPI_INCLUDE_PATH}) - target_link_libraries(rabit_mpi ${MPI_CXX_LIBRARIES}) - list(APPEND rabit_libs rabit_mpi) -endif() - -# place binaries and libraries according to GNU standards -include(GNUInstallDirs) - -# we use this to get code coverage -if ((CMAKE_CONFIGURATION_TYPES STREQUAL "Debug") AND (CMAKE_CXX_COMPILER_ID MATCHES GNU)) - foreach(lib ${rabit_libs}) - target_compile_options(${lib} - -fprofile-arcs - -ftest-coverage) - endforeach() -endif((CMAKE_CONFIGURATION_TYPES STREQUAL "Debug") AND (CMAKE_CXX_COMPILER_ID MATCHES GNU)) - -foreach(lib ${rabit_libs}) - target_include_directories(${lib} PUBLIC - "$" - "$") -endforeach() - -if (GOOGLE_TEST AND (NOT WIN32)) - enable_testing() - - # rabit mock based integration tests - list(REMOVE_ITEM rabit_libs "rabit_mock_static") # remove here to avoid installing it - set(tests lazy_recover local_recover model_recover) - - foreach(test ${tests}) - add_executable(${test} test/${test}.cc) - target_link_libraries(${test} rabit_mock_static) - set_target_properties(${test} PROPERTIES CXX_STANDARD 14 CXX_STANDARD_REQUIRED ON) - add_test(NAME ${test} COMMAND ${test} WORKING_DIRECTORY ${xgboost_BINARY_DIR}) - endforeach() - - if(RABIT_BUILD_MPI) - add_executable(speed_test_mpi test/speed_test.cc) - target_link_libraries(speed_test_mpi rabit_mpi) - add_test(NAME speed_test_mpi COMMAND speed_test_mpi WORKING_DIRECTORY ${xgboost_BINARY_DIR}) - endif(RABIT_BUILD_MPI) -endif (GOOGLE_TEST AND (NOT WIN32)) - -# Headers: -set(include_install_dir "include") -install( - DIRECTORY "include/" - DESTINATION "${include_install_dir}" - FILES_MATCHING PATTERN "*.h" - ) +set(RABIT_SOURCES ${RABIT_SOURCES} PARENT_SCOPE) diff --git a/rabit/include/rabit/c_api.h b/rabit/include/rabit/c_api.h index 47c57351d..404981457 100644 --- a/rabit/include/rabit/c_api.h +++ b/rabit/include/rabit/c_api.h @@ -73,7 +73,7 @@ RABIT_DLL int RabitIsDistributed(void); * the user who monitors the tracker * \param msg the message to be printed */ -RABIT_DLL void RabitTrackerPrint(const char *msg); +RABIT_DLL int RabitTrackerPrint(const char *msg); /*! * \brief get name of processor * \param out_name hold output string diff --git a/rabit/include/rabit/internal/socket.h b/rabit/include/rabit/internal/socket.h index b93cc953d..2e10cf50c 100644 --- a/rabit/include/rabit/internal/socket.h +++ b/rabit/include/rabit/internal/socket.h @@ -622,7 +622,9 @@ struct PollHelper { fdset.push_back(kv.second); } int ret = PollImpl(fdset.data(), fdset.size(), timeout); - if (ret <= 0) { + if (ret == 0) { + LOG(FATAL) << "Poll timeout"; + } else if (ret < 0) { Socket::Error("Poll"); } else { for (auto& pfd : fdset) { diff --git a/rabit/include/rabit/internal/utils.h b/rabit/include/rabit/internal/utils.h index 8ffe0b8f3..4fbb7c369 100644 --- a/rabit/include/rabit/internal/utils.h +++ b/rabit/include/rabit/internal/utils.h @@ -15,6 +15,7 @@ #include #include #include "dmlc/io.h" +#include "xgboost/logging.h" #include #if !defined(__GNUC__) || defined(__FreeBSD__) @@ -73,17 +74,14 @@ inline bool StringToBool(const char* s) { * \param msg error message */ inline void HandleAssertError(const char *msg) { - fprintf(stderr, - "AssertError:%s, rabit is configured to keep process running\n", msg); - throw dmlc::Error(msg); + LOG(FATAL) << msg; } /*! * \brief handling of Check error, caused by inappropriate input * \param msg error message */ inline void HandleCheckError(const char *msg) { - fprintf(stderr, "%s, rabit is configured to keep process running\n", msg); - throw dmlc::Error(msg); + LOG(FATAL) << msg; } inline void HandlePrint(const char *msg) { @@ -154,13 +152,6 @@ inline void Error(const char *fmt, ...) { HandleCheckError(msg.c_str()); } } - -/*! \brief replace fopen, report error when the file open fails */ -inline std::FILE *FopenCheck(const char *fname, const char *flag) { - std::FILE *fp = fopen64(fname, flag); - Check(fp != nullptr, "can not open file \"%s\"\n", fname); - return fp; -} } // namespace utils // Can not use std::min on Windows with msvc due to: diff --git a/rabit/src/allreduce_base.cc b/rabit/src/allreduce_base.cc index 60c8f744b..021a7eac3 100644 --- a/rabit/src/allreduce_base.cc +++ b/rabit/src/allreduce_base.cc @@ -123,7 +123,9 @@ bool AllreduceBase::Init(int argc, char* argv[]) { bool AllreduceBase::Shutdown() { try { for (auto & all_link : all_links) { - all_link.sock.Close(); + if (!all_link.sock.IsClosed()) { + all_link.sock.Close(); + } } all_links.clear(); tree_links.plinks.clear(); @@ -136,7 +138,7 @@ bool AllreduceBase::Shutdown() { utils::TCPSocket::Finalize(); return true; } catch (const std::exception& e) { - fprintf(stderr, "failed to shutdown due to %s\n", e.what()); + LOG(WARNING) << "Failed to shutdown due to" << e.what(); return false; } } @@ -217,7 +219,7 @@ void AllreduceBase::SetParam(const char *name, const char *val) { rabit_enable_tcp_no_delay = true; } else { rabit_enable_tcp_no_delay = false; -} + } } } /*! @@ -586,7 +588,7 @@ AllreduceBase::TryAllreduceTree(void *sendrecvbuf_, // eachreduce size if (max_reduce < total_size) { max_reduce = max_reduce - max_reduce % eachreduce; -} + } // peform reduce, can be at most two rounds while (size_up_reduce < max_reduce) { diff --git a/rabit/src/c_api.cc b/rabit/src/c_api.cc index 5328daf08..f59722f2c 100644 --- a/rabit/src/c_api.cc +++ b/rabit/src/c_api.cc @@ -6,6 +6,8 @@ #include "rabit/rabit.h" #include "rabit/c_api.h" +#include "../../src/c_api/c_api_error.h" + namespace rabit { namespace c_api { // helper use to avoid BitOR operator @@ -219,11 +221,19 @@ struct WriteWrapper : public Serializable { } // namespace rabit RABIT_DLL bool RabitInit(int argc, char *argv[]) { - return rabit::Init(argc, argv); + auto ret = rabit::Init(argc, argv); + if (!ret) { + XGBAPISetLastError("Failed to initialize RABIT."); + } + return ret; } RABIT_DLL bool RabitFinalize() { - return rabit::Finalize(); + auto ret = rabit::Finalize(); + if (!ret) { + XGBAPISetLastError("Failed to shutdown RABIT worker."); + } + return ret; } RABIT_DLL int RabitGetRingPrevRank() { @@ -242,9 +252,11 @@ RABIT_DLL int RabitIsDistributed() { return rabit::IsDistributed(); } -RABIT_DLL void RabitTrackerPrint(const char *msg) { +RABIT_DLL int RabitTrackerPrint(const char *msg) { + API_BEGIN() std::string m(msg); rabit::TrackerPrint(m); + API_END() } RABIT_DLL void RabitGetProcessorName(char *out_name, diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index edcb64a34..e02539435 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -5,6 +5,8 @@ list(REMOVE_ITEM CPU_SOURCES ${xgboost_SOURCE_DIR}/src/cli_main.cc) # Object library is necessary for jvm-package, which creates its own shared library. add_library(objxgboost OBJECT) target_sources(objxgboost PRIVATE ${CPU_SOURCES}) +target_sources(objxgboost PRIVATE ${RABIT_SOURCES}) + if (USE_CUDA) file(GLOB_RECURSE CUDA_SOURCES *.cu *.cuh) target_sources(objxgboost PRIVATE ${CUDA_SOURCES})