From 0afe3699dd4aabbcce2e0f230b4f7ce610019cb5 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Sat, 27 Apr 2019 02:08:34 -0400 Subject: [PATCH 01/32] Fix unnecessary warning when BUILD_PYTHON_BINDINGS is specified. --- CMakeLists.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 26dfd6e2c50..ce0d1f248ac 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -20,7 +20,9 @@ if (WIN32) option(BUILD_PYTHON_BINDINGS "Build Python bindings." OFF) option(BUILD_SHARED_LIBS "Compile shared libraries (if OFF, static libraries are compiled)." OFF) - message(WARNING "By default Python bindings are not compiled for Windows because they are not known to work. Set BUILD_PYTHON_BINDINGS to ON if you want them built.") + if (NOT BUILD_PYTHON_BINDINGS) + message(WARNING "By default Python bindings are not compiled for Windows because they are not known to work. Set BUILD_PYTHON_BINDINGS to ON if you want them built.") + endif () else () option(BUILD_PYTHON_BINDINGS "Build Python bindings." ON) option(BUILD_SHARED_LIBS From 1ccd52a57c0148e147a09bc00ceaf3f3ea773e71 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Sat, 27 Apr 2019 02:08:53 -0400 Subject: [PATCH 02/32] Set library link directories according to the CMake configuration. --- src/mlpack/bindings/python/setup.py.in | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/mlpack/bindings/python/setup.py.in b/src/mlpack/bindings/python/setup.py.in index 2263a7041dc..f7d549b0068 100644 --- a/src/mlpack/bindings/python/setup.py.in +++ b/src/mlpack/bindings/python/setup.py.in @@ -49,7 +49,8 @@ else: '${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/'] + '${CYTHON_INCLUDE_DIRECTORIES}'.split(';'), libraries=['mlpack', 'boost_serialization'], - library_dirs=['${CMAKE_BINARY_DIR}/lib/'], + library_dirs=['${CMAKE_LIBRARY_OUTPUT_DIRECTORY}'] + + '${MLPACK_LIBRARY_DIRS}'.split(';'), # CMAKE_CXX_FLAGS seems to have an extra space. extra_compile_args=extra_args, extra_link_args=extra_link_args, @@ -67,7 +68,8 @@ else: '${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/'] + '${CYTHON_INCLUDE_DIRECTORIES}'.split(';'), libraries=['mlpack', 'boost_serialization'], - library_dirs=['${CMAKE_BINARY_DIR}/lib/'], + library_dirs=['${CMAKE_LIBRARY_OUTPUT_DIRECTORY}'] + + '${MLPACK_LIBRARY_DIRS}'.split(';'), # CMAKE_CXX_FLAGS seems to have an extra space. extra_compile_args=extra_args, extra_link_args=extra_link_args, From 0f17263a3f91199b9ea26f5d05a64a77093dfa49 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Sat, 27 Apr 2019 02:09:04 -0400 Subject: [PATCH 03/32] Fix incorrect package_dir setting. --- src/mlpack/bindings/python/setup.py.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mlpack/bindings/python/setup.py.in b/src/mlpack/bindings/python/setup.py.in index f7d549b0068..0991f8c7977 100644 --- a/src/mlpack/bindings/python/setup.py.in +++ b/src/mlpack/bindings/python/setup.py.in @@ -105,7 +105,7 @@ setup(name='mlpack', 'Source': 'https://github.com/mlpack/mlpack/', 'Tracker': 'https://github.com/mlpack/mlpack/issues'}, install_requires=['cython>=0.24', 'numpy', 'pandas'], - package_dir={ '': '${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/' }, + package_dir={ '': '.' }, # Might be superfluous. packages=['mlpack'], cmdclass={ 'build_ext': build_ext }, ext_modules = modules, From 40b365afdb68f2432dded3458bf146a583528753 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Sun, 12 May 2019 23:17:13 -0400 Subject: [PATCH 04/32] Use CMake generators to get paths right on Windows (hopefully). --- src/mlpack/CMakeLists.txt | 22 ++++++++++-- src/mlpack/bindings/python/CMakeLists.txt | 8 +++-- src/mlpack/bindings/python/setup.py.in | 41 ++++++++++++----------- 3 files changed, 47 insertions(+), 24 deletions(-) diff --git a/src/mlpack/CMakeLists.txt b/src/mlpack/CMakeLists.txt index c141015fcd2..d90f13a0012 100644 --- a/src/mlpack/CMakeLists.txt +++ b/src/mlpack/CMakeLists.txt @@ -120,8 +120,26 @@ if (BUILD_PYTHON_BINDINGS) get_property(CYTHON_INCLUDE_DIRECTORIES DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} PROPERTY INCLUDE_DIRECTORIES) - configure_file(${CMAKE_SOURCE_DIR}/src/mlpack/bindings/python/setup.py.in - ${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/setup.py) + add_custom_target(python_configure + COMMAND ${CMAKE_COMMAND} + -D SETUP_PY_IN="${CMAKE_SOURCE_DIR}/src/mlpack/bindings/python/setup.py.in" + -D SETUP_PY_OUT="${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/setup.py" + -D PACKAGE_VERSION="${PACKAGE_VERSION}" + -D Boost_SERIALIZATION_LIBRARY="${Boost_SERIALIZATION_LIBRARY}" + -D MLPACK_LIBRARY="$" + -D MLPACK_PYXS="${MLPACK_PYXS}" + -D OpenMP_CXX_FLAGS="${OpenMP_CXX_FLAGS}" + -D DISABLE_CFLAGS="${DISABLE_CFLAGS}" + -D CYTHON_INCLUDE_DIRECTORIES="${CYTHON_INCLUDE_DIRECTORIES}" + -D OUTPUT_DIR="${CMAKE_BINARY_DIR}" + -P "${CMAKE_SOURCE_DIR}/src/mlpack/bindings/python/ConfigureSetup.cmake" + BYPRODUCTS "${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/setup.py" + COMMENT "Configuring setup.py...") + add_dependencies(python_configure python_copy) + add_dependencies(python_configured python_configure) + +# configure_file(${CMAKE_SOURCE_DIR}/src/mlpack/bindings/python/setup.py.in +# ${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/setup.py) endif () # If we are building Markdown documentation, we have to run some setup after we diff --git a/src/mlpack/bindings/python/CMakeLists.txt b/src/mlpack/bindings/python/CMakeLists.txt index b69f1ca567f..77a2d8aec6b 100644 --- a/src/mlpack/bindings/python/CMakeLists.txt +++ b/src/mlpack/bindings/python/CMakeLists.txt @@ -105,6 +105,8 @@ endif () add_custom_target(python ALL DEPENDS mlpack) add_custom_target(python_copy ALL DEPENDS mlpack) +# The python_configure target is added later; this is a dummy target. +add_custom_target(python_configured ALL) # Copy necessary files after making the mlpack/ directory. add_custom_command(TARGET python_copy PRE_BUILD @@ -162,7 +164,7 @@ add_custom_command(TARGET python POST_BUILD ${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/copy_artifacts.py WORKING_DIRECTORY ${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/) -add_dependencies(python python_copy) +add_dependencies(python python_configured) # Configure installation script file. execute_process(COMMAND ${PYTHON_EXECUTABLE} @@ -203,7 +205,7 @@ if (BUILD_PYTHON_BINDINGS) -DBINDING_TYPE=BINDING_TYPE_PYX) add_custom_command(TARGET generate_pyx_${name} POST_BUILD COMMAND ${CMAKE_COMMAND} - -DPROGRAM=${CMAKE_BINARY_DIR}/bin/generate_pyx_${name} + -DPROGRAM=$ -DOUTPUT_FILE=${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/mlpack/${name}.pyx -P ${CMAKE_SOURCE_DIR}/CMake/RunProgram.cmake) @@ -220,7 +222,7 @@ if (BUILD_PYTHON_BINDINGS) add_dependencies(python build_pyx_${name}) add_dependencies(build_pyx_${name} generate_pyx_${name}) - add_dependencies(generate_pyx_${name} python_copy) + add_dependencies(generate_pyx_${name} python_configured) # Add the convenience import to __init__.py. Note that this happens during # configuration. diff --git a/src/mlpack/bindings/python/setup.py.in b/src/mlpack/bindings/python/setup.py.in index 0991f8c7977..0737add2ed6 100644 --- a/src/mlpack/bindings/python/setup.py.in +++ b/src/mlpack/bindings/python/setup.py.in @@ -22,7 +22,7 @@ from setuptools import setup from setuptools.extension import Extension from Cython.Distutils import build_ext -pyxs='${MLPACK_PYXS}'.split(';') +pyxs='${MLPACK_PYXS}'.split(' ') if not '${OpenMP_CXX_FLAGS}': extra_link_args=[] @@ -35,46 +35,49 @@ if os.getenv('NO_BUILD') == '1': else: cxx_flags = '${CMAKE_CXX_FLAGS}'.strip() cxx_flags = re.sub(' +', ' ', cxx_flags) - extra_args = ['-DBINDING_TYPE=BINDING_TYPE_PYX', - '-std=c++11'] + cxx_flags.split(' ') + if cxx_flags: + extra_args = ['-DBINDING_TYPE=BINDING_TYPE_PYX', + '-std=c++11', + '-DNPY_NO_DEPRECATED_API=NPY_1_7_API_VERSION', + '${OpenMP_CXX_FLAGS}'] + cxx_flags.split(' ') + else: + extra_args = ['-DBINDING_TYPE=BINDING_TYPE_PYX', + '-std=c++11', + '-DNPY_NO_DEPRECATED_API=NPY_1_7_API_VERSION', + '${OpenMP_CXX_FLAGS}'] + # This is used for parallel builds; CMake will set PYX_TO_BUILD accordingly. if module is not None: modules=[\ Extension('mlpack.' + name.split('.')[0], - ['${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/mlpack/' + - name], + ['mlpack/' + name], language='c++', include_dirs=[ \ np.get_include(), \ - '${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/'] + - '${CYTHON_INCLUDE_DIRECTORIES}'.split(';'), - libraries=['mlpack', 'boost_serialization'], - library_dirs=['${CMAKE_LIBRARY_OUTPUT_DIRECTORY}'] + - '${MLPACK_LIBRARY_DIRS}'.split(';'), + '${OUTPUT_DIR}/src/mlpack/bindings/python/'] + + '${CYTHON_INCLUDE_DIRECTORIES}'.split(' '), + libraries=['${MLPACK_LIBRARY}', '${Boost_SERIALIZATION_LIBRARY}'], # CMAKE_CXX_FLAGS seems to have an extra space. extra_compile_args=extra_args, extra_link_args=extra_link_args, undef_macros=[] if len("${DISABLE_CFLAGS}") == 0 \ - else '${DISABLE_CFLAGS}'.split(';')) \ + else '${DISABLE_CFLAGS}'.split(' ')) \ for name in pyxs if name == module] else: modules=[\ Extension('mlpack.' + name.split('.')[0], - ['${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/mlpack/' + - name], + ['mlpack/' + name], language='c++', include_dirs=[ \ np.get_include(), \ - '${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/'] + - '${CYTHON_INCLUDE_DIRECTORIES}'.split(';'), - libraries=['mlpack', 'boost_serialization'], - library_dirs=['${CMAKE_LIBRARY_OUTPUT_DIRECTORY}'] + - '${MLPACK_LIBRARY_DIRS}'.split(';'), + '${OUTPUT_DIR}/src/mlpack/bindings/python/'] + + '${CYTHON_INCLUDE_DIRECTORIES}'.split(' '), + libraries=['${MLPACK_LIBRARY}', '${Boost_SERIALIZATION_LIBRARY}'], # CMAKE_CXX_FLAGS seems to have an extra space. extra_compile_args=extra_args, extra_link_args=extra_link_args, undef_macros=[] if len("${DISABLE_CFLAGS}") == 0 \ - else '${DISABLE_CFLAGS}'.split(';')) \ + else '${DISABLE_CFLAGS}'.split(' ')) \ for name in pyxs] setup(name='mlpack', From 819033b2fa783be652e59602367d80cc2f818a35 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Sun, 12 May 2019 23:25:20 -0400 Subject: [PATCH 05/32] Looks like we have to use the deprecated API. --- src/mlpack/bindings/python/setup.py.in | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/mlpack/bindings/python/setup.py.in b/src/mlpack/bindings/python/setup.py.in index 0737add2ed6..6f46063df42 100644 --- a/src/mlpack/bindings/python/setup.py.in +++ b/src/mlpack/bindings/python/setup.py.in @@ -38,12 +38,10 @@ else: if cxx_flags: extra_args = ['-DBINDING_TYPE=BINDING_TYPE_PYX', '-std=c++11', - '-DNPY_NO_DEPRECATED_API=NPY_1_7_API_VERSION', '${OpenMP_CXX_FLAGS}'] + cxx_flags.split(' ') else: extra_args = ['-DBINDING_TYPE=BINDING_TYPE_PYX', '-std=c++11', - '-DNPY_NO_DEPRECATED_API=NPY_1_7_API_VERSION', '${OpenMP_CXX_FLAGS}'] # This is used for parallel builds; CMake will set PYX_TO_BUILD accordingly. From 782361d1453d84d43f15504cc8b03da325579086 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Sun, 12 May 2019 23:27:26 -0400 Subject: [PATCH 06/32] Add dummy targets when not building bindings. --- src/mlpack/bindings/python/CMakeLists.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/mlpack/bindings/python/CMakeLists.txt b/src/mlpack/bindings/python/CMakeLists.txt index 77a2d8aec6b..fd88ca88869 100644 --- a/src/mlpack/bindings/python/CMakeLists.txt +++ b/src/mlpack/bindings/python/CMakeLists.txt @@ -10,6 +10,8 @@ endmacro () # If we are not supposed to make Python bindings, define the macro so it does # nothing and leave this file. if (NOT BUILD_PYTHON_BINDINGS) + add_custom_target(python_configured ALL) + add_custom_target(python ALL) not_found_return("Not building Python bindings.") endif () From c3c738e4207737a5060fc97ca6e3c03c08bbb322 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Sun, 12 May 2019 23:30:52 -0400 Subject: [PATCH 07/32] Oops, that was the wrong fix. --- src/mlpack/bindings/python/CMakeLists.txt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/mlpack/bindings/python/CMakeLists.txt b/src/mlpack/bindings/python/CMakeLists.txt index fd88ca88869..0aa71b70d9b 100644 --- a/src/mlpack/bindings/python/CMakeLists.txt +++ b/src/mlpack/bindings/python/CMakeLists.txt @@ -1,5 +1,6 @@ macro (not_found_return message) message(STATUS "${message}") + set(BUILD_PYTHON_BINDINGS OFF) macro (add_python_binding name) # Do nothing. endmacro () @@ -10,8 +11,6 @@ endmacro () # If we are not supposed to make Python bindings, define the macro so it does # nothing and leave this file. if (NOT BUILD_PYTHON_BINDINGS) - add_custom_target(python_configured ALL) - add_custom_target(python ALL) not_found_return("Not building Python bindings.") endif () From ff66d5c7dc75925fb3f2a3eda25abb7004ea2f41 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Sun, 12 May 2019 23:32:51 -0400 Subject: [PATCH 08/32] A little confusing why this is not working... --- src/mlpack/CMakeLists.txt | 2 +- src/mlpack/bindings/python/CMakeLists.txt | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/mlpack/CMakeLists.txt b/src/mlpack/CMakeLists.txt index d90f13a0012..6a496998714 100644 --- a/src/mlpack/CMakeLists.txt +++ b/src/mlpack/CMakeLists.txt @@ -104,7 +104,7 @@ add_dependencies(mlpack mlpack_headers) # If we are building Python bindings, we have to configure setup.py but only # after we've recursed into methods/. -if (BUILD_PYTHON_BINDINGS) +if (BUILDING_PYTHON_BINDINGS) # Extract the version number. file(READ "${CMAKE_SOURCE_DIR}/src/mlpack/core/util/version.hpp" VERSION_HPP_CONTENTS) diff --git a/src/mlpack/bindings/python/CMakeLists.txt b/src/mlpack/bindings/python/CMakeLists.txt index 0aa71b70d9b..a2e4c7f75cf 100644 --- a/src/mlpack/bindings/python/CMakeLists.txt +++ b/src/mlpack/bindings/python/CMakeLists.txt @@ -1,6 +1,5 @@ macro (not_found_return message) message(STATUS "${message}") - set(BUILD_PYTHON_BINDINGS OFF) macro (add_python_binding name) # Do nothing. endmacro () @@ -41,6 +40,8 @@ if (NOT PY_PANDAS) not_found_return("pandas not found; not building Python bindings.") endif () +set(BUILDING_PYTHON_BINDINGS ON) + # Nothing in this directory will be compiled into mlpack. set(BINDING_SOURCES default_param.hpp From 52809e95cf16463e02ff93061807192dc9f8fd16 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Mon, 13 May 2019 09:15:39 -0400 Subject: [PATCH 09/32] Fix other little CMake issues I failed to see. --- src/mlpack/CMakeLists.txt | 3 --- src/mlpack/bindings/CMakeLists.txt | 1 + src/mlpack/bindings/python/CMakeLists.txt | 4 ++-- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/mlpack/CMakeLists.txt b/src/mlpack/CMakeLists.txt index 6a496998714..b398d029c9a 100644 --- a/src/mlpack/CMakeLists.txt +++ b/src/mlpack/CMakeLists.txt @@ -137,9 +137,6 @@ if (BUILDING_PYTHON_BINDINGS) COMMENT "Configuring setup.py...") add_dependencies(python_configure python_copy) add_dependencies(python_configured python_configure) - -# configure_file(${CMAKE_SOURCE_DIR}/src/mlpack/bindings/python/setup.py.in -# ${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/setup.py) endif () # If we are building Markdown documentation, we have to run some setup after we diff --git a/src/mlpack/bindings/CMakeLists.txt b/src/mlpack/bindings/CMakeLists.txt index e726c515bd8..36c40bad9a5 100644 --- a/src/mlpack/bindings/CMakeLists.txt +++ b/src/mlpack/bindings/CMakeLists.txt @@ -14,3 +14,4 @@ set(MARKDOWN_CATEGORIES ${MARKDOWN_CATEGORIES} PARENT_SCOPE) set(MLPACK_SRCS ${MLPACK_SRCS} PARENT_SCOPE) set(MLPACK_PYXS ${MLPACK_PYXS} PARENT_SCOPE) set(DISABLE_CFLAGS ${DISABLE_CFLAGS} PARENT_SCOPE) +set(BUILDING_PYTHON_BINDINGS ${BUILDING_PYTHON_BINDINGS} PARENT_SCOPE) diff --git a/src/mlpack/bindings/python/CMakeLists.txt b/src/mlpack/bindings/python/CMakeLists.txt index a2e4c7f75cf..01395392e6b 100644 --- a/src/mlpack/bindings/python/CMakeLists.txt +++ b/src/mlpack/bindings/python/CMakeLists.txt @@ -40,7 +40,7 @@ if (NOT PY_PANDAS) not_found_return("pandas not found; not building Python bindings.") endif () -set(BUILDING_PYTHON_BINDINGS ON) +set(BUILDING_PYTHON_BINDINGS ON PARENT_SCOPE) # Nothing in this directory will be compiled into mlpack. set(BINDING_SOURCES @@ -141,7 +141,7 @@ if (BUILD_TESTS) endif () # Install any dependencies via setuptools automatically. -add_custom_command(TARGET python_copy POST_BUILD +add_custom_command(TARGET python_configured POST_BUILD COMMAND ${CMAKE_COMMAND} -E env NO_BUILD=1 ${PYTHON_EXECUTABLE} ${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/setup.py build WORKING_DIRECTORY ${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/) From ed6e3f57b600e70a64dc398723a49f49a60570d5 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Mon, 13 May 2019 16:48:14 -0400 Subject: [PATCH 10/32] Oops, forgot to check this in. --- src/mlpack/bindings/python/ConfigureSetup.cmake | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 src/mlpack/bindings/python/ConfigureSetup.cmake diff --git a/src/mlpack/bindings/python/ConfigureSetup.cmake b/src/mlpack/bindings/python/ConfigureSetup.cmake new file mode 100644 index 00000000000..8ee90aed5ee --- /dev/null +++ b/src/mlpack/bindings/python/ConfigureSetup.cmake @@ -0,0 +1,17 @@ +# ConfigureSetup.cmake: generate the setup.py file given several environment +# variables. +# +# This file depends on the following variables being set: +# +# - SETUP_PY_IN: location of input file +# - SETUP_PY_OUT: location of output file +# - PACKAGE_VERSION: version of package +# - Boost_SERIALIZATION_LIBRARY: location of Boost serialization library +# - MLPACK_LIBRARY: location of mlpack library +# - MLPACK_PYXS: list of pyx files +# - OpenMP_CXX_FLAGS: OpenMP C++ compilation flags +# - DISABLE_CFLAGS: list of CFLAGS or CXXFLAGS to be disabled +# - CYTHON_INCLUDE_DIRECTORIES: include directories for Cython +# - OUTPUT_DIR: binary output directory for CMake +message(STATUS "OUTPUT_DIR is ${OUTPUT_DIR}") +configure_file(${SETUP_PY_IN} ${SETUP_PY_OUT}) From 267eb51d45dc833b1b52caa488e54fc32f866cd7 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Thu, 16 May 2019 19:34:23 -0400 Subject: [PATCH 11/32] Another attempt to make the Windows build work. --- src/mlpack/CMakeLists.txt | 4 +++- src/mlpack/bindings/python/ConfigureSetup.cmake | 14 +++++++++++++- src/mlpack/bindings/python/setup.py.in | 10 ++++++++-- 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/mlpack/CMakeLists.txt b/src/mlpack/CMakeLists.txt index b398d029c9a..2806f77331f 100644 --- a/src/mlpack/CMakeLists.txt +++ b/src/mlpack/CMakeLists.txt @@ -126,11 +126,13 @@ if (BUILDING_PYTHON_BINDINGS) -D SETUP_PY_OUT="${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/setup.py" -D PACKAGE_VERSION="${PACKAGE_VERSION}" -D Boost_SERIALIZATION_LIBRARY="${Boost_SERIALIZATION_LIBRARY}" - -D MLPACK_LIBRARY="$" + -D MLPACK_LIBRARY="$" -D MLPACK_PYXS="${MLPACK_PYXS}" -D OpenMP_CXX_FLAGS="${OpenMP_CXX_FLAGS}" -D DISABLE_CFLAGS="${DISABLE_CFLAGS}" -D CYTHON_INCLUDE_DIRECTORIES="${CYTHON_INCLUDE_DIRECTORIES}" + -D MLPACK_LIBDIR="$" + -D BOOST_LIBDIRS="${BOOST_LIBRARY_DIRS}" -D OUTPUT_DIR="${CMAKE_BINARY_DIR}" -P "${CMAKE_SOURCE_DIR}/src/mlpack/bindings/python/ConfigureSetup.cmake" BYPRODUCTS "${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/setup.py" diff --git a/src/mlpack/bindings/python/ConfigureSetup.cmake b/src/mlpack/bindings/python/ConfigureSetup.cmake index 8ee90aed5ee..bfd46d09132 100644 --- a/src/mlpack/bindings/python/ConfigureSetup.cmake +++ b/src/mlpack/bindings/python/ConfigureSetup.cmake @@ -12,6 +12,18 @@ # - OpenMP_CXX_FLAGS: OpenMP C++ compilation flags # - DISABLE_CFLAGS: list of CFLAGS or CXXFLAGS to be disabled # - CYTHON_INCLUDE_DIRECTORIES: include directories for Cython +# - MLPACK_LIBDIR: path to mlpack libraries +# - BOOST_LIBDIRS: paths to Boost libraries # - OUTPUT_DIR: binary output directory for CMake -message(STATUS "OUTPUT_DIR is ${OUTPUT_DIR}") + +get_filename_component(Boost_SERIALIZATION_LIB_NAME_IN + ${Boost_SERIALIZATION_LIBRARY} NAME_WE) +get_filename_component(MLPACK_LIB_NAME_IN + ${MLPACK_LIBRARY} NAME_WE) + +# Strip 'lib' off the front of the name, if it's there. +string(REGEX REPLACE "^lib" "" Boost_SERIALIZATION_LIB_NAME + "${Boost_SERIALIZATION_LIB_NAME_IN}") +string(REGEX REPLACE "^lib" "" MLPACK_LIB_NAME "${MLPACK_LIB_NAME_IN}") + configure_file(${SETUP_PY_IN} ${SETUP_PY_OUT}) diff --git a/src/mlpack/bindings/python/setup.py.in b/src/mlpack/bindings/python/setup.py.in index 6f46063df42..cda1e80272c 100644 --- a/src/mlpack/bindings/python/setup.py.in +++ b/src/mlpack/bindings/python/setup.py.in @@ -54,7 +54,10 @@ else: np.get_include(), \ '${OUTPUT_DIR}/src/mlpack/bindings/python/'] + '${CYTHON_INCLUDE_DIRECTORIES}'.split(' '), - libraries=['${MLPACK_LIBRARY}', '${Boost_SERIALIZATION_LIBRARY}'], + libraries=['${MLPACK_LIB_NAME}', + '${Boost_SERIALIZATION_LIB_NAME}'], + library_dirs=['${MLPACK_LIBDIR}'] + + '${BOOST_LIBDIRS}'.split(' '), # CMAKE_CXX_FLAGS seems to have an extra space. extra_compile_args=extra_args, extra_link_args=extra_link_args, @@ -70,7 +73,10 @@ else: np.get_include(), \ '${OUTPUT_DIR}/src/mlpack/bindings/python/'] + '${CYTHON_INCLUDE_DIRECTORIES}'.split(' '), - libraries=['${MLPACK_LIBRARY}', '${Boost_SERIALIZATION_LIBRARY}'], + libraries=['${MLPACK_LIB_NAME}', + '${Boost_SERIALIZATION_LIB_NAME}'], + library_dirs=['${MLPACK_LIBDIR}'] + + '${BOOST_LIBDIRS}'.split(' '), # CMAKE_CXX_FLAGS seems to have an extra space. extra_compile_args=extra_args, extra_link_args=extra_link_args, From 4906722f0e3e7d5c456d5c6edbd7705097424eef Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Thu, 16 May 2019 22:36:06 -0400 Subject: [PATCH 12/32] Remove empty library_dirs. --- src/mlpack/bindings/python/setup.py.in | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/mlpack/bindings/python/setup.py.in b/src/mlpack/bindings/python/setup.py.in index cda1e80272c..b129c3785fe 100644 --- a/src/mlpack/bindings/python/setup.py.in +++ b/src/mlpack/bindings/python/setup.py.in @@ -56,8 +56,8 @@ else: '${CYTHON_INCLUDE_DIRECTORIES}'.split(' '), libraries=['${MLPACK_LIB_NAME}', '${Boost_SERIALIZATION_LIB_NAME}'], - library_dirs=['${MLPACK_LIBDIR}'] + - '${BOOST_LIBDIRS}'.split(' '), + library_dirs=filter(None, ['${MLPACK_LIBDIR}'] + + '${BOOST_LIBDIRS}'.split(' ')), # CMAKE_CXX_FLAGS seems to have an extra space. extra_compile_args=extra_args, extra_link_args=extra_link_args, @@ -75,8 +75,8 @@ else: '${CYTHON_INCLUDE_DIRECTORIES}'.split(' '), libraries=['${MLPACK_LIB_NAME}', '${Boost_SERIALIZATION_LIB_NAME}'], - library_dirs=['${MLPACK_LIBDIR}'] + - '${BOOST_LIBDIRS}'.split(' '), + library_dirs=filter(None, ['${MLPACK_LIBDIR}'] + + '${BOOST_LIBDIRS}'.split(' ')), # CMAKE_CXX_FLAGS seems to have an extra space. extra_compile_args=extra_args, extra_link_args=extra_link_args, From a44fdfe8c608b6afe52d11963c093748101f966c Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Thu, 16 May 2019 22:39:28 -0400 Subject: [PATCH 13/32] Set Boost library directories properly. --- src/mlpack/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mlpack/CMakeLists.txt b/src/mlpack/CMakeLists.txt index 2806f77331f..fd1486dc815 100644 --- a/src/mlpack/CMakeLists.txt +++ b/src/mlpack/CMakeLists.txt @@ -132,7 +132,7 @@ if (BUILDING_PYTHON_BINDINGS) -D DISABLE_CFLAGS="${DISABLE_CFLAGS}" -D CYTHON_INCLUDE_DIRECTORIES="${CYTHON_INCLUDE_DIRECTORIES}" -D MLPACK_LIBDIR="$" - -D BOOST_LIBDIRS="${BOOST_LIBRARY_DIRS}" + -D BOOST_LIBDIRS="${Boost_LIBRARY_DIRS}" -D OUTPUT_DIR="${CMAKE_BINARY_DIR}" -P "${CMAKE_SOURCE_DIR}/src/mlpack/bindings/python/ConfigureSetup.cmake" BYPRODUCTS "${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/setup.py" From 34d6bd77a6ccf8bc58e0d9e1b55613f00e3460f8 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Sat, 1 Jun 2019 01:12:10 -0400 Subject: [PATCH 14/32] Make library_dirs None when it's empty. --- src/mlpack/bindings/python/setup.py.in | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/mlpack/bindings/python/setup.py.in b/src/mlpack/bindings/python/setup.py.in index b129c3785fe..dcef00c4c79 100644 --- a/src/mlpack/bindings/python/setup.py.in +++ b/src/mlpack/bindings/python/setup.py.in @@ -29,6 +29,12 @@ if not '${OpenMP_CXX_FLAGS}': else: extra_link_args=['${OpenMP_CXX_FLAGS}'] +# Get list of library dirs. +library_dirs = filter(None, ['${MLPACK_LIBDIR}'] + + '${BOOST_LIBDIRS}'.split(' ')) +if library_dirs == []: + library_dirs = None + # Only build the extensions if we are asked to. if os.getenv('NO_BUILD') == '1': modules = [] @@ -56,8 +62,7 @@ else: '${CYTHON_INCLUDE_DIRECTORIES}'.split(' '), libraries=['${MLPACK_LIB_NAME}', '${Boost_SERIALIZATION_LIB_NAME}'], - library_dirs=filter(None, ['${MLPACK_LIBDIR}'] + - '${BOOST_LIBDIRS}'.split(' ')), + library_dirs=library_dirs, # CMAKE_CXX_FLAGS seems to have an extra space. extra_compile_args=extra_args, extra_link_args=extra_link_args, @@ -75,8 +80,7 @@ else: '${CYTHON_INCLUDE_DIRECTORIES}'.split(' '), libraries=['${MLPACK_LIB_NAME}', '${Boost_SERIALIZATION_LIB_NAME}'], - library_dirs=filter(None, ['${MLPACK_LIBDIR}'] + - '${BOOST_LIBDIRS}'.split(' ')), + library_dirs=library_dirs, # CMAKE_CXX_FLAGS seems to have an extra space. extra_compile_args=extra_args, extra_link_args=extra_link_args, From 87e174b0cc4a0e464bd835b80f1fa26041da7d13 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Thu, 6 Jun 2019 20:56:19 -0400 Subject: [PATCH 15/32] Temporarily comment out filtering for Windows build. --- src/mlpack/bindings/python/setup.py.in | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/mlpack/bindings/python/setup.py.in b/src/mlpack/bindings/python/setup.py.in index dcef00c4c79..c57007f2b98 100644 --- a/src/mlpack/bindings/python/setup.py.in +++ b/src/mlpack/bindings/python/setup.py.in @@ -30,10 +30,13 @@ else: extra_link_args=['${OpenMP_CXX_FLAGS}'] # Get list of library dirs. -library_dirs = filter(None, ['${MLPACK_LIBDIR}'] + - '${BOOST_LIBDIRS}'.split(' ')) -if library_dirs == []: - library_dirs = None +#library_dirs = filter(None, ['${MLPACK_LIBDIR}'] + +# '${BOOST_LIBDIRS}'.split(' ')) +#if library_dirs == []: +# library_dirs = None + +# Specific to the debugging for #1872. +library_dirs = ['${MLPACK_LIBDIR}', '${BOOST_LIBDIRS}'] # Only build the extensions if we are asked to. if os.getenv('NO_BUILD') == '1': From 1bb542c70e4932f5b3df93f3f6f7c82c0315d946 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Sat, 15 Jun 2019 16:02:29 -0400 Subject: [PATCH 16/32] Link against Armadillo dependencies. --- src/mlpack/CMakeLists.txt | 1 + .../bindings/python/ConfigureSetup.cmake | 21 ++++++++++++++++++- src/mlpack/bindings/python/setup.py.in | 9 +++++--- 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/src/mlpack/CMakeLists.txt b/src/mlpack/CMakeLists.txt index fd1486dc815..6d2f3adf5d6 100644 --- a/src/mlpack/CMakeLists.txt +++ b/src/mlpack/CMakeLists.txt @@ -126,6 +126,7 @@ if (BUILDING_PYTHON_BINDINGS) -D SETUP_PY_OUT="${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/setup.py" -D PACKAGE_VERSION="${PACKAGE_VERSION}" -D Boost_SERIALIZATION_LIBRARY="${Boost_SERIALIZATION_LIBRARY}" + -D ARMADILLO_LIBRARIES="${ARMADILLO_LIBRARIES}" -D MLPACK_LIBRARY="$" -D MLPACK_PYXS="${MLPACK_PYXS}" -D OpenMP_CXX_FLAGS="${OpenMP_CXX_FLAGS}" diff --git a/src/mlpack/bindings/python/ConfigureSetup.cmake b/src/mlpack/bindings/python/ConfigureSetup.cmake index bfd46d09132..f1acad10591 100644 --- a/src/mlpack/bindings/python/ConfigureSetup.cmake +++ b/src/mlpack/bindings/python/ConfigureSetup.cmake @@ -7,6 +7,7 @@ # - SETUP_PY_OUT: location of output file # - PACKAGE_VERSION: version of package # - Boost_SERIALIZATION_LIBRARY: location of Boost serialization library +# - ARMADILLO_LIBRARIES: space-separated list of Armadillo dependencies # - MLPACK_LIBRARY: location of mlpack library # - MLPACK_PYXS: list of pyx files # - OpenMP_CXX_FLAGS: OpenMP C++ compilation flags @@ -15,7 +16,6 @@ # - MLPACK_LIBDIR: path to mlpack libraries # - BOOST_LIBDIRS: paths to Boost libraries # - OUTPUT_DIR: binary output directory for CMake - get_filename_component(Boost_SERIALIZATION_LIB_NAME_IN ${Boost_SERIALIZATION_LIBRARY} NAME_WE) get_filename_component(MLPACK_LIB_NAME_IN @@ -26,4 +26,23 @@ string(REGEX REPLACE "^lib" "" Boost_SERIALIZATION_LIB_NAME "${Boost_SERIALIZATION_LIB_NAME_IN}") string(REGEX REPLACE "^lib" "" MLPACK_LIB_NAME "${MLPACK_LIB_NAME_IN}") +# There may be multiple Armadillo libraries, so first convert the string we got +# to a list. +string(REPLACE " " ";" ARMADILLO_LIBRARIES_LIST "${ARMADILLO_LIBRARIES}") +set(ARMADILLO_LIBRARIES "") +set(ARMADILLO_LIBDIRS "") +foreach(l ${ARMADILLO_LIBRARIES_LIST}) + get_filename_component(l_in "${l}" NAME_WE) + get_filename_component(l_dir "${l}" DIRECTORY) + string(REGEX REPLACE "^[ ]*lib" "" l_out "${l_in}") + string(STRIP "${l_dir}" l_dir_out) + set(ARMADILLO_LIBRARIES "${ARMADILLO_LIBRARIES} ${l_out}") + set(ARMADILLO_LIBDIRS "${ARMADILLO_LIBDIRS} ${l_dir_out}") +endforeach() + +string(STRIP "${ARMADILLO_LIBRARIES}" tmp) +set(ARMADILLO_LIBRARIES "${tmp}") +string(STRIP "${ARMADILLO_LIBDIRS}" tmp) +set(ARMADILLO_LIBDIRS "${tmp}") + configure_file(${SETUP_PY_IN} ${SETUP_PY_OUT}) diff --git a/src/mlpack/bindings/python/setup.py.in b/src/mlpack/bindings/python/setup.py.in index c57007f2b98..3ef2986ff57 100644 --- a/src/mlpack/bindings/python/setup.py.in +++ b/src/mlpack/bindings/python/setup.py.in @@ -36,7 +36,8 @@ else: # library_dirs = None # Specific to the debugging for #1872. -library_dirs = ['${MLPACK_LIBDIR}', '${BOOST_LIBDIRS}'] +library_dirs = ['${MLPACK_LIBDIR}', '${BOOST_LIBDIRS}'] + \ + '${ARMADILLO_LIBDIRS}'.split(' ') # Only build the extensions if we are asked to. if os.getenv('NO_BUILD') == '1': @@ -64,7 +65,8 @@ else: '${OUTPUT_DIR}/src/mlpack/bindings/python/'] + '${CYTHON_INCLUDE_DIRECTORIES}'.split(' '), libraries=['${MLPACK_LIB_NAME}', - '${Boost_SERIALIZATION_LIB_NAME}'], + '${Boost_SERIALIZATION_LIB_NAME}'] + + '${ARMADILLO_LIBRARIES}'.split(' '), library_dirs=library_dirs, # CMAKE_CXX_FLAGS seems to have an extra space. extra_compile_args=extra_args, @@ -82,7 +84,8 @@ else: '${OUTPUT_DIR}/src/mlpack/bindings/python/'] + '${CYTHON_INCLUDE_DIRECTORIES}'.split(' '), libraries=['${MLPACK_LIB_NAME}', - '${Boost_SERIALIZATION_LIB_NAME}'], + '${Boost_SERIALIZATION_LIB_NAME}'] + + '${ARMADILLO_LIBRARIES}'.split(' '), library_dirs=library_dirs, # CMAKE_CXX_FLAGS seems to have an extra space. extra_compile_args=extra_args, From 75f816bd24b01b42f88e763879016dfb00c3e99e Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Mon, 17 Jun 2019 16:15:16 -0400 Subject: [PATCH 17/32] Fix character encoding issue. --- src/mlpack/methods/kernel_pca/kernel_pca_main.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mlpack/methods/kernel_pca/kernel_pca_main.cpp b/src/mlpack/methods/kernel_pca/kernel_pca_main.cpp index 11765e53e80..66d5c7bb933 100644 --- a/src/mlpack/methods/kernel_pca/kernel_pca_main.cpp +++ b/src/mlpack/methods/kernel_pca/kernel_pca_main.cpp @@ -91,7 +91,7 @@ PROGRAM_INFO("Kernel Principal Components Analysis", PRINT_PARAM_STRING("offset") + ", or " + PRINT_PARAM_STRING("degree") + " (or a combination of those parameters)." "\n\n" - "Optionally, the Nystr\u00F6m method (\"Using the Nystroem method to speed " + "Optionally, the Nystroem method (\"Using the Nystroem method to speed " "up kernel machines\", 2001) can be used to calculate the kernel matrix by " "specifying the " + PRINT_PARAM_STRING("nystroem_method") + " parameter. " "This approach works by using a subset of the data as basis to reconstruct " From 68148389edbe99a615ebced47c3fccc449f4e2c5 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Mon, 17 Jun 2019 21:14:30 -0400 Subject: [PATCH 18/32] Fix other encoding issue. --- src/mlpack/methods/kernel_pca/kernel_pca_main.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mlpack/methods/kernel_pca/kernel_pca_main.cpp b/src/mlpack/methods/kernel_pca/kernel_pca_main.cpp index 66d5c7bb933..88e543e6183 100644 --- a/src/mlpack/methods/kernel_pca/kernel_pca_main.cpp +++ b/src/mlpack/methods/kernel_pca/kernel_pca_main.cpp @@ -97,7 +97,7 @@ PROGRAM_INFO("Kernel Principal Components Analysis", "This approach works by using a subset of the data as basis to reconstruct " "the kernel matrix; to specify the sampling scheme, the " + PRINT_PARAM_STRING("sampling") + " parameter is used. The " - "sampling scheme for the Nystr\u00F6m method can be chosen from the " + "sampling scheme for the Nystroem method can be chosen from the " "following list: 'kmeans', 'random', 'ordered'.", SEE_ALSO("Kernel principal component analysis on Wikipedia", "https://en.wikipedia.org/wiki/Kernel_principal_component_analysis"), From 9ec264dbb22dd7346af6ebde8970a3a2144ac486 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Tue, 18 Jun 2019 22:38:12 -0400 Subject: [PATCH 19/32] Try specifying libraries directly via extra_link_args. I'm not yet sure if this will work on Windows. --- src/mlpack/CMakeLists.txt | 4 +-- .../bindings/python/ConfigureSetup.cmake | 28 ------------------- src/mlpack/bindings/python/setup.py.in | 21 +++++++------- 3 files changed, 11 insertions(+), 42 deletions(-) diff --git a/src/mlpack/CMakeLists.txt b/src/mlpack/CMakeLists.txt index 6d2f3adf5d6..8e978dc0799 100644 --- a/src/mlpack/CMakeLists.txt +++ b/src/mlpack/CMakeLists.txt @@ -127,13 +127,11 @@ if (BUILDING_PYTHON_BINDINGS) -D PACKAGE_VERSION="${PACKAGE_VERSION}" -D Boost_SERIALIZATION_LIBRARY="${Boost_SERIALIZATION_LIBRARY}" -D ARMADILLO_LIBRARIES="${ARMADILLO_LIBRARIES}" - -D MLPACK_LIBRARY="$" + -D MLPACK_LIBRARY="$" -D MLPACK_PYXS="${MLPACK_PYXS}" -D OpenMP_CXX_FLAGS="${OpenMP_CXX_FLAGS}" -D DISABLE_CFLAGS="${DISABLE_CFLAGS}" -D CYTHON_INCLUDE_DIRECTORIES="${CYTHON_INCLUDE_DIRECTORIES}" - -D MLPACK_LIBDIR="$" - -D BOOST_LIBDIRS="${Boost_LIBRARY_DIRS}" -D OUTPUT_DIR="${CMAKE_BINARY_DIR}" -P "${CMAKE_SOURCE_DIR}/src/mlpack/bindings/python/ConfigureSetup.cmake" BYPRODUCTS "${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/setup.py" diff --git a/src/mlpack/bindings/python/ConfigureSetup.cmake b/src/mlpack/bindings/python/ConfigureSetup.cmake index f1acad10591..81245950f38 100644 --- a/src/mlpack/bindings/python/ConfigureSetup.cmake +++ b/src/mlpack/bindings/python/ConfigureSetup.cmake @@ -16,33 +16,5 @@ # - MLPACK_LIBDIR: path to mlpack libraries # - BOOST_LIBDIRS: paths to Boost libraries # - OUTPUT_DIR: binary output directory for CMake -get_filename_component(Boost_SERIALIZATION_LIB_NAME_IN - ${Boost_SERIALIZATION_LIBRARY} NAME_WE) -get_filename_component(MLPACK_LIB_NAME_IN - ${MLPACK_LIBRARY} NAME_WE) - -# Strip 'lib' off the front of the name, if it's there. -string(REGEX REPLACE "^lib" "" Boost_SERIALIZATION_LIB_NAME - "${Boost_SERIALIZATION_LIB_NAME_IN}") -string(REGEX REPLACE "^lib" "" MLPACK_LIB_NAME "${MLPACK_LIB_NAME_IN}") - -# There may be multiple Armadillo libraries, so first convert the string we got -# to a list. -string(REPLACE " " ";" ARMADILLO_LIBRARIES_LIST "${ARMADILLO_LIBRARIES}") -set(ARMADILLO_LIBRARIES "") -set(ARMADILLO_LIBDIRS "") -foreach(l ${ARMADILLO_LIBRARIES_LIST}) - get_filename_component(l_in "${l}" NAME_WE) - get_filename_component(l_dir "${l}" DIRECTORY) - string(REGEX REPLACE "^[ ]*lib" "" l_out "${l_in}") - string(STRIP "${l_dir}" l_dir_out) - set(ARMADILLO_LIBRARIES "${ARMADILLO_LIBRARIES} ${l_out}") - set(ARMADILLO_LIBDIRS "${ARMADILLO_LIBDIRS} ${l_dir_out}") -endforeach() - -string(STRIP "${ARMADILLO_LIBRARIES}" tmp) -set(ARMADILLO_LIBRARIES "${tmp}") -string(STRIP "${ARMADILLO_LIBDIRS}" tmp) -set(ARMADILLO_LIBDIRS "${tmp}") configure_file(${SETUP_PY_IN} ${SETUP_PY_OUT}) diff --git a/src/mlpack/bindings/python/setup.py.in b/src/mlpack/bindings/python/setup.py.in index 3ef2986ff57..a881015cc73 100644 --- a/src/mlpack/bindings/python/setup.py.in +++ b/src/mlpack/bindings/python/setup.py.in @@ -35,9 +35,16 @@ else: #if library_dirs == []: # library_dirs = None -# Specific to the debugging for #1872. -library_dirs = ['${MLPACK_LIBDIR}', '${BOOST_LIBDIRS}'] + \ - '${ARMADILLO_LIBDIRS}'.split(' ') +# We'll link with the exact paths to each library using extra_objects, instead +# of linking with 'libraries' and 'library_dirs', because of differences in +# Windows and Linux linking behavior. +libraries = ['${MLPACK_LIBRARY}', + '${Boost_SERIALIZATION_LIBRARY}'] + \ + '${ARMADILLO_LIBRARIES}'.split(' ') + +# Potentially faulty assumption: we can always link against libraries directly +# by just specifying the full path to them on the command line. +extra_link_args += libraries # Only build the extensions if we are asked to. if os.getenv('NO_BUILD') == '1': @@ -64,10 +71,6 @@ else: np.get_include(), \ '${OUTPUT_DIR}/src/mlpack/bindings/python/'] + '${CYTHON_INCLUDE_DIRECTORIES}'.split(' '), - libraries=['${MLPACK_LIB_NAME}', - '${Boost_SERIALIZATION_LIB_NAME}'] + - '${ARMADILLO_LIBRARIES}'.split(' '), - library_dirs=library_dirs, # CMAKE_CXX_FLAGS seems to have an extra space. extra_compile_args=extra_args, extra_link_args=extra_link_args, @@ -83,10 +86,6 @@ else: np.get_include(), \ '${OUTPUT_DIR}/src/mlpack/bindings/python/'] + '${CYTHON_INCLUDE_DIRECTORIES}'.split(' '), - libraries=['${MLPACK_LIB_NAME}', - '${Boost_SERIALIZATION_LIB_NAME}'] + - '${ARMADILLO_LIBRARIES}'.split(' '), - library_dirs=library_dirs, # CMAKE_CXX_FLAGS seems to have an extra space. extra_compile_args=extra_args, extra_link_args=extra_link_args, From 632c9cdcfb288ddf55b2b11f9e46bd45a8d3998e Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Wed, 26 Jun 2019 18:48:10 -0400 Subject: [PATCH 20/32] I hope that maybe on Windows this *just* gives the release library... --- src/mlpack/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mlpack/CMakeLists.txt b/src/mlpack/CMakeLists.txt index 8e978dc0799..a1584656501 100644 --- a/src/mlpack/CMakeLists.txt +++ b/src/mlpack/CMakeLists.txt @@ -125,7 +125,7 @@ if (BUILDING_PYTHON_BINDINGS) -D SETUP_PY_IN="${CMAKE_SOURCE_DIR}/src/mlpack/bindings/python/setup.py.in" -D SETUP_PY_OUT="${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/setup.py" -D PACKAGE_VERSION="${PACKAGE_VERSION}" - -D Boost_SERIALIZATION_LIBRARY="${Boost_SERIALIZATION_LIBRARY}" + -D Boost_SERIALIZATION_LIBRARY="${Boost_SERIALIZATION_LIBRARY_RELEASE}" -D ARMADILLO_LIBRARIES="${ARMADILLO_LIBRARIES}" -D MLPACK_LIBRARY="$" -D MLPACK_PYXS="${MLPACK_PYXS}" From 0573ef7752099284f73d7167f3f549a51b39f2cd Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Sun, 30 Jun 2019 12:39:21 -0400 Subject: [PATCH 21/32] Try stripping any unnecessary lib/ from the front. --- .../bindings/python/ConfigureSetup.cmake | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/mlpack/bindings/python/ConfigureSetup.cmake b/src/mlpack/bindings/python/ConfigureSetup.cmake index 81245950f38..1c23288752d 100644 --- a/src/mlpack/bindings/python/ConfigureSetup.cmake +++ b/src/mlpack/bindings/python/ConfigureSetup.cmake @@ -17,4 +17,25 @@ # - BOOST_LIBDIRS: paths to Boost libraries # - OUTPUT_DIR: binary output directory for CMake +# It's possible that the FindBoost CMake script may have returned a Boost +# library with "lib" improperly prepended to it. So we need to see if the file +# exists, and if it doesn't, but it has a "lib" in it, then we will try +# stripping the "lib" off the front. +if (NOT EXISTS "$(Boost_SERIALIZATION_LIBRARY}") + # Split the filename to see if it starts with lib. + set(Boost_SERIALIZATION_LIBRARY_ORIG "${Boost_SERIALIZATION_LIBRARY}") + get_filename_component(SER_LIB_DIRECTORY "${Boost_SERIALIZATION_LIBRARY}" + DIRECTORY) + get_filename_component(SER_LIB_FILENAME "${Boost_SERIALIZATION_LIBRARY}" NAME) + + # Strip any preceding "lib/". + string(REGEX REPLACE "^lib" "" STRIPPED_FILENAME "${SER_LIB_FILENAME}") + set(Boost_SERIALIZATION_LIBRARY "${SER_LIB_DIRECTORY}/${STRIPPED_FILENAME}") + + if (NOT EXISTS "${Boost_SERIALIZATION_LIBRARY}") + # We didn't find it, so for ease of debugging just revert to the original. + set (Boost_SERIALIZATION_LIBRARY "${Boost_SERIALIZATION_LIBRARY_ORIG}") + endif () +endif () + configure_file(${SETUP_PY_IN} ${SETUP_PY_OUT}) From 8154e4f6f47657afcf0f46fc1303bcf902186aa1 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Thu, 29 Aug 2019 21:10:18 -0400 Subject: [PATCH 22/32] Update configuration to set right library dirs. --- src/mlpack/CMakeLists.txt | 4 +++- src/mlpack/bindings/python/ConfigureSetup.cmake | 9 +++++++-- src/mlpack/bindings/python/setup.py.in | 9 +++++---- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/mlpack/CMakeLists.txt b/src/mlpack/CMakeLists.txt index a1584656501..509f8c8f836 100644 --- a/src/mlpack/CMakeLists.txt +++ b/src/mlpack/CMakeLists.txt @@ -126,8 +126,10 @@ if (BUILDING_PYTHON_BINDINGS) -D SETUP_PY_OUT="${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/setup.py" -D PACKAGE_VERSION="${PACKAGE_VERSION}" -D Boost_SERIALIZATION_LIBRARY="${Boost_SERIALIZATION_LIBRARY_RELEASE}" + -D Boost_LIBRARY_DIRS="${Boost_LIBRARY_DIRS}" -D ARMADILLO_LIBRARIES="${ARMADILLO_LIBRARIES}" - -D MLPACK_LIBRARY="$" + -D MLPACK_LIBRARY="$" + -D MLPACK_LIBDIR="$" -D MLPACK_PYXS="${MLPACK_PYXS}" -D OpenMP_CXX_FLAGS="${OpenMP_CXX_FLAGS}" -D DISABLE_CFLAGS="${DISABLE_CFLAGS}" diff --git a/src/mlpack/bindings/python/ConfigureSetup.cmake b/src/mlpack/bindings/python/ConfigureSetup.cmake index 1c23288752d..f981a4c40a7 100644 --- a/src/mlpack/bindings/python/ConfigureSetup.cmake +++ b/src/mlpack/bindings/python/ConfigureSetup.cmake @@ -7,6 +7,7 @@ # - SETUP_PY_OUT: location of output file # - PACKAGE_VERSION: version of package # - Boost_SERIALIZATION_LIBRARY: location of Boost serialization library +# - Boost_LIBRARY_DIRS: paths to boost libraries # - ARMADILLO_LIBRARIES: space-separated list of Armadillo dependencies # - MLPACK_LIBRARY: location of mlpack library # - MLPACK_PYXS: list of pyx files @@ -14,23 +15,27 @@ # - DISABLE_CFLAGS: list of CFLAGS or CXXFLAGS to be disabled # - CYTHON_INCLUDE_DIRECTORIES: include directories for Cython # - MLPACK_LIBDIR: path to mlpack libraries -# - BOOST_LIBDIRS: paths to Boost libraries # - OUTPUT_DIR: binary output directory for CMake # It's possible that the FindBoost CMake script may have returned a Boost # library with "lib" improperly prepended to it. So we need to see if the file # exists, and if it doesn't, but it has a "lib" in it, then we will try # stripping the "lib" off the front. -if (NOT EXISTS "$(Boost_SERIALIZATION_LIBRARY}") +message(STATUS "Run with ${Boost_SERIALIZATION_LIBRARY}.") +if (NOT EXISTS "${Boost_SERIALIZATION_LIBRARY}") + message(STATUS "Did not find serialization library ${Boost_SERIALIZATION_LIBRARY}!") # Split the filename to see if it starts with lib. set(Boost_SERIALIZATION_LIBRARY_ORIG "${Boost_SERIALIZATION_LIBRARY}") get_filename_component(SER_LIB_DIRECTORY "${Boost_SERIALIZATION_LIBRARY}" DIRECTORY) get_filename_component(SER_LIB_FILENAME "${Boost_SERIALIZATION_LIBRARY}" NAME) + message(STATUS "Name component is ${SER_LIB_FILENAME}, and directory is ${SER_LIB_DIRECTORY}.") # Strip any preceding "lib/". string(REGEX REPLACE "^lib" "" STRIPPED_FILENAME "${SER_LIB_FILENAME}") + message(STATUS "Regex gave us ${STRIPPED_FILENAME}.") set(Boost_SERIALIZATION_LIBRARY "${SER_LIB_DIRECTORY}/${STRIPPED_FILENAME}") + message(STATUS "New library ${Boost_SERIALIZATION_LIBRARY}.") if (NOT EXISTS "${Boost_SERIALIZATION_LIBRARY}") # We didn't find it, so for ease of debugging just revert to the original. diff --git a/src/mlpack/bindings/python/setup.py.in b/src/mlpack/bindings/python/setup.py.in index a881015cc73..207559fe57b 100644 --- a/src/mlpack/bindings/python/setup.py.in +++ b/src/mlpack/bindings/python/setup.py.in @@ -30,10 +30,8 @@ else: extra_link_args=['${OpenMP_CXX_FLAGS}'] # Get list of library dirs. -#library_dirs = filter(None, ['${MLPACK_LIBDIR}'] + -# '${BOOST_LIBDIRS}'.split(' ')) -#if library_dirs == []: -# library_dirs = None +library_dirs = list(filter(None, ['${MLPACK_LIBDIR}'] + + '${Boost_LIBRARY_DIRS}'.split(' '))) # We'll link with the exact paths to each library using extra_objects, instead # of linking with 'libraries' and 'library_dirs', because of differences in @@ -60,6 +58,7 @@ else: extra_args = ['-DBINDING_TYPE=BINDING_TYPE_PYX', '-std=c++11', '${OpenMP_CXX_FLAGS}'] + extra_args = extra_args + ['/MD', '/O2', '/Ob2', '/DNDEBUG'] # This is used for parallel builds; CMake will set PYX_TO_BUILD accordingly. if module is not None: @@ -71,6 +70,7 @@ else: np.get_include(), \ '${OUTPUT_DIR}/src/mlpack/bindings/python/'] + '${CYTHON_INCLUDE_DIRECTORIES}'.split(' '), + library_dirs=library_dirs, # CMAKE_CXX_FLAGS seems to have an extra space. extra_compile_args=extra_args, extra_link_args=extra_link_args, @@ -86,6 +86,7 @@ else: np.get_include(), \ '${OUTPUT_DIR}/src/mlpack/bindings/python/'] + '${CYTHON_INCLUDE_DIRECTORIES}'.split(' '), + library_dirs=library_dirs, # CMAKE_CXX_FLAGS seems to have an extra space. extra_compile_args=extra_args, extra_link_args=extra_link_args, From a2cf05e9748b427bc533ea048fb7aa7d3beb06d9 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Thu, 29 Aug 2019 21:11:52 -0400 Subject: [PATCH 23/32] Fix spacing for some input options. --- .../python/print_input_processing.hpp | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/mlpack/bindings/python/print_input_processing.hpp b/src/mlpack/bindings/python/print_input_processing.hpp index 688c9077821..dc9f903bf2a 100644 --- a/src/mlpack/bindings/python/print_input_processing.hpp +++ b/src/mlpack/bindings/python/print_input_processing.hpp @@ -323,44 +323,44 @@ void PrintInputProcessing( { if (T::is_row || T::is_col) { - std::cout << prefix << " " << d.name << "_tuple = to_matrix(" + std::cout << prefix << d.name << "_tuple = to_matrix(" << d.name << ", dtype=" << GetNumpyType() << ", copy=CLI.HasParam('copy_all_inputs'))" << std::endl; - std::cout << prefix << " if len(" << d.name << "_tuple[0].shape) > 1:" + std::cout << prefix << "if len(" << d.name << "_tuple[0].shape) > 1:" << std::endl; - std::cout << prefix << " if " << d.name << "_tuple[0]" + std::cout << prefix << " if " << d.name << "_tuple[0]" << ".shape[0] == 1 or " << d.name << "_tuple[0].shape[1] == 1:" << std::endl; - std::cout << prefix << " " << d.name << "_tuple[0].shape = (" + std::cout << prefix << " " << d.name << "_tuple[0].shape = (" << d.name << "_tuple[0].size,)" << std::endl; - std::cout << prefix << " " << d.name << "_mat = arma_numpy.numpy_to_" + std::cout << prefix << d.name << "_mat = arma_numpy.numpy_to_" << GetArmaType() << "_" << GetNumpyTypeChar() << "(" << d.name << "_tuple[0], " << d.name << "_tuple[1])" << std::endl; - std::cout << prefix << " SetParam[" << GetCythonType(d) + std::cout << prefix << "SetParam[" << GetCythonType(d) << "]( '" << d.name << "', dereference(" << d.name << "_mat))"<< std::endl; - std::cout << prefix << " CLI.SetPassed( '" << d.name + std::cout << prefix << "CLI.SetPassed( '" << d.name << "')" << std::endl; - std::cout << prefix << " del " << d.name << "_mat" << std::endl; + std::cout << prefix << "del " << d.name << "_mat" << std::endl; } else { - std::cout << prefix << " " << d.name << "_tuple = to_matrix(" + std::cout << prefix << d.name << "_tuple = to_matrix(" << d.name << ", dtype=" << GetNumpyType() << ", copy=CLI.HasParam('copy_all_inputs'))" << std::endl; - std::cout << prefix << " if len(" << d.name << "_tuple[0].shape) > 2:" + std::cout << prefix << "if len(" << d.name << "_tuple[0].shape) > 2:" << std::endl; - std::cout << prefix << " " << d.name << "_tuple[0].shape = (" << d.name + std::cout << prefix << " " << d.name << "_tuple[0].shape = (" << d.name << "_tuple[0].shape[0], 1)" << std::endl; - std::cout << prefix << " " << d.name << "_mat = arma_numpy.numpy_to_" + std::cout << prefix << d.name << "_mat = arma_numpy.numpy_to_" << GetArmaType() << "_" << GetNumpyTypeChar() << "(" << d.name << "_tuple[0], " << d.name << "_tuple[1])" << std::endl; - std::cout << prefix << " SetParam[" << GetCythonType(d) + std::cout << prefix << "SetParam[" << GetCythonType(d) << "]( '" << d.name << "', dereference(" << d.name << "_mat))"<< std::endl; - std::cout << prefix << " CLI.SetPassed( '" << d.name + std::cout << prefix << "CLI.SetPassed( '" << d.name << "')" << std::endl; - std::cout << prefix << " del " << d.name << "_mat" << std::endl; + std::cout << prefix << "del " << d.name << "_mat" << std::endl; } } std::cout << std::endl; From ea2c4bcf8f600fb45d77fcdf018ab7841486a6ca Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Thu, 29 Aug 2019 21:24:02 -0400 Subject: [PATCH 24/32] Always copy memory on Windows. It seems we need to avoid passing memory pointers back and forth---it seems like on Windows, anything allocated in Python cannot be freed in C++, and vice versa. --- .../bindings/python/mlpack/arma_numpy.pyx | 84 ++++++++++++------- 1 file changed, 53 insertions(+), 31 deletions(-) diff --git a/src/mlpack/bindings/python/mlpack/arma_numpy.pyx b/src/mlpack/bindings/python/mlpack/arma_numpy.pyx index dcf6908d94f..e968e888074 100644 --- a/src/mlpack/bindings/python/mlpack/arma_numpy.pyx +++ b/src/mlpack/bindings/python/mlpack/arma_numpy.pyx @@ -26,6 +26,9 @@ numpy.import_array() cimport arma from libcpp cimport bool +import platform +isWin = (platform.system() == "Windows") + cdef extern from "numpy/arrayobject.h": void PyArray_ENABLEFLAGS(numpy.ndarray arr, int flags) void PyArray_CLEARFLAGS(numpy.ndarray arr, int flags) @@ -45,16 +48,16 @@ cdef arma.Mat[double]* numpy_to_mat_d(numpy.ndarray[numpy.double_t, ndim=2] X, \ """ Convert a numpy ndarray to a matrix. The memory will still be owned by numpy. """ - if not (X.flags.c_contiguous or X.flags.owndata): + if not X.flags.c_contiguous or (not X.flags.owndata and not isWin)): # If needed, make a copy where we own the memory. X = X.copy(order="C") takeOwnership = True cdef arma.Mat[double]* m = new arma.Mat[double]( X.data, X.shape[1],\ - X.shape[0], False, False) + X.shape[0], isWin, False) - # Take ownership of the memory, if we need to. - if takeOwnership: + # Take ownership of the memory, if we need to and we are not on Windows. + if takeOwnership and !isWin: PyArray_CLEARFLAGS(X, numpy.NPY_OWNDATA) SetMemState[arma.Mat[double]](m[0], 0) @@ -65,16 +68,17 @@ cdef arma.Mat[size_t]* numpy_to_mat_s(numpy.ndarray[numpy.npy_intp, ndim=2] X, \ """ Convert a numpy ndarray to a matrix. The memory will still be owned by numpy. """ - if not (X.flags.c_contiguous or X.flags.owndata): - # If needed, make a copy where we own the memory. + if not X.flags.c_contiguous or (not X.flags.owndata and not isWin)): + # If needed, make a copy where we own the memory, except on Windows where + # we never copy. X = X.copy(order="C") takeOwnership = True cdef arma.Mat[size_t]* m = new arma.Mat[size_t]( X.data, X.shape[1], - X.shape[0], False, False) + X.shape[0], isWin, False) # Take ownership of the memory, if we need to. - if takeOwnership: + if takeOwnership and !isWin: PyArray_CLEARFLAGS(X, numpy.NPY_OWNDATA) SetMemState[arma.Mat[size_t]](m[0], 0) @@ -91,9 +95,11 @@ cdef numpy.ndarray[numpy.double_t, ndim=2] mat_to_numpy_d(arma.Mat[double]& X) \ dims[1] = X.n_rows cdef numpy.ndarray[numpy.double_t, ndim=2] output = \ numpy.PyArray_SimpleNewFromData(2, &dims[0], numpy.NPY_DOUBLE, GetMemory(X)) + if isWin: + output = output.copy(order="C") # Transfer memory ownership, if needed. - if GetMemState[arma.Mat[double]](X) == 0: + if GetMemState[arma.Mat[double]](X) == 0 and not isWin: SetMemState[arma.Mat[double]](X, 1) PyArray_ENABLEFLAGS(output, numpy.NPY_OWNDATA) @@ -110,9 +116,11 @@ cdef numpy.ndarray[numpy.npy_intp, ndim=2] mat_to_numpy_s(arma.Mat[size_t]& X) \ dims[1] = X.n_rows cdef numpy.ndarray[numpy.npy_intp, ndim=2] output = \ numpy.PyArray_SimpleNewFromData(2, &dims[0], numpy.NPY_INTP, GetMemory(X)) + if isWin: + output = output.copy(order="C") # Transfer memory ownership, if needed. - if GetMemState[arma.Mat[size_t]](X) == 0: + if GetMemState[arma.Mat[size_t]](X) == 0 and not isWin: SetMemState[arma.Mat[size_t]](X, 1) PyArray_ENABLEFLAGS(output, numpy.NPY_OWNDATA) @@ -124,16 +132,17 @@ cdef arma.Row[double]* numpy_to_row_d(numpy.ndarray[numpy.double_t, ndim=1] X, \ Convert a numpy one-dimensional ndarray to a row. The memory will still be owned by numpy. """ - if not (X.flags.c_contiguous or X.flags.owndata): - # If needed, make a copy where we own the memory. + if not X.flags.c_contiguous or (not X.flags.owndata and not isWin)): + # If needed, make a copy where we own the memory, except on Windows where + # we never copy. X = X.copy(order="C") takeOwnership = True cdef arma.Row[double]* m = new arma.Row[double]( X.data, X.shape[0], - False, False) + isWin, False) # Transfer memory ownership, if needed. - if takeOwnership: + if takeOwnership and not isWin: PyArray_CLEARFLAGS(X, numpy.NPY_OWNDATA) SetMemState[arma.Row[double]](m[0], 0) @@ -145,16 +154,17 @@ cdef arma.Row[size_t]* numpy_to_row_s(numpy.ndarray[numpy.npy_intp, ndim=1] X, \ Convert a numpy one-dimensional ndarray to a row. The memory will still be owned by numpy. """ - if not (X.flags.c_contiguous or X.flags.owndata): - # If needed, make a copy where we own the memory. + if not X.flags.c_contiguous or (not X.flags.owndata and not isWin)): + # If needed, make a copy where we own the memory, except on Windows where + # we never copy. X = X.copy(order="C") takeOwnership = True cdef arma.Row[size_t]* m = new arma.Row[size_t]( X.data, X.shape[0], - False, False) + isWin, False) # Transfer memory ownership, if needed. - if takeOwnership: + if takeOwnership and not isWin: PyArray_CLEARFLAGS(X, numpy.NPY_OWNDATA) SetMemState[arma.Row[size_t]](m[0], 0) @@ -169,9 +179,11 @@ cdef numpy.ndarray[numpy.double_t, ndim=1] row_to_numpy_d(arma.Row[double]& X) \ cdef numpy.npy_intp dim = X.n_elem cdef numpy.ndarray[numpy.double_t, ndim=1] output = \ numpy.PyArray_SimpleNewFromData(1, &dim, numpy.NPY_DOUBLE, GetMemory(X)) + if isWin: + output = output.copy(order="C") # Transfer memory ownership, if needed. - if GetMemState[arma.Row[double]](X) == 0: + if GetMemState[arma.Row[double]](X) == 0 and not isWin: SetMemState[arma.Row[double]](X, 1) PyArray_ENABLEFLAGS(output, numpy.NPY_OWNDATA) @@ -182,13 +194,16 @@ cdef numpy.ndarray[numpy.npy_intp, ndim=1] row_to_numpy_s(arma.Row[size_t]& X) \ """ Convert an Armadillo row vector to a one-dimensional numpy ndarray. """ +# print("called row_to_numpy_s()\n") # Extract dimensions. cdef numpy.npy_intp dim = X.n_elem cdef numpy.ndarray[numpy.npy_intp, ndim=1] output = \ numpy.PyArray_SimpleNewFromData(1, &dim, numpy.NPY_INTP, GetMemory(X)) + if isWin: + output = output.copy(order="C") # Transfer memory ownership, if needed. - if GetMemState[arma.Row[size_t]](X) == 0: + if GetMemState[arma.Row[size_t]](X) == 0 and not isWin: SetMemState[arma.Row[size_t]](X, 1) PyArray_ENABLEFLAGS(output, numpy.NPY_OWNDATA) @@ -200,16 +215,17 @@ cdef arma.Col[double]* numpy_to_col_d(numpy.ndarray[numpy.double_t, ndim=1] X, \ Convert a numpy one-dimensional ndarray to a column vector. The memory will still be owned by numpy. """ - if not (X.flags.c_contiguous or X.flags.owndata): - # If needed, make a copy where we own the memory. + if not X.flags.c_contiguous or (not X.flags.owndata and not isWin)): + # If needed, make a copy where we own the memory, except on Windows where + # we never copy. X = X.copy(order="C") takeOwnership = True cdef arma.Col[double]* m = new arma.Col[double]( X.data, X.shape[0], - False, True) + isWin, False) # Transfer memory ownership, if needed. - if takeOwnership: + if takeOwnership and not isWin: PyArray_CLEARFLAGS(X, numpy.NPY_OWNDATA) SetMemState[arma.Col[double]](m[0], 0) @@ -221,15 +237,17 @@ cdef arma.Col[size_t]* numpy_to_col_s(numpy.ndarray[numpy.npy_intp, ndim=1] X, \ Convert a numpy one-dimensional ndarray to a column vector. The memory will still be owned by numpy. """ - if not (X.flags.c_contiguous or X.flags.owndata): - # If needed, make a copy where we own the memory. + if not X.flags.c_contiguous or (not X.flags.owndata and not isWin)): + # If needed, make a copy where we own the memory, except on Windows where + # we never copy. X = X.copy(order="C") + takeOwnership = True cdef arma.Col[size_t]* m = new arma.Col[size_t]( X.data, X.shape[0], - False, False) + isWin, False) # Transfer memory ownership, if needed. - if takeOwnership: + if takeOwnership and not isWin: PyArray_CLEARFLAGS(X, numpy.NPY_OWNDATA) SetMemState[arma.Col[size_t]](m[0], 0) @@ -244,9 +262,11 @@ cdef numpy.ndarray[numpy.double_t, ndim=1] col_to_numpy_d(arma.Col[double]& X) \ cdef numpy.npy_intp dim = X.n_elem cdef numpy.ndarray[numpy.double_t, ndim=1] output = \ numpy.PyArray_SimpleNewFromData(1, &dim, numpy.NPY_DOUBLE, GetMemory(X)) + if isWin: + output = output.copy(order="C") # Transfer memory ownership, if needed. - if GetMemState[arma.Col[double]](X) == 0: + if GetMemState[arma.Col[double]](X) == 0 and not isWin: SetMemState[arma.Col[double]](X, 1) PyArray_ENABLEFLAGS(output, numpy.NPY_OWNDATA) @@ -261,10 +281,12 @@ cdef numpy.ndarray[numpy.npy_intp, ndim=1] col_to_numpy_s(arma.Col[size_t]& X) \ cdef numpy.npy_intp dim = X.n_elem cdef numpy.ndarray[numpy.npy_intp, ndim=1] output = \ numpy.PyArray_SimpleNewFromData(1, &dim, numpy.NPY_INTP, GetMemory(X)) + if isWin: + output = output.copy(order="C") # Transfer memory ownership, if needed. - if GetMemState[arma.Col[size_t]](X) == 0: + if GetMemState[arma.Col[size_t]](X) == 0 and not isWin: SetMemState[arma.Col[size_t]](X, 1) PyArray_ENABLEFLAGS(output, numpy.NPY_OWNDATA) - return output + return output2 From a80511565ddd28e8a6a78678c88a2b3fbb411725 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Thu, 29 Aug 2019 21:24:43 -0400 Subject: [PATCH 25/32] Fix test types for Windows compatibility. --- .../python/tests/dataset_info_test.py | 6 ++-- .../python/tests/test_python_binding.py | 32 +++++++++---------- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/src/mlpack/bindings/python/tests/dataset_info_test.py b/src/mlpack/bindings/python/tests/dataset_info_test.py index 0a5d9e73140..660558fcdcb 100644 --- a/src/mlpack/bindings/python/tests/dataset_info_test.py +++ b/src/mlpack/bindings/python/tests/dataset_info_test.py @@ -60,7 +60,8 @@ def testPandasMixedToMatrix(self): """ d = pd.DataFrame({'a': range(50)}) d['b'] = np.random.randn(50, 1) - self.assertEqual(d['a'].dtype, int) + self.assertTrue((d['a'].dtype == np.dtype('int32')) or + (d['a'].dtype == np.dtype('int64'))) self.assertEqual(d['b'].dtype, np.dtype(np.double)) m, _ = to_matrix(d) @@ -190,7 +191,8 @@ def testPandasMixedToMatrix(self): """ d = pd.DataFrame({'a': range(50)}) d['b'] = np.random.randn(50, 1) - self.assertEqual(d['a'].dtype, int) + self.assertTrue((d['a'].dtype == np.dtype('int32')) or + (d['a'].dtype == np.dtype('int64'))) self.assertEqual(d['b'].dtype, np.dtype(np.double)) m, _, dims = to_matrix_with_info(d, np.double) diff --git a/src/mlpack/bindings/python/tests/test_python_binding.py b/src/mlpack/bindings/python/tests/test_python_binding.py index 0856b0d6be7..f3867a6f309 100644 --- a/src/mlpack/bindings/python/tests/test_python_binding.py +++ b/src/mlpack/bindings/python/tests/test_python_binding.py @@ -237,7 +237,7 @@ def testPandasSeriesUMatrix(self): s_umatrix_in=z) self.assertEqual(output['s_umatrix_out'].shape[0], 100) - self.assertEqual(output['s_umatrix_out'].dtype, np.long) + self.assertEqual(output['s_umatrix_out'].dtype, np.dtype('intp')) for i in range(100): self.assertEqual(output['s_umatrix_out'][i, 0], x.iloc[i] * 2) @@ -256,7 +256,7 @@ def testPandasSeriesUMatrixForceCopy(self): copy_all_inputs=True) self.assertEqual(output['s_umatrix_out'].shape[0], 100) - self.assertEqual(output['s_umatrix_out'].dtype, np.long) + self.assertEqual(output['s_umatrix_out'].dtype, np.dtype('intp')) for i in range(100): self.assertEqual(output['s_umatrix_out'][i, 0], x.iloc[i] * 2) @@ -418,7 +418,7 @@ def testNumpyUmatrix(self): self.assertEqual(output['umatrix_out'].shape[0], 100) self.assertEqual(output['umatrix_out'].shape[1], 4) - self.assertEqual(output['umatrix_out'].dtype, np.long) + self.assertEqual(output['umatrix_out'].dtype, np.dtype('intp')) for i in [0, 1, 3]: for j in range(100): self.assertEqual(x[j, i], output['umatrix_out'][j, i]) @@ -440,7 +440,7 @@ def testNumpyUmatrixForceCopy(self): self.assertEqual(output['umatrix_out'].shape[0], 100) self.assertEqual(output['umatrix_out'].shape[1], 4) - self.assertEqual(output['umatrix_out'].dtype, np.long) + self.assertEqual(output['umatrix_out'].dtype, np.dtype('intp')) for i in [0, 1, 3]: for j in range(100): self.assertEqual(x[j, i], output['umatrix_out'][j, i]) @@ -463,7 +463,7 @@ def testArraylikeUmatrix(self): self.assertEqual(output['umatrix_out'].shape[0], 3) self.assertEqual(output['umatrix_out'].shape[1], 4) - self.assertEqual(output['umatrix_out'].dtype, np.long) + self.assertEqual(output['umatrix_out'].dtype, np.dtype('intp')) self.assertEqual(output['umatrix_out'][0, 0], 1) self.assertEqual(output['umatrix_out'][0, 1], 2) self.assertEqual(output['umatrix_out'][0, 2], 6) @@ -495,7 +495,7 @@ def testArraylikeUmatrixForceCopy(self): self.assertEqual(output['umatrix_out'].shape[1], 4) self.assertEqual(len(x), 3) self.assertEqual(len(x[0]), 5) - self.assertEqual(output['umatrix_out'].dtype, np.long) + self.assertEqual(output['umatrix_out'].dtype, np.dtype('intp')) self.assertEqual(output['umatrix_out'][0, 0], 1) self.assertEqual(output['umatrix_out'][0, 1], 2) self.assertEqual(output['umatrix_out'][0, 2], 6) @@ -558,7 +558,7 @@ def testUcol(self): ucol_in=z) self.assertEqual(output['ucol_out'].shape[0], 100) - self.assertEqual(output['ucol_out'].dtype, np.long) + self.assertEqual(output['ucol_out'].dtype, np.dtype('intp')) for i in range(100): self.assertEqual(output['ucol_out'][i], x[i] * 2) @@ -575,7 +575,7 @@ def testUcolForceCopy(self): copy_all_inputs=True) self.assertEqual(output['ucol_out'].shape[0], 100) - self.assertEqual(output['ucol_out'].dtype, np.long) + self.assertEqual(output['ucol_out'].dtype, np.dtype('intp')) for i in range(100): self.assertEqual(output['ucol_out'][i], x[i] * 2) @@ -628,7 +628,7 @@ def testUrow(self): urow_in=z) self.assertEqual(output['urow_out'].shape[0], 100) - self.assertEqual(output['urow_out'].dtype, np.long) + self.assertEqual(output['urow_out'].dtype, np.dtype('intp')) for i in range(100): self.assertEqual(output['urow_out'][i], x[i] * 2) @@ -646,7 +646,7 @@ def testUrowForceCopy(self): copy_all_inputs=True) self.assertEqual(output['urow_out'].shape[0], 100) - self.assertEqual(output['urow_out'].dtype, np.long) + self.assertEqual(output['urow_out'].dtype, np.dtype('intp')) for i in range(100): self.assertEqual(output['urow_out'][i], x[i] * 2) @@ -837,7 +837,7 @@ def testOneDimensionNumpyUmatrix(self): s_umatrix_in=z) self.assertEqual(output['s_umatrix_out'].shape[0], 100) - self.assertEqual(output['s_umatrix_out'].dtype, np.long) + self.assertEqual(output['s_umatrix_out'].dtype, np.dtype('intp')) for i in range(100): self.assertEqual(output['s_umatrix_out'][i, 0], x[i] * 2) @@ -855,7 +855,7 @@ def testOneDimensionNumpyUmatrixForceCopy(self): copy_all_inputs=True) self.assertEqual(output['s_umatrix_out'].shape[0], 100) - self.assertEqual(output['s_umatrix_out'].dtype, np.long) + self.assertEqual(output['s_umatrix_out'].dtype, np.dtype('intp')) for i in range(100): self.assertEqual(output['s_umatrix_out'][i, 0], x[i] * 2) @@ -909,7 +909,7 @@ def testTwoDimensionUcol(self): ucol_in=z) self.assertEqual(output['ucol_out'].shape[0], 100) - self.assertEqual(output['ucol_out'].dtype, np.long) + self.assertEqual(output['ucol_out'].dtype, np.dtype('intp')) for i in range(100): self.assertEqual(output['ucol_out'][i], x[i] * 2) @@ -926,7 +926,7 @@ def testTwoDimensionUcolForceCopy(self): copy_all_inputs=True) self.assertEqual(output['ucol_out'].shape[0], 100) - self.assertEqual(output['ucol_out'].dtype, np.long) + self.assertEqual(output['ucol_out'].dtype, np.dtype('intp')) for i in range(100): self.assertEqual(output['ucol_out'][i], x[i] * 2) @@ -979,7 +979,7 @@ def testTwoDimensionUrow(self): urow_in=z) self.assertEqual(output['urow_out'].shape[0], 100) - self.assertEqual(output['urow_out'].dtype, np.long) + self.assertEqual(output['urow_out'].dtype, np.dtype('intp')) for i in range(100): self.assertEqual(output['urow_out'][i], x[i] * 2) @@ -997,7 +997,7 @@ def testTwoDimensionUrowForceCopy(self): copy_all_inputs=True) self.assertEqual(output['urow_out'].shape[0], 101) - self.assertEqual(output['urow_out'].dtype, np.long) + self.assertEqual(output['urow_out'].dtype, np.dtype('intp')) for i in range(101): self.assertEqual(output['urow_out'][i], x[0][i] * 2) From dfe1a9fcb6e971a891b5320657d5ab18c46f3dc7 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Thu, 29 Aug 2019 23:00:12 -0400 Subject: [PATCH 26/32] Fix small syntax issues. --- .../bindings/python/mlpack/arma_numpy.pyx | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/mlpack/bindings/python/mlpack/arma_numpy.pyx b/src/mlpack/bindings/python/mlpack/arma_numpy.pyx index e968e888074..7869f9b8b25 100644 --- a/src/mlpack/bindings/python/mlpack/arma_numpy.pyx +++ b/src/mlpack/bindings/python/mlpack/arma_numpy.pyx @@ -48,7 +48,7 @@ cdef arma.Mat[double]* numpy_to_mat_d(numpy.ndarray[numpy.double_t, ndim=2] X, \ """ Convert a numpy ndarray to a matrix. The memory will still be owned by numpy. """ - if not X.flags.c_contiguous or (not X.flags.owndata and not isWin)): + if not X.flags.c_contiguous or (not X.flags.owndata and not isWin): # If needed, make a copy where we own the memory. X = X.copy(order="C") takeOwnership = True @@ -57,7 +57,7 @@ cdef arma.Mat[double]* numpy_to_mat_d(numpy.ndarray[numpy.double_t, ndim=2] X, \ X.shape[0], isWin, False) # Take ownership of the memory, if we need to and we are not on Windows. - if takeOwnership and !isWin: + if takeOwnership and not isWin: PyArray_CLEARFLAGS(X, numpy.NPY_OWNDATA) SetMemState[arma.Mat[double]](m[0], 0) @@ -68,7 +68,7 @@ cdef arma.Mat[size_t]* numpy_to_mat_s(numpy.ndarray[numpy.npy_intp, ndim=2] X, \ """ Convert a numpy ndarray to a matrix. The memory will still be owned by numpy. """ - if not X.flags.c_contiguous or (not X.flags.owndata and not isWin)): + if not X.flags.c_contiguous or (not X.flags.owndata and not isWin): # If needed, make a copy where we own the memory, except on Windows where # we never copy. X = X.copy(order="C") @@ -78,7 +78,7 @@ cdef arma.Mat[size_t]* numpy_to_mat_s(numpy.ndarray[numpy.npy_intp, ndim=2] X, \ X.shape[0], isWin, False) # Take ownership of the memory, if we need to. - if takeOwnership and !isWin: + if takeOwnership and not isWin: PyArray_CLEARFLAGS(X, numpy.NPY_OWNDATA) SetMemState[arma.Mat[size_t]](m[0], 0) @@ -132,7 +132,7 @@ cdef arma.Row[double]* numpy_to_row_d(numpy.ndarray[numpy.double_t, ndim=1] X, \ Convert a numpy one-dimensional ndarray to a row. The memory will still be owned by numpy. """ - if not X.flags.c_contiguous or (not X.flags.owndata and not isWin)): + if not X.flags.c_contiguous or (not X.flags.owndata and not isWin): # If needed, make a copy where we own the memory, except on Windows where # we never copy. X = X.copy(order="C") @@ -154,7 +154,7 @@ cdef arma.Row[size_t]* numpy_to_row_s(numpy.ndarray[numpy.npy_intp, ndim=1] X, \ Convert a numpy one-dimensional ndarray to a row. The memory will still be owned by numpy. """ - if not X.flags.c_contiguous or (not X.flags.owndata and not isWin)): + if not X.flags.c_contiguous or (not X.flags.owndata and not isWin): # If needed, make a copy where we own the memory, except on Windows where # we never copy. X = X.copy(order="C") @@ -215,7 +215,7 @@ cdef arma.Col[double]* numpy_to_col_d(numpy.ndarray[numpy.double_t, ndim=1] X, \ Convert a numpy one-dimensional ndarray to a column vector. The memory will still be owned by numpy. """ - if not X.flags.c_contiguous or (not X.flags.owndata and not isWin)): + if not X.flags.c_contiguous or (not X.flags.owndata and not isWin): # If needed, make a copy where we own the memory, except on Windows where # we never copy. X = X.copy(order="C") @@ -237,7 +237,7 @@ cdef arma.Col[size_t]* numpy_to_col_s(numpy.ndarray[numpy.npy_intp, ndim=1] X, \ Convert a numpy one-dimensional ndarray to a column vector. The memory will still be owned by numpy. """ - if not X.flags.c_contiguous or (not X.flags.owndata and not isWin)): + if not X.flags.c_contiguous or (not X.flags.owndata and not isWin): # If needed, make a copy where we own the memory, except on Windows where # we never copy. X = X.copy(order="C") @@ -289,4 +289,4 @@ cdef numpy.ndarray[numpy.npy_intp, ndim=1] col_to_numpy_s(arma.Col[size_t]& X) \ SetMemState[arma.Col[size_t]](X, 1) PyArray_ENABLEFLAGS(output, numpy.NPY_OWNDATA) - return output2 + return output From ba690908aac1e7c845d661cb3c3ce061259465de Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Fri, 30 Aug 2019 10:06:12 -0400 Subject: [PATCH 27/32] Better handling of paths with spaces. --- src/mlpack/bindings/python/ConfigureSetup.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mlpack/bindings/python/ConfigureSetup.cmake b/src/mlpack/bindings/python/ConfigureSetup.cmake index f981a4c40a7..63de5edd729 100644 --- a/src/mlpack/bindings/python/ConfigureSetup.cmake +++ b/src/mlpack/bindings/python/ConfigureSetup.cmake @@ -43,4 +43,4 @@ if (NOT EXISTS "${Boost_SERIALIZATION_LIBRARY}") endif () endif () -configure_file(${SETUP_PY_IN} ${SETUP_PY_OUT}) +configure_file("${SETUP_PY_IN}" "${SETUP_PY_OUT}") From 9cc44a2958e89aeb8fd67a3823c2c131f54476eb Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Fri, 30 Aug 2019 10:07:27 -0400 Subject: [PATCH 28/32] Only add these options on Windows. --- src/mlpack/bindings/python/setup.py.in | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/mlpack/bindings/python/setup.py.in b/src/mlpack/bindings/python/setup.py.in index fb6cfddfad3..032af857651 100644 --- a/src/mlpack/bindings/python/setup.py.in +++ b/src/mlpack/bindings/python/setup.py.in @@ -11,6 +11,7 @@ import os import sys import numpy as np import re +import platform # Process input argument --module={name} first if needed. module = None @@ -58,7 +59,10 @@ else: extra_args = ['-DBINDING_TYPE=BINDING_TYPE_PYX', '-std=c++11', '${OpenMP_CXX_FLAGS}'] - extra_args = extra_args + ['/MD', '/O2', '/Ob2', '/DNDEBUG'] + + # Extra options for MSVC compiler. + if platform.system() == 'Windows': + extra_args = extra_args + ['/MD', '/O2', '/Ob2', '/DNDEBUG'] # This is used for parallel builds; CMake will set PYX_TO_BUILD accordingly. if module is not None: From f2edbfa2de49a0644d9ed33087af8fcd55f65d56 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Fri, 30 Aug 2019 14:25:26 -0400 Subject: [PATCH 29/32] Allow slightly longer to build. --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 62d3ef7851e..9c1b4ab0b1b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -41,7 +41,7 @@ matrix: - brew install openblas armadillo || brew install openblas armadillo install: - - mkdir build && cd build && cmake $CMAKE_OPTIONS .. && travis_wait 60 make -j2 + - mkdir build && cd build && cmake $CMAKE_OPTIONS .. && travis_wait 75 make -j2 script: - CTEST_OUTPUT_ON_FAILURE=1 travis_wait 30 ctest -j2 From 2005a2fbb1f2f8e6c9173a7a444f4a78282911aa Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Tue, 24 Sep 2019 23:51:25 -0400 Subject: [PATCH 30/32] Copy all runtime DLLs to build directories. (slightly hacky but I don't see any better way) --- CMakeLists.txt | 38 ++++++++++++++++++++++- src/mlpack/bindings/python/CMakeLists.txt | 20 ++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index c410261c7f7..21487eab92d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -24,6 +24,9 @@ if (WIN32) if (NOT BUILD_PYTHON_BINDINGS) message(WARNING "By default Python bindings are not compiled for Windows because they are not known to work. Set BUILD_PYTHON_BINDINGS to ON if you want them built.") endif () + + set(DLL_COPY_DIRS "" CACHE STRING "List of directories (separated by ';') containing DLLs to copy for runtime.") + set(DLL_COPY_LIBS "" CACHE STRING "List of DLLs (separated by ';') that should be copied for runtime.") else () option(BUILD_PYTHON_BINDINGS "Build Python bindings." ON) option(BUILD_SHARED_LIBS @@ -310,7 +313,23 @@ if (WIN32) # Piggyback LAPACK and BLAS linking into Armadillo link. set(ARMADILLO_LIBRARIES - ${ARMADILLO_LIBRARIES} ${BLAS_LIBRARY} ${LAPACK_LIBRARY}) + ${ARMADILLO_LIBRARIES} ${BLAS_LIBRARY} ${LAPACK_LIBRARY}) + + # Ensure that the libraries are added to the MSVC IDE runtime path. + get_filename_component(BLAS_DIR ${BLAS_LIBRARY} DIRECTORY) + get_filename_component(LAPACK_DIR ${LAPACK_LIBRARY} DIRECTORY) + + # Sometimes, especially with an OpenBLAS install via nuget, the DLLs are + # actually in ../../bin/x64/. Automatically add these. + if (EXISTS "${BLAS_DIR}/../../bin/x64/") + get_filename_component(BLAS_DLL_DIR "${BLAS_DIR}/../../bin/x64" ABSOLUTE) + set(DLL_COPY_DIRS ${DLL_COPY_DIRS} "${BLAS_DLL_DIR}") + endif () + + if (EXISTS "${LAPACK_DIR}/../../bin/x64/") + get_filename_component(LAPACK_DLL_DIR "${LAPACK_DIR}/../../bin/x64" ABSOLUTE) + set(DLL_COPY_DIRS ${DLL_COPY_DIRS} "${BLAS_DLL_DIR}") + endif () endif () # Include directories for the previous dependencies. @@ -454,6 +473,8 @@ link_directories(${Boost_LIBRARY_DIRS}) # handle it. if (MSVC) link_directories(${Boost_LIBRARY_DIRS}) + set(CMAKE_MSVCIDE_RUN_PATH ${CMAKE_MSVCIDE_RUN_PATH} ${Boost_LIBRARY_DIRS}) + message("boost lib dirs ${Boost_LIBRARY_DIRS}") set(Boost_LIBRARIES "") endif () @@ -506,6 +527,21 @@ if (WIN32) set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}) set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}) set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}) + + # Copy all necessary DLLs for runtime to the build directory. + # This is a little hackish, but I can't figure out clear ways to make CMake + # consistently link everything 100% statically across platforms or set the + # runtime path right always, so this is the best I know how to do for now. + foreach(dir ${DLL_COPY_DIRS}) + file(GLOB dir_dll_list "${dir}/*.dll") + file(COPY ${dir_dll_list} DESTINATION ${CMAKE_BINARY_DIR}/Release/) + file(COPY ${dir_dll_list} DESTINATION ${CMAKE_BINARY_DIR}/Debug/) + endforeach () + + foreach(file ${DLL_COPY_LIBS}) + file(COPY ${file} DESTINATION ${CMAKE_BINARY_DIR}/Release/) + file(COPY ${file} DESTINATION ${CMAKE_BINARY_DIR}/Debug/) + endforeach() else () # If not on Windows, put them under more standard UNIX-like places. This is # necessary, otherwise they would all end up in diff --git a/src/mlpack/bindings/python/CMakeLists.txt b/src/mlpack/bindings/python/CMakeLists.txt index 01395392e6b..b396c6af182 100644 --- a/src/mlpack/bindings/python/CMakeLists.txt +++ b/src/mlpack/bindings/python/CMakeLists.txt @@ -184,6 +184,26 @@ install(SCRIPT ${CMAKE_CURRENT_SOURCE_DIR}/PythonInstall.cmake) file(COPY mlpack/__init__.py DESTINATION ${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/mlpack/) +if (WIN32) + # Copy all necessary DLLs to the Python build directory. + foreach (dir ${DLL_COPY_DIRS}) + file(GLOB dll_dir_files "${dir}/*.dll") + file(COPY ${dll_dir_files} DESTINATION ${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/mlpack/) + endforeach () + + foreach (dll ${DLL_COPY_LIBS}) + file(COPY ${dll} DESTINATION ${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/mlpack/) + endforeach () + + # We also need to copy the boost DLLs over. + file(GLOB boost_ser_dll_files "${Boost_LIBRARY_DIRS}/*serialization*.dll") + file(COPY ${boost_ser_dll_files} DESTINATION ${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/mlpack/) + file(GLOB boost_po_dll_files "${Boost_LIBRARY_DIRS}/*program*options*.dll") + file(COPY ${boost_po_dll_files} DESTINATION ${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/mlpack/) + file(GLOB boost_utf_dll_files "${Boost_LIBRARY_DIRS}/*unit*test*framework*.dll") + file(COPY ${boost_utf_dll_files} DESTINATION ${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/mlpack/) +endif () + # Add a macro to build a python binding. macro (add_python_binding name) if (BUILD_PYTHON_BINDINGS) From 3bbec9743e0bdc44e3996a7d0d8f290aacf9dcdf Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Wed, 9 Oct 2019 20:48:41 -0400 Subject: [PATCH 31/32] Extra paranoia with strings to handle paths with spaces. --- CMakeLists.txt | 22 +++++++++++----------- src/mlpack/CMakeLists.txt | 10 +++++----- src/mlpack/bindings/cli/CMakeLists.txt | 2 +- src/mlpack/bindings/python/CMakeLists.txt | 2 +- 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 5127d427eb1..4aa9d3d14f3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -342,7 +342,7 @@ find_package(StbImage) if (NOT STB_IMAGE_FOUND) if (DOWNLOAD_STB_IMAGE) set(STB_DIR "stb") - install(DIRECTORY DESTINATION ${CMAKE_BINARY_DIR}/deps/${STB_DIR}) + install(DIRECTORY DESTINATION "${CMAKE_BINARY_DIR}/deps/${STB_DIR}") file(DOWNLOAD http://mlpack.org/files/stb/stb_image.h "${CMAKE_BINARY_DIR}/deps/${STB_DIR}/stb_image.h" STATUS STB_IMAGE_DOWNLOAD_STATUS_LIST LOG STB_IMAGE_DOWNLOAD_LOG @@ -362,8 +362,8 @@ if (NOT STB_IMAGE_FOUND) message(STATUS "Successfully downloaded stb into ${CMAKE_BINARY_DIR}/deps/${STB_DIR}/") # Now we have to also ensure these header files get installed. - install(FILES ${CMAKE_BINARY_DIR}/deps/${STB_DIR}/stb_image.h DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}) - install(FILES ${CMAKE_BINARY_DIR}/deps/${STB_DIR}/stb_image_write.h DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}) + install(FILES "${CMAKE_BINARY_DIR}/deps/${STB_DIR}/stb_image.h" DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}") + install(FILES "${CMAKE_BINARY_DIR}/deps/${STB_DIR}/stb_image_write.h" DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}") add_definitions(-DHAS_STB) else () list(GET STB_IMAGE_DOWNLOAD_STATUS_LIST 1 STB_DOWNLOAD_ERROR) @@ -410,8 +410,8 @@ if (NOT ENSMALLEN_FOUND) "Successfully downloaded ensmallen into ${CMAKE_BINARY_DIR}/deps/${ENSMALLEN_INCLUDE_DIR}/") # Now we have to also ensure these header files get installed. - install(DIRECTORY ${CMAKE_BINARY_DIR}/deps/${ENSMALLEN_INCLUDE_DIR}/include/ensmallen_bits/ DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/ensmallen_bits) - install(FILES ${CMAKE_BINARY_DIR}/deps/${ENSMALLEN_INCLUDE_DIR}/include/ensmallen.hpp DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}) + install(DIRECTORY "${CMAKE_BINARY_DIR}/deps/${ENSMALLEN_INCLUDE_DIR}/include/ensmallen_bits/" DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/ensmallen_bits") + install(FILES "${CMAKE_BINARY_DIR}/deps/${ENSMALLEN_INCLUDE_DIR}/include/ensmallen.hpp" DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}") else () message(FATAL_ERROR "Problem unpacking ensmallen! Expected only one directory ensmallen-x.y.z/; found ${ENS_DIRECTORIES}. Try removing the directory ${CMAKE_BINARY_DIR}/deps and reconfiguring.") endif () @@ -616,8 +616,8 @@ if (BUILD_CLI_EXECUTABLES AND UNIX) ) # Set the rules to install the documentation. - install(DIRECTORY ${CMAKE_BINARY_DIR}/share/man/ - DESTINATION ${CMAKE_INSTALL_MANDIR}) + install(DIRECTORY "${CMAKE_BINARY_DIR}/share/man/" + DESTINATION "${CMAKE_INSTALL_MANDIR}") endif () endif () @@ -668,8 +668,8 @@ if (DOXYGEN_FOUND) COMMENT "Generating API documentation with Doxygen" ) - install(DIRECTORY ${CMAKE_BINARY_DIR}/doc/html - DESTINATION ${CMAKE_INSTALL_DOCDIR} + install(DIRECTORY "${CMAKE_BINARY_DIR}/doc/html" + DESTINATION "${CMAKE_INSTALL_DOCDIR}" COMPONENT doc OPTIONAL ) @@ -748,6 +748,6 @@ if (PKG_CONFIG_FOUND) DEPENDS mlpack_headers COMMENT "Generating mlpack.pc (pkg-config) file.") - install(FILES ${CMAKE_CURRENT_BINARY_DIR}/lib/pkgconfig/mlpack.pc - DESTINATION ${CMAKE_INSTALL_LIBDIR}/pkgconfig/) + install(FILES "${CMAKE_CURRENT_BINARY_DIR}/lib/pkgconfig/mlpack.pc" + DESTINATION "${CMAKE_INSTALL_LIBDIR}/pkgconfig/") endif () diff --git a/src/mlpack/CMakeLists.txt b/src/mlpack/CMakeLists.txt index 594f85f4aa0..fce358817f3 100644 --- a/src/mlpack/CMakeLists.txt +++ b/src/mlpack/CMakeLists.txt @@ -90,15 +90,15 @@ endforeach() # At install time, we simply install that directory of header files we # collected to include/. -install(DIRECTORY ${CMAKE_BINARY_DIR}/include/mlpack DESTINATION - ${CMAKE_INSTALL_INCLUDEDIR}) +install(DIRECTORY "${CMAKE_BINARY_DIR}/include/mlpack" DESTINATION + "${CMAKE_INSTALL_INCLUDEDIR}") # Set generated executables to be installed. Unfortunately they must manually # be entered... install(TARGETS mlpack - RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} - LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} - ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}) + RUNTIME DESTINATION "${CMAKE_INSTALL_BINDIR}" + LIBRARY DESTINATION "${CMAKE_INSTALL_LIBDIR}" + ARCHIVE DESTINATION "${CMAKE_INSTALL_LIBDIR}") add_dependencies(mlpack mlpack_headers) diff --git a/src/mlpack/bindings/cli/CMakeLists.txt b/src/mlpack/bindings/cli/CMakeLists.txt index 1a16bd93434..76dd6fd5ffb 100644 --- a/src/mlpack/bindings/cli/CMakeLists.txt +++ b/src/mlpack/bindings/cli/CMakeLists.txt @@ -59,7 +59,7 @@ if (BUILD_CLI_EXECUTABLES) # compiled with the correct int main() call. set_target_properties(mlpack_${name} PROPERTIES COMPILE_FLAGS -DBINDING_TYPE=BINDING_TYPE_CLI) - install(TARGETS mlpack_${name} RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}) + install(TARGETS mlpack_${name} RUNTIME DESTINATION "${CMAKE_INSTALL_BINDIR}") # If man documentation is being generated, make sure this is a dependency. if (TXT2MAN) diff --git a/src/mlpack/bindings/python/CMakeLists.txt b/src/mlpack/bindings/python/CMakeLists.txt index b396c6af182..2882c59545a 100644 --- a/src/mlpack/bindings/python/CMakeLists.txt +++ b/src/mlpack/bindings/python/CMakeLists.txt @@ -178,7 +178,7 @@ install(CODE "set(PYTHON_EXECUTABLE \"${PYTHON_EXECUTABLE}\")") install(CODE "set(CMAKE_BINARY_DIR \"${CMAKE_BINARY_DIR}\")") install(CODE "set(CMAKE_INSTALL_PREFIX \"${CMAKE_INSTALL_PREFIX}\")") install(CODE "execute_process(COMMAND mkdir -p $ENV{DESTDIR}${CMAKE_PYTHON_PATH})") -install(SCRIPT ${CMAKE_CURRENT_SOURCE_DIR}/PythonInstall.cmake) +install(SCRIPT "${CMAKE_CURRENT_SOURCE_DIR}/PythonInstall.cmake") # Prepare __init__.py for having all of the convenience imports appended to it. file(COPY mlpack/__init__.py DESTINATION From d6d163b3f81ed5c5266485361006c3174a0a30ce Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Fri, 11 Oct 2019 18:31:31 -0400 Subject: [PATCH 32/32] Remove now-unnecessary warning and set default to ON for Windows. --- CMakeLists.txt | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 4aa9d3d14f3..16e37c9a6f2 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -14,21 +14,15 @@ option(BUILD_TESTS "Build tests." ON) option(BUILD_CLI_EXECUTABLES "Build command-line executables." ON) option(DOWNLOAD_ENSMALLEN "If ensmallen is not found, download it." ON) option(DOWNLOAD_STB_IMAGE "Download stb_image for image loading." ON) +option(BUILD_PYTHON_BINDINGS "Build Python bindings." ON) -# Currently Python bindings aren't known to build successfully on Windows, so -# set BUILD_PYTHON_BINDINGS to OFF when the platform is Windows. if (WIN32) - option(BUILD_PYTHON_BINDINGS "Build Python bindings." OFF) option(BUILD_SHARED_LIBS "Compile shared libraries (if OFF, static libraries are compiled)." OFF) - if (NOT BUILD_PYTHON_BINDINGS) - message(WARNING "By default Python bindings are not compiled for Windows because they are not known to work. Set BUILD_PYTHON_BINDINGS to ON if you want them built.") - endif () set(DLL_COPY_DIRS "" CACHE STRING "List of directories (separated by ';') containing DLLs to copy for runtime.") set(DLL_COPY_LIBS "" CACHE STRING "List of DLLs (separated by ';') that should be copied for runtime.") else () - option(BUILD_PYTHON_BINDINGS "Build Python bindings." ON) option(BUILD_SHARED_LIBS "Compile shared libraries (if OFF, static libraries are compiled)." ON) endif()