From 7adc4bbc952c57b103b50e6c2fb6722b988f4d95 Mon Sep 17 00:00:00 2001 From: Hyunsu Cho Date: Fri, 8 Jan 2021 20:14:16 -0800 Subject: [PATCH 01/12] Update dmlc-core --- cmake/ExternalLibs.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/ExternalLibs.cmake b/cmake/ExternalLibs.cmake index d815aa16..fee59372 100644 --- a/cmake/ExternalLibs.cmake +++ b/cmake/ExternalLibs.cmake @@ -3,7 +3,7 @@ include(FetchContent) FetchContent_Declare( dmlccore GIT_REPOSITORY https://github.com/dmlc/dmlc-core - GIT_TAG v0.4 + GIT_TAG f0ff3146705e27ac2b1e6081fa0983a8a6fda72d ) FetchContent_MakeAvailable(dmlccore) target_compile_options(dmlc PRIVATE From 516e98e36a1020ae3ad8c2efbeaf0317f25b9894 Mon Sep 17 00:00:00 2001 From: Hyunsu Cho Date: Fri, 8 Jan 2021 20:14:33 -0800 Subject: [PATCH 02/12] Lower CMake requirement to 3.13 --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 6a49810b..ab885576 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,5 +1,5 @@ set (CMAKE_FIND_NO_INSTALL_PREFIX TRUE FORCE) -cmake_minimum_required (VERSION 3.14) +cmake_minimum_required (VERSION 3.13) project(treelite LANGUAGES CXX C VERSION 1.0.0) set(CMAKE_CXX_STANDARD 14) From 404d8f023706830470fd3cb5c0fc84ec49666c6c Mon Sep 17 00:00:00 2001 From: Hyunsu Cho Date: Fri, 8 Jan 2021 20:21:24 -0800 Subject: [PATCH 03/12] Make static libs optional; Add CMake option BUILD_STATIC_LIBS --- CMakeLists.txt | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ab885576..f188ec8d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -14,6 +14,7 @@ endif() option(TEST_COVERAGE "C++ test coverage" OFF) option(USE_OPENMP "Use OpenMP" ON) option(BUILD_CPP_TEST "Build C++ tests" OFF) +option(BUILD_STATIC_LIBS "Build static libs, in addition to dynamic libs" OFF) option(DETECT_CONDA_ENV "Enable detection of conda environment for dependencies" ON) option(BUILD_JVM_RUNTIME "Build Treelite runtime for JVM" OFF) option(ENABLE_ALL_WARNINGS "Enable all compiler warnings. Only effective for GCC/Clang" OFF) @@ -51,15 +52,21 @@ if(BUILD_CPP_TEST) endif() add_library(treelite SHARED) -add_library(treelite_static STATIC) add_library(treelite_runtime SHARED) -add_library(treelite_runtime_static STATIC) target_link_libraries(treelite PRIVATE objtreelite objtreelite_common) -target_link_libraries(treelite_static PRIVATE objtreelite objtreelite_common) target_link_libraries(treelite_runtime PRIVATE objtreelite_runtime objtreelite_common) -target_link_libraries(treelite_runtime_static PRIVATE objtreelite_runtime objtreelite_common) -foreach(lib treelite treelite_static treelite_runtime treelite_runtime_static) +set(TREELITE_TARGETS treelite treelite_runtime) + +if(BUILD_STATIC_LIBS) + add_library(treelite_static STATIC) + add_library(treelite_runtime_static STATIC) + target_link_libraries(treelite_static PRIVATE objtreelite objtreelite_common) + target_link_libraries(treelite_runtime_static PRIVATE objtreelite_runtime objtreelite_common) + list(APPEND TREELITE_TARGETS treelite_static treelite_runtime_static) +endif() + +foreach(lib ${TREELITE_TARGETS}) set_output_directory(${lib} ${PROJECT_BINARY_DIR}) target_link_libraries(${lib} INTERFACE dmlc) endforeach() @@ -67,7 +74,7 @@ endforeach() # Export install targets include(GNUInstallDirs) include(CMakePackageConfigHelpers) -set(INSTALL_TARGETS treelite treelite_static treelite_runtime treelite_runtime_static +set(INSTALL_TARGETS ${TREELITE_TARGETS} objtreelite objtreelite_common objtreelite_runtime dmlc rapidjson) if(NOT FMTLIB_FROM_SYSTEM_ROOT) list(APPEND INSTALL_TARGETS fmt-header-only) From 53a03c78ca8f226859ef22b45afcee3625a7c8b6 Mon Sep 17 00:00:00 2001 From: Hyunsu Cho Date: Fri, 8 Jan 2021 20:22:40 -0800 Subject: [PATCH 04/12] Fix loading DLL on Windows for Python 3.8+ --- python/treelite/core.py | 3 +++ runtime/python/treelite_runtime/predictor.py | 2 ++ 2 files changed, 5 insertions(+) diff --git a/python/treelite/core.py b/python/treelite/core.py index 120a612f..2bb77a0a 100644 --- a/python/treelite/core.py +++ b/python/treelite/core.py @@ -3,6 +3,7 @@ from __future__ import absolute_import as _abs import sys +import os import ctypes from .util import py_str, _log_callback, TreeliteError @@ -12,6 +13,8 @@ def _load_lib(): """Load Treelite Library.""" lib_path = find_lib_path() + if sys.version_info >= (3, 8) and sys.platform == 'win32': + os.add_dll_directory(os.path.join(os.path.normpath(sys.prefix), 'Library', 'bin')) lib = ctypes.cdll.LoadLibrary(lib_path[0]) lib.TreeliteGetLastError.restype = ctypes.c_char_p lib.callback = _log_callback diff --git a/runtime/python/treelite_runtime/predictor.py b/runtime/python/treelite_runtime/predictor.py index 949fd119..28f80df2 100644 --- a/runtime/python/treelite_runtime/predictor.py +++ b/runtime/python/treelite_runtime/predictor.py @@ -17,6 +17,8 @@ def _load_runtime_lib(): """Load Treelite runtime""" lib_path = find_lib_path() + if sys.version_info >= (3, 8) and sys.platform == 'win32': + os.add_dll_directory(os.path.join(os.path.normpath(sys.prefix), 'Library', 'bin')) lib = ctypes.cdll.LoadLibrary(lib_path[0]) lib.TreeliteGetLastError.restype = ctypes.c_char_p lib.callback = _log_callback From 2f57c8f389ed354c1e39e89129521bff25e8ae6f Mon Sep 17 00:00:00 2001 From: Hyunsu Cho Date: Fri, 8 Jan 2021 20:31:42 -0800 Subject: [PATCH 05/12] [CI] The cmake_import_test should build Treelite with static libs --- tests/travis/run_test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/travis/run_test.sh b/tests/travis/run_test.sh index bebd498e..91269665 100755 --- a/tests/travis/run_test.sh +++ b/tests/travis/run_test.sh @@ -43,7 +43,7 @@ then rm -rf build/ mkdir build cd build - cmake .. -DUSE_OPENMP=ON -GNinja + cmake .. -DUSE_OPENMP=ON -DBUILD_STATIC_LIBS=ON -GNinja ninja install # Try compiling a sample application From 2a4d07c4c9d78775aab5badad982f2674963df5e Mon Sep 17 00:00:00 2001 From: Hyunsu Cho Date: Fri, 8 Jan 2021 20:36:58 -0800 Subject: [PATCH 06/12] Fix pylint --- python/treelite/core.py | 1 + runtime/python/treelite_runtime/predictor.py | 1 + 2 files changed, 2 insertions(+) diff --git a/python/treelite/core.py b/python/treelite/core.py index 2bb77a0a..4af6bd0f 100644 --- a/python/treelite/core.py +++ b/python/treelite/core.py @@ -14,6 +14,7 @@ def _load_lib(): """Load Treelite Library.""" lib_path = find_lib_path() if sys.version_info >= (3, 8) and sys.platform == 'win32': + # pylint: disable=no-member os.add_dll_directory(os.path.join(os.path.normpath(sys.prefix), 'Library', 'bin')) lib = ctypes.cdll.LoadLibrary(lib_path[0]) lib.TreeliteGetLastError.restype = ctypes.c_char_p diff --git a/runtime/python/treelite_runtime/predictor.py b/runtime/python/treelite_runtime/predictor.py index 28f80df2..9fba46fb 100644 --- a/runtime/python/treelite_runtime/predictor.py +++ b/runtime/python/treelite_runtime/predictor.py @@ -18,6 +18,7 @@ def _load_runtime_lib(): """Load Treelite runtime""" lib_path = find_lib_path() if sys.version_info >= (3, 8) and sys.platform == 'win32': + # pylint: disable=no-member os.add_dll_directory(os.path.join(os.path.normpath(sys.prefix), 'Library', 'bin')) lib = ctypes.cdll.LoadLibrary(lib_path[0]) lib.TreeliteGetLastError.restype = ctypes.c_char_p From 6041daa9ebb79967ebc0aba3798000b003bf162d Mon Sep 17 00:00:00 2001 From: Hyunsu Cho Date: Fri, 8 Jan 2021 20:51:03 -0800 Subject: [PATCH 07/12] Create a compatibility layer for CMake 3.13 to use FetchContent_MakeAvailable --- cmake/ExternalLibs.cmake | 1 + cmake/FetchContentMakeAvailable.cmake | 10 ++++++++++ 2 files changed, 11 insertions(+) create mode 100644 cmake/FetchContentMakeAvailable.cmake diff --git a/cmake/ExternalLibs.cmake b/cmake/ExternalLibs.cmake index fee59372..f826c71b 100644 --- a/cmake/ExternalLibs.cmake +++ b/cmake/ExternalLibs.cmake @@ -1,4 +1,5 @@ include(FetchContent) +include(cmake/FetchContentMakeAvailable.cmake) FetchContent_Declare( dmlccore diff --git a/cmake/FetchContentMakeAvailable.cmake b/cmake/FetchContentMakeAvailable.cmake new file mode 100644 index 00000000..7b5cf055 --- /dev/null +++ b/cmake/FetchContentMakeAvailable.cmake @@ -0,0 +1,10 @@ +# Credit: https://github.com/ademuri/cmake-fetch-content +if(${CMAKE_VERSION} VERSION_LESS 3.14) + macro(FetchContent_MakeAvailable NAME) + FetchContent_GetProperties(${NAME}) + if(NOT ${NAME}_POPULATED) + FetchContent_Populate(${NAME}) + add_subdirectory(${${NAME}_SOURCE_DIR} ${${NAME}_BINARY_DIR}) + endif() + endmacro() +endif() From 0020e656939a8d4bfcd8ebb6514543a525054808 Mon Sep 17 00:00:00 2001 From: Hyunsu Cho Date: Fri, 8 Jan 2021 22:39:25 -0800 Subject: [PATCH 08/12] [CI] Use gcc-10 to run export_lib tests --- .travis.yml | 2 +- tests/travis/run_test.sh | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index 1226a39e..43fe28f4 100644 --- a/.travis.yml +++ b/.travis.yml @@ -30,7 +30,7 @@ addons: homebrew: packages: - cmake - - gcc@7 + - gcc@10 - libomp - wget - lcov diff --git a/tests/travis/run_test.sh b/tests/travis/run_test.sh index 91269665..530feb35 100755 --- a/tests/travis/run_test.sh +++ b/tests/travis/run_test.sh @@ -22,7 +22,7 @@ then python -m pip install xgboost python -m pip install lightgbm codecov ./build/treelite_cpp_test - export GCC_PATH=gcc-7 + export GCC_PATH=gcc-10 PYTHONPATH=./python:./runtime/python python -m pytest --cov=treelite --cov=treelite_runtime -v --fulltrace tests/python lcov --directory . --capture --output-file coverage.info lcov --remove coverage.info '*dmlccore*' --output-file coverage.info @@ -92,7 +92,7 @@ then conda install -c conda-forge numpy scipy pandas pytest scikit-learn coverage python -m pip install xgboost python -m pip install lightgbm - export GCC_PATH=gcc-7 + export GCC_PATH=gcc-10 python -m pytest -v --fulltrace tests/python # Deploy binary wheel to S3 @@ -124,7 +124,7 @@ if [ ${TASK} == "python_sdist_test" ]; then conda install -c conda-forge numpy scipy pandas pytest scikit-learn coverage python -m pip install xgboost python -m pip install lightgbm - export GCC_PATH=gcc-7 + export GCC_PATH=gcc-10 python -m pytest -v --fulltrace tests/python # Deploy source wheel to S3 From d431d2bce2d26913d80c26c0b72c305bbcb9db62 Mon Sep 17 00:00:00 2001 From: Hyunsu Cho Date: Sat, 9 Jan 2021 00:06:52 -0800 Subject: [PATCH 09/12] [CI] Just use Apple Clang on MacOS --- .travis.yml | 1 - tests/python/test_lightgbm_integration.py | 4 +++- tests/python/test_model_query.py | 11 +++++++---- tests/python/util.py | 11 +++++++++++ tests/travis/run_test.sh | 3 --- 5 files changed, 21 insertions(+), 9 deletions(-) diff --git a/.travis.yml b/.travis.yml index 43fe28f4..53f0c75d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -30,7 +30,6 @@ addons: homebrew: packages: - cmake - - gcc@10 - libomp - wget - lcov diff --git a/tests/python/test_lightgbm_integration.py b/tests/python/test_lightgbm_integration.py index 43a81ed4..e1de696f 100644 --- a/tests/python/test_lightgbm_integration.py +++ b/tests/python/test_lightgbm_integration.py @@ -10,7 +10,7 @@ from treelite.contrib import _libext from treelite.util import has_sklearn from .metadata import dataset_db, _qualify_path -from .util import os_compatible_toolchains, os_platform, check_predictor +from .util import os_compatible_toolchains, os_platform, check_predictor, is_apple_clang try: import lightgbm @@ -227,6 +227,8 @@ def test_sparse_categorical_model(tmpdir, quantize, toolchain): pytest.xfail(reason='Clang cannot handle long if conditional') if os_platform() == 'windows': pytest.xfail(reason='MSVC cannot handle long if conditional') + if os_platform() == 'osx' and is_apple_clang(toolchain): + pytest.xfail(reason='Apple Clang cannot handle long if conditional') dataset = 'sparse_categorical' libpath = os.path.join(tmpdir, dataset_db[dataset].libname + _libext()) model = treelite.Model.load(dataset_db[dataset].model, model_format=dataset_db[dataset].format) diff --git a/tests/python/test_model_query.py b/tests/python/test_model_query.py index cf164147..8ba6439d 100644 --- a/tests/python/test_model_query.py +++ b/tests/python/test_model_query.py @@ -9,7 +9,7 @@ import treelite_runtime from treelite.contrib import _libext from .metadata import dataset_db -from .util import os_platform, os_compatible_toolchains +from .util import os_platform, os_compatible_toolchains, is_apple_clang ModelFact = collections.namedtuple( 'ModelFact', @@ -28,8 +28,12 @@ 'dataset', ['mushroom', 'dermatology', 'letor', 'toy_categorical', 'sparse_categorical']) def test_model_query(tmpdir, dataset): """Test all query functions for every example model""" - if dataset == 'sparse_categorical' and os_platform() == 'windows': - pytest.xfail('MSVC cannot handle long if conditional') + toolchain = os_compatible_toolchains()[0] + if dataset == 'sparse_categorical': + if os_platform() == 'windows': + pytest.xfail('MSVC cannot handle long if conditional') + elif os_platform() == 'osx' and is_apple_clang(toolchain): + pytest.xfail('Apple Clang cannot handle long if conditional') if dataset == 'letor' and os_platform() == 'windows': pytest.xfail('export_lib() is too slow for letor on MSVC') @@ -39,7 +43,6 @@ def test_model_query(tmpdir, dataset): assert model.num_class == _model_facts[dataset].num_class assert model.num_tree == _model_facts[dataset].num_tree - toolchain = os_compatible_toolchains()[0] model.export_lib(toolchain=toolchain, libpath=libpath, params={'quantize': 1, 'parallel_comp': model.num_tree}, verbose=True) predictor = treelite_runtime.Predictor(libpath=libpath, verbose=True) diff --git a/tests/python/util.py b/tests/python/util.py index c0a392c1..e193619b 100644 --- a/tests/python/util.py +++ b/tests/python/util.py @@ -1,6 +1,8 @@ # -*- coding: utf-8 -*- """Utility functions for tests""" import os +import subprocess +import re from sys import platform as _platform from contextlib import contextmanager @@ -42,6 +44,15 @@ def os_platform(): return 'unix' +def is_apple_clang(toolchain): + try: + proc = subprocess.run([toolchain, '--version'], check=True, capture_output=True) + stdout = proc.stdout.decode('utf-8') + return re.search(r'Apple Clang', stdout, re.IGNORECASE): + except: + return False + + def libname(fmt): """Format name for a shared library, using appropriate file extension""" return fmt.format(_libext()) diff --git a/tests/travis/run_test.sh b/tests/travis/run_test.sh index 530feb35..696d19a4 100755 --- a/tests/travis/run_test.sh +++ b/tests/travis/run_test.sh @@ -22,7 +22,6 @@ then python -m pip install xgboost python -m pip install lightgbm codecov ./build/treelite_cpp_test - export GCC_PATH=gcc-10 PYTHONPATH=./python:./runtime/python python -m pytest --cov=treelite --cov=treelite_runtime -v --fulltrace tests/python lcov --directory . --capture --output-file coverage.info lcov --remove coverage.info '*dmlccore*' --output-file coverage.info @@ -92,7 +91,6 @@ then conda install -c conda-forge numpy scipy pandas pytest scikit-learn coverage python -m pip install xgboost python -m pip install lightgbm - export GCC_PATH=gcc-10 python -m pytest -v --fulltrace tests/python # Deploy binary wheel to S3 @@ -124,7 +122,6 @@ if [ ${TASK} == "python_sdist_test" ]; then conda install -c conda-forge numpy scipy pandas pytest scikit-learn coverage python -m pip install xgboost python -m pip install lightgbm - export GCC_PATH=gcc-10 python -m pytest -v --fulltrace tests/python # Deploy source wheel to S3 From 38b6f9983184d34a05dc45c2b030ff1afd96359c Mon Sep 17 00:00:00 2001 From: Hyunsu Cho Date: Sat, 9 Jan 2021 00:22:02 -0800 Subject: [PATCH 10/12] fix typo --- tests/python/util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/python/util.py b/tests/python/util.py index e193619b..400a68e0 100644 --- a/tests/python/util.py +++ b/tests/python/util.py @@ -48,7 +48,7 @@ def is_apple_clang(toolchain): try: proc = subprocess.run([toolchain, '--version'], check=True, capture_output=True) stdout = proc.stdout.decode('utf-8') - return re.search(r'Apple Clang', stdout, re.IGNORECASE): + return re.search(r'Apple Clang', stdout, re.IGNORECASE) except: return False From cf11b40683234191991e6dc9eaea841ab2c8770b Mon Sep 17 00:00:00 2001 From: Hyunsu Cho Date: Sat, 9 Jan 2021 00:26:42 -0800 Subject: [PATCH 11/12] Fix lint --- tests/python/util.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/python/util.py b/tests/python/util.py index 400a68e0..4b78d80d 100644 --- a/tests/python/util.py +++ b/tests/python/util.py @@ -45,11 +45,12 @@ def os_platform(): def is_apple_clang(toolchain): + """Query whether a given compiler executable is Apple Clang compiler.""" try: proc = subprocess.run([toolchain, '--version'], check=True, capture_output=True) stdout = proc.stdout.decode('utf-8') return re.search(r'Apple Clang', stdout, re.IGNORECASE) - except: + except subprocess.CalledProcessError: return False From 7514d43a0ee395e8c97670b123160a84e0d34af6 Mon Sep 17 00:00:00 2001 From: Hyunsu Cho Date: Sat, 9 Jan 2021 01:08:42 -0800 Subject: [PATCH 12/12] Skip sparse categorical tests on MacOS --- tests/python/test_lightgbm_integration.py | 4 ++-- tests/python/test_model_query.py | 6 +++--- tests/python/util.py | 12 ------------ 3 files changed, 5 insertions(+), 17 deletions(-) diff --git a/tests/python/test_lightgbm_integration.py b/tests/python/test_lightgbm_integration.py index e1de696f..0c16ad3f 100644 --- a/tests/python/test_lightgbm_integration.py +++ b/tests/python/test_lightgbm_integration.py @@ -10,7 +10,7 @@ from treelite.contrib import _libext from treelite.util import has_sklearn from .metadata import dataset_db, _qualify_path -from .util import os_compatible_toolchains, os_platform, check_predictor, is_apple_clang +from .util import os_compatible_toolchains, os_platform, check_predictor try: import lightgbm @@ -227,7 +227,7 @@ def test_sparse_categorical_model(tmpdir, quantize, toolchain): pytest.xfail(reason='Clang cannot handle long if conditional') if os_platform() == 'windows': pytest.xfail(reason='MSVC cannot handle long if conditional') - if os_platform() == 'osx' and is_apple_clang(toolchain): + if os_platform() == 'osx': pytest.xfail(reason='Apple Clang cannot handle long if conditional') dataset = 'sparse_categorical' libpath = os.path.join(tmpdir, dataset_db[dataset].libname + _libext()) diff --git a/tests/python/test_model_query.py b/tests/python/test_model_query.py index 8ba6439d..665fa45f 100644 --- a/tests/python/test_model_query.py +++ b/tests/python/test_model_query.py @@ -9,7 +9,7 @@ import treelite_runtime from treelite.contrib import _libext from .metadata import dataset_db -from .util import os_platform, os_compatible_toolchains, is_apple_clang +from .util import os_platform, os_compatible_toolchains ModelFact = collections.namedtuple( 'ModelFact', @@ -28,11 +28,10 @@ 'dataset', ['mushroom', 'dermatology', 'letor', 'toy_categorical', 'sparse_categorical']) def test_model_query(tmpdir, dataset): """Test all query functions for every example model""" - toolchain = os_compatible_toolchains()[0] if dataset == 'sparse_categorical': if os_platform() == 'windows': pytest.xfail('MSVC cannot handle long if conditional') - elif os_platform() == 'osx' and is_apple_clang(toolchain): + elif os_platform() == 'osx': pytest.xfail('Apple Clang cannot handle long if conditional') if dataset == 'letor' and os_platform() == 'windows': pytest.xfail('export_lib() is too slow for letor on MSVC') @@ -43,6 +42,7 @@ def test_model_query(tmpdir, dataset): assert model.num_class == _model_facts[dataset].num_class assert model.num_tree == _model_facts[dataset].num_tree + toolchain = os_compatible_toolchains()[0] model.export_lib(toolchain=toolchain, libpath=libpath, params={'quantize': 1, 'parallel_comp': model.num_tree}, verbose=True) predictor = treelite_runtime.Predictor(libpath=libpath, verbose=True) diff --git a/tests/python/util.py b/tests/python/util.py index 4b78d80d..c0a392c1 100644 --- a/tests/python/util.py +++ b/tests/python/util.py @@ -1,8 +1,6 @@ # -*- coding: utf-8 -*- """Utility functions for tests""" import os -import subprocess -import re from sys import platform as _platform from contextlib import contextmanager @@ -44,16 +42,6 @@ def os_platform(): return 'unix' -def is_apple_clang(toolchain): - """Query whether a given compiler executable is Apple Clang compiler.""" - try: - proc = subprocess.run([toolchain, '--version'], check=True, capture_output=True) - stdout = proc.stdout.decode('utf-8') - return re.search(r'Apple Clang', stdout, re.IGNORECASE) - except subprocess.CalledProcessError: - return False - - def libname(fmt): """Format name for a shared library, using appropriate file extension""" return fmt.format(_libext())