From 993c282905b13e2f78fe7ee66815370020aab7a1 Mon Sep 17 00:00:00 2001 From: chrisbeall Date: Mon, 22 May 2017 16:23:46 -0400 Subject: [PATCH 1/4] Add build dependency to ensure cython wrapper is built after cpp library --- cmake/GtsamCythonWrap.cmake | 11 ++++++----- gtsam/CMakeLists.txt | 1 + gtsam_unstable/CMakeLists.txt | 1 + 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/cmake/GtsamCythonWrap.cmake b/cmake/GtsamCythonWrap.cmake index 291b6a22a..1cf03e3e0 100644 --- a/cmake/GtsamCythonWrap.cmake +++ b/cmake/GtsamCythonWrap.cmake @@ -26,17 +26,18 @@ endif() # to setup.py file by cmake and used to compile the Cython module # by invoking "python setup.py build_ext --inplace" # install_path: destination to install the library -function(wrap_and_install_library_cython interface_header extra_imports setup_py_in_path install_path) +# dependencies: Dependencies which need to be built before the wrapper +function(wrap_and_install_library_cython interface_header extra_imports setup_py_in_path install_path dependencies) # Paths for generated files get_filename_component(module_name "${interface_header}" NAME_WE) set(generated_files_path "${PROJECT_BINARY_DIR}/cython/${module_name}") - wrap_library_cython("${interface_header}" "${generated_files_path}" "${extra_imports}" "${setup_py_in_path}") + wrap_library_cython("${interface_header}" "${generated_files_path}" "${extra_imports}" "${setup_py_in_path}" "${dependencies}") install_cython_wrapped_library("${interface_header}" "${generated_files_path}" "${install_path}") endfunction() # Internal function that wraps a library and compiles the wrapper -function(wrap_library_cython interface_header generated_files_path extra_imports setup_py_in_path) +function(wrap_library_cython interface_header generated_files_path extra_imports setup_py_in_path dependencies) # Wrap codegen interface # Extract module path and name from interface header file name # wrap requires interfacePath to be *absolute* @@ -73,8 +74,8 @@ function(wrap_library_cython interface_header generated_files_path extra_imports WORKING_DIRECTORY ${generated_files_path}) # Set up building of mex module - add_custom_target(${module_name}_cython_wrapper ALL DEPENDS ${generated_cpp_file} ${interface_header}) - add_custom_target(wrap_${module_name}_cython_distclean + add_custom_target(${module_name}_cython_wrapper ALL DEPENDS ${generated_cpp_file} ${interface_header} ${dependencies}) + add_custom_target(wrap_${module_name}_cython_distclean COMMAND cmake -E remove_directory ${generated_files_path}) endfunction() diff --git a/gtsam/CMakeLists.txt b/gtsam/CMakeLists.txt index eb9154091..8e8302ba5 100644 --- a/gtsam/CMakeLists.txt +++ b/gtsam/CMakeLists.txt @@ -176,5 +176,6 @@ if (GTSAM_INSTALL_CYTHON_TOOLBOX) "" # extra imports "../cython/gtsam" # path to setup.py.in "${GTSAM_CYTHON_INSTALL_PATH}/gtsam" # install path + gtsam # dependencies which need to be built before the wrapper ) endif () diff --git a/gtsam_unstable/CMakeLists.txt b/gtsam_unstable/CMakeLists.txt index 7e26bca04..bcf53946c 100644 --- a/gtsam_unstable/CMakeLists.txt +++ b/gtsam_unstable/CMakeLists.txt @@ -129,6 +129,7 @@ if (GTSAM_INSTALL_CYTHON_TOOLBOX) "from gtsam.gtsam cimport *" # extra imports "../cython/gtsam_unstable" # path to setup.py.in "${GTSAM_CYTHON_INSTALL_PATH}/gtsam" # install path + gtsam_unstable # dependencies which need to be built before the wrapper ) add_dependencies(gtsam_unstable_cython_wrapper gtsam_cython_wrapper) endif () From 544b06510a526bf2f9cf80863289b13a19e24824 Mon Sep 17 00:00:00 2001 From: Duy-Nguyen Ta Date: Wed, 24 May 2017 23:46:36 +0800 Subject: [PATCH 2/4] remove whitespaces --- cmake/GtsamCythonWrap.cmake | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/cmake/GtsamCythonWrap.cmake b/cmake/GtsamCythonWrap.cmake index 1cf03e3e0..96ba18692 100644 --- a/cmake/GtsamCythonWrap.cmake +++ b/cmake/GtsamCythonWrap.cmake @@ -11,16 +11,16 @@ if(NOT GTSAM_CYTHON_INSTALL_PATH) set(GTSAM_CYTHON_INSTALL_PATH "${CMAKE_INSTALL_PREFIX}/cython") endif() -# User-friendly Cython wrapping and installing function. -# Builds a Cython module from the provided interface_header. +# User-friendly Cython wrapping and installing function. +# Builds a Cython module from the provided interface_header. # For example, for the interface header gtsam.h, # this will build the wrap module 'gtsam'. # # Arguments: # # interface_header: The relative path to the wrapper interface definition file. -# extra_imports: extra header to import in the Cython pxd file. -# For example, to use Cython gtsam.pxd in your own module, +# extra_imports: extra header to import in the Cython pxd file. +# For example, to use Cython gtsam.pxd in your own module, # use "from gtsam cimport *" # setup_py_in_path: Path to the setup.py.in config file, which will be converted # to setup.py file by cmake and used to compile the Cython module @@ -46,7 +46,7 @@ function(wrap_library_cython interface_header generated_files_path extra_imports get_filename_component(module_name "${interface_header}" NAME_WE) set(generated_cpp_file "${generated_files_path}/${module_name}.cpp") - + message(STATUS "Building wrap module ${module_name}") # Get build type postfix - gtsam_library_postfix is used in setup.py.in @@ -97,8 +97,8 @@ function(install_cython_wrapped_library interface_header generated_files_path in # Split up filename to strip trailing '/' in GTSAM_CYTHON_INSTALL_PATH if there is one get_filename_component(location "${install_path}" PATH) get_filename_component(name "${install_path}" NAME) - install(DIRECTORY "${generated_files_path}/" DESTINATION "${location}/${name}${build_type_tag}" - CONFIGURATIONS "${build_type}" + install(DIRECTORY "${generated_files_path}/" DESTINATION "${location}/${name}${build_type_tag}" + CONFIGURATIONS "${build_type}" PATTERN "build" EXCLUDE PATTERN "CMakeFiles" EXCLUDE PATTERN "Makefile" EXCLUDE @@ -107,7 +107,7 @@ function(install_cython_wrapped_library interface_header generated_files_path in PATTERN "*.py" EXCLUDE) endforeach() else() - install(DIRECTORY "${generated_files_path}/" DESTINATION ${install_path} + install(DIRECTORY "${generated_files_path}/" DESTINATION ${install_path} PATTERN "build" EXCLUDE PATTERN "CMakeFiles" EXCLUDE PATTERN "Makefile" EXCLUDE @@ -158,7 +158,7 @@ endfunction() # should be installed to all build type toolboxes # # Arguments: -# source_files: The source files to be installed. +# source_files: The source files to be installed. # dest_directory: The destination directory to install to. function(install_cython_files source_files dest_directory) From 1521a7e8ef59209b4b27e22170d5a2e2b4e41a99 Mon Sep 17 00:00:00 2001 From: Duy-Nguyen Ta Date: Wed, 24 May 2017 23:51:45 +0800 Subject: [PATCH 3/4] compile cython using the manual 2-step process This is to leverage all compile and linking flags within the cmake build system. http://cython.readthedocs.io/en/latest/src/reference/compilation.html#compiling-from-the-command-line --- cmake/FindEigency.cmake | 27 ++++++++++++++++++++ cmake/GtsamCythonWrap.cmake | 48 +++++++++++++++++------------------ cython/CMakeLists.txt | 35 +++++++++++++++++-------- gtsam/CMakeLists.txt | 13 ---------- gtsam_unstable/CMakeLists.txt | 15 ----------- 5 files changed, 76 insertions(+), 62 deletions(-) create mode 100644 cmake/FindEigency.cmake diff --git a/cmake/FindEigency.cmake b/cmake/FindEigency.cmake new file mode 100644 index 000000000..3e48dab64 --- /dev/null +++ b/cmake/FindEigency.cmake @@ -0,0 +1,27 @@ +# Find Eigency +# +# This code sets the following variables: +# +# EIGENCY_FOUND +# EIGENCY_INCLUDE_DIRS +# + +# Find python +find_package( PythonInterp ) +if ( PYTHONINTERP_FOUND ) + execute_process( COMMAND "${PYTHON_EXECUTABLE}" "-c" + "import eigency; includes=eigency.get_includes(include_eigen=False); print includes[0], ';', includes[1]" + RESULT_VARIABLE RESULT + OUTPUT_VARIABLE EIGENCY_INCLUDE_DIRS + OUTPUT_STRIP_TRAILING_WHITESPACE + ) +endif () + +include( FindPackageHandleStandardArgs ) +find_package_handle_standard_args(Eigency + FOUND_VAR + EIGENCY_FOUND + REQUIRED_VARS + EIGENCY_INCLUDE_DIRS +) + diff --git a/cmake/GtsamCythonWrap.cmake b/cmake/GtsamCythonWrap.cmake index 96ba18692..e0ed1eb89 100644 --- a/cmake/GtsamCythonWrap.cmake +++ b/cmake/GtsamCythonWrap.cmake @@ -22,22 +22,19 @@ endif() # extra_imports: extra header to import in the Cython pxd file. # For example, to use Cython gtsam.pxd in your own module, # use "from gtsam cimport *" -# setup_py_in_path: Path to the setup.py.in config file, which will be converted -# to setup.py file by cmake and used to compile the Cython module -# by invoking "python setup.py build_ext --inplace" # install_path: destination to install the library # dependencies: Dependencies which need to be built before the wrapper -function(wrap_and_install_library_cython interface_header extra_imports setup_py_in_path install_path dependencies) +function(wrap_and_install_library_cython interface_header extra_imports install_path dependencies) # Paths for generated files get_filename_component(module_name "${interface_header}" NAME_WE) set(generated_files_path "${PROJECT_BINARY_DIR}/cython/${module_name}") - wrap_library_cython("${interface_header}" "${generated_files_path}" "${extra_imports}" "${setup_py_in_path}" "${dependencies}") + wrap_library_cython("${interface_header}" "${generated_files_path}" "${extra_imports}" "${dependencies}") install_cython_wrapped_library("${interface_header}" "${generated_files_path}" "${install_path}") endfunction() # Internal function that wraps a library and compiles the wrapper -function(wrap_library_cython interface_header generated_files_path extra_imports setup_py_in_path dependencies) +function(wrap_library_cython interface_header generated_files_path extra_imports dependencies) # Wrap codegen interface # Extract module path and name from interface header file name # wrap requires interfacePath to be *absolute* @@ -48,34 +45,37 @@ function(wrap_library_cython interface_header generated_files_path extra_imports set(generated_cpp_file "${generated_files_path}/${module_name}.cpp") message(STATUS "Building wrap module ${module_name}") - - # Get build type postfix - gtsam_library_postfix is used in setup.py.in - # to link cythonized library to appropriate version of gtsam library - set(gtsam_library_postfix "") - if(GTSAM_BUILD_TYPE_POSTFIXES) - string(TOUPPER "${CMAKE_BUILD_TYPE}" build_type_upper) - set(gtsam_library_postfix ${CMAKE_${build_type_upper}_POSTFIX}) - endif() # Set up generation of module source file file(MAKE_DIRECTORY "${generated_files_path}") - configure_file(${setup_py_in_path}/setup.py.in ${generated_files_path}/setup.py) add_custom_command( OUTPUT ${generated_cpp_file} - DEPENDS ${interface_header} wrap - COMMAND + DEPENDS ${interface_header} wrap + COMMAND wrap --cython ${module_path} - ${module_name} - ${generated_files_path} + ${module_name} + ${generated_files_path} "${extra_imports}" - && python setup.py build_ext --inplace + && cython --cplus -I. "${generated_files_path}/${module_name}.pyx" VERBATIM - WORKING_DIRECTORY ${generated_files_path}) - - # Set up building of mex module + WORKING_DIRECTORY ${generated_files_path}/../) add_custom_target(${module_name}_cython_wrapper ALL DEPENDS ${generated_cpp_file} ${interface_header} ${dependencies}) - add_custom_target(wrap_${module_name}_cython_distclean + + # Set up building of cython module + find_package(PythonLibs 2.7 REQUIRED) + include_directories(${PYTHON_INCLUDE_DIRS}) + find_package(Eigency REQUIRED) + include_directories(${EIGENCY_INCLUDE_DIRS}) + + add_library(${module_name}_cython MODULE ${generated_cpp_file}) + set_target_properties(${module_name}_cython PROPERTIES LINK_FLAGS "-undefined dynamic_lookup" + OUTPUT_NAME ${module_name} PREFIX "" LIBRARY_OUTPUT_DIRECTORY ${generated_files_path}) + target_link_libraries(${module_name}_cython ${module_name}) + add_dependencies(${module_name}_cython ${module_name}_cython_wrapper) + + # distclean + add_custom_target(wrap_${module_name}_cython_distclean COMMAND cmake -E remove_directory ${generated_files_path}) endfunction() diff --git a/cython/CMakeLists.txt b/cython/CMakeLists.txt index e04f29608..ea00e18d5 100644 --- a/cython/CMakeLists.txt +++ b/cython/CMakeLists.txt @@ -1,15 +1,30 @@ # Install cython components include(GtsamCythonWrap) -# install scripts and tests -install_cython_scripts("${PROJECT_SOURCE_DIR}/cython/gtsam" "${GTSAM_CYTHON_INSTALL_PATH}" "*.py") +# Create the cython toolbox for the gtsam library +if (GTSAM_INSTALL_CYTHON_TOOLBOX) + wrap_and_install_library_cython("../gtsam.h" # interface_header + "" # extra imports + "${GTSAM_CYTHON_INSTALL_PATH}/gtsam" # install path + gtsam # dependencies which need to be built before the wrapper + ) -# generate __init__.py into build folder (configured with or without gtsam_unstable import line) -# This also makes the build/cython/gtsam folder a python package, so gtsam can be found while wrapping gtsam_unstable -if(GTSAM_BUILD_UNSTABLE) - set(GTSAM_UNSTABLE_IMPORT "from gtsam_unstable import *") -endif() -configure_file(${PROJECT_SOURCE_DIR}/cython/gtsam/__init__.py.in ${PROJECT_BINARY_DIR}/cython/gtsam/__init__.py) + # wrap gtsam_unstable + if(GTSAM_BUILD_UNSTABLE) + set(GTSAM_UNSTABLE_IMPORT "from gtsam_unstable import *") + wrap_and_install_library_cython("../gtsam_unstable/gtsam_unstable.h" # interface_header + "from gtsam.gtsam cimport *" # extra imports + "${GTSAM_CYTHON_INSTALL_PATH}/gtsam" # install path + gtsam_unstable # dependencies which need to be built before the wrapper + ) + add_dependencies(gtsam_unstable_cython_wrapper gtsam_cython_wrapper) + endif() -# Install the custom-generated __init__.py -install_cython_files("${PROJECT_BINARY_DIR}/cython/gtsam/__init__.py" "${GTSAM_CYTHON_INSTALL_PATH}/gtsam") + # Install the custom-generated __init__.py with gtsam_unstable disabled + # This is to make the build/cython/gtsam folder a python package, so gtsam can be found while wrapping gtsam_unstable + configure_file(${PROJECT_SOURCE_DIR}/cython/gtsam/__init__.py.in ${PROJECT_BINARY_DIR}/cython/gtsam/__init__.py) + install_cython_files("${PROJECT_BINARY_DIR}/cython/gtsam/__init__.py" "${GTSAM_CYTHON_INSTALL_PATH}/gtsam") + # install scripts and tests + install_cython_scripts("${PROJECT_SOURCE_DIR}/cython/gtsam" "${GTSAM_CYTHON_INSTALL_PATH}" "*.py") + +endif () diff --git a/gtsam/CMakeLists.txt b/gtsam/CMakeLists.txt index 8e8302ba5..ed80ff119 100644 --- a/gtsam/CMakeLists.txt +++ b/gtsam/CMakeLists.txt @@ -166,16 +166,3 @@ if (GTSAM_INSTALL_MATLAB_TOOLBOX) wrap_and_install_library(../gtsam.h "${GTSAM_ADDITIONAL_LIBRARIES}" "" "${mexFlags}") endif () -# Create the cython toolbox for the gtsam library -if (GTSAM_INSTALL_CYTHON_TOOLBOX) - # Set up codegen - include(GtsamCythonWrap) - - # Wrap - wrap_and_install_library_cython("../gtsam.h" # interface_header - "" # extra imports - "../cython/gtsam" # path to setup.py.in - "${GTSAM_CYTHON_INSTALL_PATH}/gtsam" # install path - gtsam # dependencies which need to be built before the wrapper - ) -endif () diff --git a/gtsam_unstable/CMakeLists.txt b/gtsam_unstable/CMakeLists.txt index bcf53946c..448e8b00e 100644 --- a/gtsam_unstable/CMakeLists.txt +++ b/gtsam_unstable/CMakeLists.txt @@ -118,21 +118,6 @@ if (GTSAM_INSTALL_MATLAB_TOOLBOX) wrap_and_install_library(gtsam_unstable.h "gtsam" "" "${mexFlags}") endif(GTSAM_INSTALL_MATLAB_TOOLBOX) -# Cython wrap for gtsam_unstable -# Create the matlab toolbox for the gtsam library -if (GTSAM_INSTALL_CYTHON_TOOLBOX) - # Set up codegen - include(GtsamCythonWrap) - - # Wrap - wrap_and_install_library_cython("../gtsam_unstable/gtsam_unstable.h" # interface_header - "from gtsam.gtsam cimport *" # extra imports - "../cython/gtsam_unstable" # path to setup.py.in - "${GTSAM_CYTHON_INSTALL_PATH}/gtsam" # install path - gtsam_unstable # dependencies which need to be built before the wrapper - ) - add_dependencies(gtsam_unstable_cython_wrapper gtsam_cython_wrapper) -endif () # Build examples add_subdirectory(examples) From 504022a514ca71899e41947567f1856c22dea532 Mon Sep 17 00:00:00 2001 From: Duy-Nguyen Ta Date: Wed, 24 May 2017 23:56:08 +0800 Subject: [PATCH 4/4] remove now redundant setup.py.in Not used anymore with the manual cython compiling process --- cython/gtsam/setup.py.in | 24 ------------------------ cython/gtsam_unstable/setup.py.in | 29 ----------------------------- 2 files changed, 53 deletions(-) delete mode 100644 cython/gtsam/setup.py.in delete mode 100644 cython/gtsam_unstable/setup.py.in diff --git a/cython/gtsam/setup.py.in b/cython/gtsam/setup.py.in deleted file mode 100644 index 1ae067690..000000000 --- a/cython/gtsam/setup.py.in +++ /dev/null @@ -1,24 +0,0 @@ -from distutils.core import setup -from distutils.extension import Extension -from Cython.Build import cythonize -import eigency -import sys -import os - -os.environ["CXX"] = "${CMAKE_CXX_COMPILER}" -os.environ["CC"] = "${CMAKE_C_COMPILER}" - -setup( - ext_modules = cythonize(Extension( - "gtsam", - sources=["gtsam.pyx"], - include_dirs = ["${PROJECT_SOURCE_DIR}", "${CMAKE_BINARY_DIR}", - "${PROJECT_SOURCE_DIR}/gtsam/3rdparty/Eigen", - "${Boost_INCLUDE_DIR}" - ] + eigency.get_includes(include_eigen=False), - libraries = ['gtsam${gtsam_library_postfix}'], - library_dirs = ["${CMAKE_CURRENT_BINARY_DIR}"], - language="c++", - extra_compile_args="${CMAKE_CXX_FLAGS}".split(), - extra_link_args="${CMAKE_SHARED_LINKER_FLAGS}".split())) -) diff --git a/cython/gtsam_unstable/setup.py.in b/cython/gtsam_unstable/setup.py.in deleted file mode 100644 index d7057cec7..000000000 --- a/cython/gtsam_unstable/setup.py.in +++ /dev/null @@ -1,29 +0,0 @@ -from distutils.core import setup -from distutils.extension import Extension -from Cython.Build import cythonize -import eigency -import sys -import os - -# so that it can find the wrapped gtsam package -sys.path.append("..") - -os.environ["CXX"] = "${CMAKE_CXX_COMPILER}" -os.environ["CC"] = "${CMAKE_C_COMPILER}" - -setup( - ext_modules=cythonize(Extension( - "gtsam_unstable", - sources=["gtsam_unstable.pyx"], - include_dirs = ["${CMAKE_SOURCE_DIR}", "${CMAKE_BINARY_DIR}", - "${CMAKE_SOURCE_DIR}/gtsam/3rdparty/Eigen", - "${Boost_INCLUDE_DIR}" - ] + eigency.get_includes(include_eigen=False), - libraries=['gtsam${gtsam_library_postfix}', 'gtsam_unstable${gtsam_library_postfix}'], - library_dirs = ["${CMAKE_BINARY_DIR}/gtsam", - "${CMAKE_BINARY_DIR}/gtsam_unstable"], - language="c++", - extra_compile_args="${CMAKE_CXX_FLAGS}".split(), - extra_link_args="${CMAKE_SHARED_LINKER_FLAGS}".split())) -) -