From 1725a577cf881ae5478dbe08cef19b4b3f5a4f55 Mon Sep 17 00:00:00 2001 From: Varun Agrawal Date: Sat, 21 Mar 2020 14:52:17 -0400 Subject: [PATCH 01/29] cmake function to install python package once make install is completed --- cmake/GtsamCythonWrap.cmake | 7 +++++++ cython/CMakeLists.txt | 2 ++ 2 files changed, 9 insertions(+) diff --git a/cmake/GtsamCythonWrap.cmake b/cmake/GtsamCythonWrap.cmake index f1382729f..3ca8b903f 100644 --- a/cmake/GtsamCythonWrap.cmake +++ b/cmake/GtsamCythonWrap.cmake @@ -279,3 +279,10 @@ function(install_cython_files source_files dest_directory) endfunction() +function(install_python_package install_path) + set(package_path "${install_path}${GTSAM_BUILD_TAG}") + # set cython directory permissions to user so we don't get permission denied + install(CODE "execute_process(COMMAND sh \"-c\" \"chown -R $(logname):$(logname) ${package_path}\")") + # go to cython directory and run setup.py + install(CODE "execute_process(COMMAND sh \"-c\" \"cd ${package_path} && python setup.py install\")") +endfunction() diff --git a/cython/CMakeLists.txt b/cython/CMakeLists.txt index 4cc9d2f5d..96503b82f 100644 --- a/cython/CMakeLists.txt +++ b/cython/CMakeLists.txt @@ -45,4 +45,6 @@ if (GTSAM_INSTALL_CYTHON_TOOLBOX) install_cython_scripts("${PROJECT_SOURCE_DIR}/cython/gtsam" "${GTSAM_CYTHON_INSTALL_PATH}" "*.py") install_cython_scripts("${PROJECT_SOURCE_DIR}/cython/gtsam_unstable" "${GTSAM_CYTHON_INSTALL_PATH}" "*.py") + install_python_package("${GTSAM_CYTHON_INSTALL_PATH}") + endif () From 93a00a38a40a21e21a1c20da9209192a7076fc85 Mon Sep 17 00:00:00 2001 From: Varun Agrawal Date: Mon, 22 Jun 2020 20:14:03 -0500 Subject: [PATCH 02/29] add new make command for installing python wrapper --- cmake/GtsamCythonWrap.cmake | 17 ++++++++++++----- cython/CMakeLists.txt | 3 +++ cython/scripts/install.bat | 0 cython/scripts/install.sh | 21 +++++++++++++++++++++ 4 files changed, 36 insertions(+), 5 deletions(-) create mode 100755 cython/scripts/install.bat create mode 100755 cython/scripts/install.sh diff --git a/cmake/GtsamCythonWrap.cmake b/cmake/GtsamCythonWrap.cmake index 3ca8b903f..361038ce0 100644 --- a/cmake/GtsamCythonWrap.cmake +++ b/cmake/GtsamCythonWrap.cmake @@ -280,9 +280,16 @@ function(install_cython_files source_files dest_directory) endfunction() function(install_python_package install_path) - set(package_path "${install_path}${GTSAM_BUILD_TAG}") - # set cython directory permissions to user so we don't get permission denied - install(CODE "execute_process(COMMAND sh \"-c\" \"chown -R $(logname):$(logname) ${package_path}\")") - # go to cython directory and run setup.py - install(CODE "execute_process(COMMAND sh \"-c\" \"cd ${package_path} && python setup.py install\")") +#TODO this will only work for Linux. Need to make it work on macOS and Windows as well +#TODO Running `sudo make install` makes this run in admin space causing Python 2.7 to be picked up. + # # go to cython directory and run setup.py + # install(CODE "execute_process(COMMAND sh \"-c\" \"cd ${package_path} && python setup.py install\")") + if(CMAKE_SYSTEM_NAME STREQUAL "Windows") + set(PYTHON_INSTALL_SCRIPT "install.bat") + elseif(CMAKE_SYSTEM_NAME STREQUAL "Linux" OR CMAKE_SYSTEM_NAME STREQUAL "Darwin") + set(PYTHON_INSTALL_SCRIPT "install.sh") + endif() + + configure_file(${PROJECT_SOURCE_DIR}/cython/scripts/${PYTHON_INSTALL_SCRIPT} ${PROJECT_BINARY_DIR}/cython/scripts/${PYTHON_INSTALL_SCRIPT}) + add_custom_target(python-install "${PROJECT_BINARY_DIR}/cython/scripts/${PYTHON_INSTALL_SCRIPT}") endfunction() diff --git a/cython/CMakeLists.txt b/cython/CMakeLists.txt index 96503b82f..638bdfb2d 100644 --- a/cython/CMakeLists.txt +++ b/cython/CMakeLists.txt @@ -45,6 +45,9 @@ if (GTSAM_INSTALL_CYTHON_TOOLBOX) install_cython_scripts("${PROJECT_SOURCE_DIR}/cython/gtsam" "${GTSAM_CYTHON_INSTALL_PATH}" "*.py") install_cython_scripts("${PROJECT_SOURCE_DIR}/cython/gtsam_unstable" "${GTSAM_CYTHON_INSTALL_PATH}" "*.py") + # file(GLOB GTSAM_PYTHON_INSTALL_SCRIPTS "scripts/*") + # file(COPY ${GTSAM_PYTHON_INSTALL_SCRIPTS} DESTINATION ${PROJECT_BINARY_DIR}/cython/scripts) + install_python_package("${GTSAM_CYTHON_INSTALL_PATH}") endif () diff --git a/cython/scripts/install.bat b/cython/scripts/install.bat new file mode 100755 index 000000000..e69de29bb diff --git a/cython/scripts/install.sh b/cython/scripts/install.sh new file mode 100755 index 000000000..04759ca5e --- /dev/null +++ b/cython/scripts/install.sh @@ -0,0 +1,21 @@ +#!/bin/sh +echo "Installing GTSAM Python Wrapper" + +PACKAGE_PATH=${GTSAM_CYTHON_INSTALL_PATH}${GTSAM_BUILD_TAG} + +if [ ! -d "$PACKAGE_PATH" ] +then + echo "Directory $PACKAGE_PATH DOES NOT exist. Please run 'make install' first."; + exit 1; +fi + +# set cython directory permissions to user so we don't get permission denied +if [ "$(whoami)" != "root" ] +then + sudo chown -R $(logname) ${GTSAM_CYTHON_INSTALL_PATH} +else + chown -R $(logname) ${GTSAM_CYTHON_INSTALL_PATH} +fi + +echo "Running setup.py in $PACKAGE_PATH" +${PYTHON_EXECUTABLE} $PACKAGE_PATH/setup.py install From ca46ebfda81d8b54b1098cce83a41f0c664ee8b8 Mon Sep 17 00:00:00 2001 From: Varun Agrawal Date: Mon, 22 Jun 2020 20:20:50 -0500 Subject: [PATCH 03/29] added comments and removed unnecessary code --- cmake/GtsamCythonWrap.cmake | 8 ++++---- cython/CMakeLists.txt | 3 --- cython/scripts/install.sh | 10 +++++++++- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/cmake/GtsamCythonWrap.cmake b/cmake/GtsamCythonWrap.cmake index 361038ce0..4cd061852 100644 --- a/cmake/GtsamCythonWrap.cmake +++ b/cmake/GtsamCythonWrap.cmake @@ -280,16 +280,16 @@ function(install_cython_files source_files dest_directory) endfunction() function(install_python_package install_path) -#TODO this will only work for Linux. Need to make it work on macOS and Windows as well -#TODO Running `sudo make install` makes this run in admin space causing Python 2.7 to be picked up. - # # go to cython directory and run setup.py - # install(CODE "execute_process(COMMAND sh \"-c\" \"cd ${package_path} && python setup.py install\")") + # Select the correct install script based on the OS if(CMAKE_SYSTEM_NAME STREQUAL "Windows") set(PYTHON_INSTALL_SCRIPT "install.bat") elseif(CMAKE_SYSTEM_NAME STREQUAL "Linux" OR CMAKE_SYSTEM_NAME STREQUAL "Darwin") set(PYTHON_INSTALL_SCRIPT "install.sh") endif() + # Configure the variables in the script configure_file(${PROJECT_SOURCE_DIR}/cython/scripts/${PYTHON_INSTALL_SCRIPT} ${PROJECT_BINARY_DIR}/cython/scripts/${PYTHON_INSTALL_SCRIPT}) + + # Add the new make target command add_custom_target(python-install "${PROJECT_BINARY_DIR}/cython/scripts/${PYTHON_INSTALL_SCRIPT}") endfunction() diff --git a/cython/CMakeLists.txt b/cython/CMakeLists.txt index 638bdfb2d..96503b82f 100644 --- a/cython/CMakeLists.txt +++ b/cython/CMakeLists.txt @@ -45,9 +45,6 @@ if (GTSAM_INSTALL_CYTHON_TOOLBOX) install_cython_scripts("${PROJECT_SOURCE_DIR}/cython/gtsam" "${GTSAM_CYTHON_INSTALL_PATH}" "*.py") install_cython_scripts("${PROJECT_SOURCE_DIR}/cython/gtsam_unstable" "${GTSAM_CYTHON_INSTALL_PATH}" "*.py") - # file(GLOB GTSAM_PYTHON_INSTALL_SCRIPTS "scripts/*") - # file(COPY ${GTSAM_PYTHON_INSTALL_SCRIPTS} DESTINATION ${PROJECT_BINARY_DIR}/cython/scripts) - install_python_package("${GTSAM_CYTHON_INSTALL_PATH}") endif () diff --git a/cython/scripts/install.sh b/cython/scripts/install.sh index 04759ca5e..8e409803d 100755 --- a/cython/scripts/install.sh +++ b/cython/scripts/install.sh @@ -1,15 +1,22 @@ #!/bin/sh + +# This script runs the installation flow for python wrapped GTSAM. +# It does so by first setting the correct ownership permissions on the package directory, +# and then running `python setup.py install` to install the wrapped package. + echo "Installing GTSAM Python Wrapper" +# Set the package path PACKAGE_PATH=${GTSAM_CYTHON_INSTALL_PATH}${GTSAM_BUILD_TAG} +# Check if package directory exists. If not, print warning and exit. if [ ! -d "$PACKAGE_PATH" ] then echo "Directory $PACKAGE_PATH DOES NOT exist. Please run 'make install' first."; exit 1; fi -# set cython directory permissions to user so we don't get permission denied +# Set cython directory permissions to user so we don't get permission denied if [ "$(whoami)" != "root" ] then sudo chown -R $(logname) ${GTSAM_CYTHON_INSTALL_PATH} @@ -17,5 +24,6 @@ else chown -R $(logname) ${GTSAM_CYTHON_INSTALL_PATH} fi +# Run setup.py install with full paths echo "Running setup.py in $PACKAGE_PATH" ${PYTHON_EXECUTABLE} $PACKAGE_PATH/setup.py install From 6972a5c9a70e5ff8eaa3f8b5b0f9392c2829a195 Mon Sep 17 00:00:00 2001 From: Varun Agrawal Date: Wed, 24 Jun 2020 11:06:01 -0500 Subject: [PATCH 04/29] updated comments in shell script --- cython/scripts/install.sh | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/cython/scripts/install.sh b/cython/scripts/install.sh index 8e409803d..fdf86f130 100755 --- a/cython/scripts/install.sh +++ b/cython/scripts/install.sh @@ -11,12 +11,13 @@ PACKAGE_PATH=${GTSAM_CYTHON_INSTALL_PATH}${GTSAM_BUILD_TAG} # Check if package directory exists. If not, print warning and exit. if [ ! -d "$PACKAGE_PATH" ] -then +then echo "Directory $PACKAGE_PATH DOES NOT exist. Please run 'make install' first."; exit 1; fi -# Set cython directory permissions to user so we don't get permission denied +# Set cython directory permissions to user so we don't get permission denied. +# This also works inside Docker containers. if [ "$(whoami)" != "root" ] then sudo chown -R $(logname) ${GTSAM_CYTHON_INSTALL_PATH} @@ -24,6 +25,6 @@ else chown -R $(logname) ${GTSAM_CYTHON_INSTALL_PATH} fi -# Run setup.py install with full paths +# Run setup.py install with full paths. echo "Running setup.py in $PACKAGE_PATH" ${PYTHON_EXECUTABLE} $PACKAGE_PATH/setup.py install From 530016edf00e9a56cbdb4de27ce612580337a66a Mon Sep 17 00:00:00 2001 From: Varun Agrawal Date: Wed, 24 Jun 2020 11:06:35 -0500 Subject: [PATCH 05/29] added Windows batch script to install python wrapped package --- cython/scripts/install.bat | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/cython/scripts/install.bat b/cython/scripts/install.bat index e69de29bb..4dbd2051f 100755 --- a/cython/scripts/install.bat +++ b/cython/scripts/install.bat @@ -0,0 +1,18 @@ +:: This script runs the installation flow for python wrapped GTSAM. +:: It does so by running `python setup.py install` to install the wrapped package. + +echo "Installing GTSAM Python Wrapper" + +:: Set the package path +PACKAGE_PATH=${GTSAM_CYTHON_INSTALL_PATH}${GTSAM_BUILD_TAG} + +:: Check if package directory exists. If not, print warning and exit. +if [ ! -d "$PACKAGE_PATH" ] +then + echo "Directory $PACKAGE_PATH DOES NOT exist. Please run 'make install' first."; + exit 1; +fi + +:: Run setup.py install with full paths. +echo "Running setup.py in $PACKAGE_PATH" +${PYTHON_EXECUTABLE} $PACKAGE_PATH/setup.py install From efde078b944e1865ea9e53e68726c48c5cf2d2e7 Mon Sep 17 00:00:00 2001 From: Varun Agrawal Date: Wed, 24 Jun 2020 14:05:17 -0500 Subject: [PATCH 06/29] pure CMake script to install Python wrapper after compiling --- cmake/GtsamCythonWrap.cmake | 15 --------------- cython/CMakeLists.txt | 3 ++- cython/scripts/install.bat | 18 ------------------ cython/scripts/install.sh | 30 ------------------------------ 4 files changed, 2 insertions(+), 64 deletions(-) delete mode 100755 cython/scripts/install.bat delete mode 100755 cython/scripts/install.sh diff --git a/cmake/GtsamCythonWrap.cmake b/cmake/GtsamCythonWrap.cmake index 4cd061852..6331d1e95 100644 --- a/cmake/GtsamCythonWrap.cmake +++ b/cmake/GtsamCythonWrap.cmake @@ -278,18 +278,3 @@ function(install_cython_files source_files dest_directory) endif() endfunction() - -function(install_python_package install_path) - # Select the correct install script based on the OS - if(CMAKE_SYSTEM_NAME STREQUAL "Windows") - set(PYTHON_INSTALL_SCRIPT "install.bat") - elseif(CMAKE_SYSTEM_NAME STREQUAL "Linux" OR CMAKE_SYSTEM_NAME STREQUAL "Darwin") - set(PYTHON_INSTALL_SCRIPT "install.sh") - endif() - - # Configure the variables in the script - configure_file(${PROJECT_SOURCE_DIR}/cython/scripts/${PYTHON_INSTALL_SCRIPT} ${PROJECT_BINARY_DIR}/cython/scripts/${PYTHON_INSTALL_SCRIPT}) - - # Add the new make target command - add_custom_target(python-install "${PROJECT_BINARY_DIR}/cython/scripts/${PYTHON_INSTALL_SCRIPT}") -endfunction() diff --git a/cython/CMakeLists.txt b/cython/CMakeLists.txt index 96503b82f..bce9f2308 100644 --- a/cython/CMakeLists.txt +++ b/cython/CMakeLists.txt @@ -45,6 +45,7 @@ if (GTSAM_INSTALL_CYTHON_TOOLBOX) install_cython_scripts("${PROJECT_SOURCE_DIR}/cython/gtsam" "${GTSAM_CYTHON_INSTALL_PATH}" "*.py") install_cython_scripts("${PROJECT_SOURCE_DIR}/cython/gtsam_unstable" "${GTSAM_CYTHON_INSTALL_PATH}" "*.py") - install_python_package("${GTSAM_CYTHON_INSTALL_PATH}") + # Add the new make target command + add_custom_target(python-install COMMAND ${PYTHON_EXECUTABLE} ${GTSAM_CYTHON_INSTALL_PATH}${GTSAM_BUILD_TAG}/setup.py install) endif () diff --git a/cython/scripts/install.bat b/cython/scripts/install.bat deleted file mode 100755 index 4dbd2051f..000000000 --- a/cython/scripts/install.bat +++ /dev/null @@ -1,18 +0,0 @@ -:: This script runs the installation flow for python wrapped GTSAM. -:: It does so by running `python setup.py install` to install the wrapped package. - -echo "Installing GTSAM Python Wrapper" - -:: Set the package path -PACKAGE_PATH=${GTSAM_CYTHON_INSTALL_PATH}${GTSAM_BUILD_TAG} - -:: Check if package directory exists. If not, print warning and exit. -if [ ! -d "$PACKAGE_PATH" ] -then - echo "Directory $PACKAGE_PATH DOES NOT exist. Please run 'make install' first."; - exit 1; -fi - -:: Run setup.py install with full paths. -echo "Running setup.py in $PACKAGE_PATH" -${PYTHON_EXECUTABLE} $PACKAGE_PATH/setup.py install diff --git a/cython/scripts/install.sh b/cython/scripts/install.sh deleted file mode 100755 index fdf86f130..000000000 --- a/cython/scripts/install.sh +++ /dev/null @@ -1,30 +0,0 @@ -#!/bin/sh - -# This script runs the installation flow for python wrapped GTSAM. -# It does so by first setting the correct ownership permissions on the package directory, -# and then running `python setup.py install` to install the wrapped package. - -echo "Installing GTSAM Python Wrapper" - -# Set the package path -PACKAGE_PATH=${GTSAM_CYTHON_INSTALL_PATH}${GTSAM_BUILD_TAG} - -# Check if package directory exists. If not, print warning and exit. -if [ ! -d "$PACKAGE_PATH" ] -then - echo "Directory $PACKAGE_PATH DOES NOT exist. Please run 'make install' first."; - exit 1; -fi - -# Set cython directory permissions to user so we don't get permission denied. -# This also works inside Docker containers. -if [ "$(whoami)" != "root" ] -then - sudo chown -R $(logname) ${GTSAM_CYTHON_INSTALL_PATH} -else - chown -R $(logname) ${GTSAM_CYTHON_INSTALL_PATH} -fi - -# Run setup.py install with full paths. -echo "Running setup.py in $PACKAGE_PATH" -${PYTHON_EXECUTABLE} $PACKAGE_PATH/setup.py install From 9698b032537e9a74a6414ff46e09f9cdef196bab Mon Sep 17 00:00:00 2001 From: Varun Agrawal Date: Wed, 24 Jun 2020 14:06:25 -0500 Subject: [PATCH 07/29] removed extra line --- cmake/GtsamCythonWrap.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/GtsamCythonWrap.cmake b/cmake/GtsamCythonWrap.cmake index 6331d1e95..b6c4c2856 100644 --- a/cmake/GtsamCythonWrap.cmake +++ b/cmake/GtsamCythonWrap.cmake @@ -277,4 +277,4 @@ function(install_cython_files source_files dest_directory) install(FILES "${source_files}" DESTINATION "${dest_directory}") endif() -endfunction() +endfunction() \ No newline at end of file From 5feaf6dd9da35b9a347dc5c0d006c72ab0a3f2d4 Mon Sep 17 00:00:00 2001 From: Varun Agrawal Date: Wed, 24 Jun 2020 14:07:28 -0500 Subject: [PATCH 08/29] reset to previous version --- cmake/GtsamCythonWrap.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/GtsamCythonWrap.cmake b/cmake/GtsamCythonWrap.cmake index b6c4c2856..6331d1e95 100644 --- a/cmake/GtsamCythonWrap.cmake +++ b/cmake/GtsamCythonWrap.cmake @@ -277,4 +277,4 @@ function(install_cython_files source_files dest_directory) install(FILES "${source_files}" DESTINATION "${dest_directory}") endif() -endfunction() \ No newline at end of file +endfunction() From 2475e6c68c6581cce518f22c8d6683c857b0fc1b Mon Sep 17 00:00:00 2001 From: Varun Agrawal Date: Wed, 24 Jun 2020 17:44:03 -0500 Subject: [PATCH 09/29] Load Cython requirements file instead of reading it in cmake --- cython/CMakeLists.txt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cython/CMakeLists.txt b/cython/CMakeLists.txt index bce9f2308..0d2af6a33 100644 --- a/cython/CMakeLists.txt +++ b/cython/CMakeLists.txt @@ -32,8 +32,7 @@ if (GTSAM_INSTALL_CYTHON_TOOLBOX) ) endif() - file(READ "${PROJECT_SOURCE_DIR}/cython/requirements.txt" CYTHON_INSTALL_REQUIREMENTS) - file(READ "${PROJECT_SOURCE_DIR}/README.md" README_CONTENTS) + set(CYTHON_INSTALL_REQUIREMENTS_FILE "${PROJECT_SOURCE_DIR}/cython/requirements.txt") # Install the custom-generated __init__.py # This is to make the build/cython/gtsam folder a python package, so gtsam can be found while wrapping gtsam_unstable From 453d3a74164613375a083566eea9f8b16c782d74 Mon Sep 17 00:00:00 2001 From: Varun Agrawal Date: Wed, 24 Jun 2020 18:11:24 -0500 Subject: [PATCH 10/29] Added cmake variable GTSAM_CYTHON_INSTALL_FULLPATH to include build tag directly --- CMakeLists.txt | 4 +++- cmake/GtsamCythonWrap.cmake | 2 +- cython/gtsam_eigency/CMakeLists.txt | 8 ++++---- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index a810ac9df..2cbdbf00c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -458,7 +458,9 @@ if (GTSAM_INSTALL_CYTHON_TOOLBOX) if(NOT GTSAM_CYTHON_INSTALL_PATH) set(GTSAM_CYTHON_INSTALL_PATH "${CMAKE_INSTALL_PREFIX}/cython") endif() - set(GTSAM_EIGENCY_INSTALL_PATH ${GTSAM_CYTHON_INSTALL_PATH}/gtsam_eigency) + # Cython install path appended with Build type (e.g. cython, cythonDebug, etc). + set(GTSAM_CYTHON_INSTALL_FULLPATH "${GTSAM_CYTHON_INSTALL_PATH}${GTSAM_BUILD_TAG}") + set(GTSAM_EIGENCY_INSTALL_PATH ${GTSAM_CYTHON_INSTALL_FULLPATH}/gtsam_eigency) add_subdirectory(cython) else() set(GTSAM_INSTALL_CYTHON_TOOLBOX 0) # This will go into config.h diff --git a/cmake/GtsamCythonWrap.cmake b/cmake/GtsamCythonWrap.cmake index 6331d1e95..851f53cfe 100644 --- a/cmake/GtsamCythonWrap.cmake +++ b/cmake/GtsamCythonWrap.cmake @@ -184,7 +184,7 @@ function(install_cython_wrapped_library interface_header generated_files_path in # Split up filename to strip trailing '/' in GTSAM_CYTHON_INSTALL_PATH/subdirectory if there is one get_filename_component(location "${install_path}" PATH) get_filename_component(name "${install_path}" NAME) - message(STATUS "Installing Cython Toolbox to ${location}${GTSAM_BUILD_TAG}/${name}") #${GTSAM_CYTHON_INSTALL_PATH}" + message(STATUS "Installing Cython Toolbox to ${location}${GTSAM_BUILD_TAG}/${name}") #${GTSAM_CYTHON_INSTALL_FULLPATH}" if(GTSAM_BUILD_TYPE_POSTFIXES) foreach(build_type ${CMAKE_CONFIGURATION_TYPES}) diff --git a/cython/gtsam_eigency/CMakeLists.txt b/cython/gtsam_eigency/CMakeLists.txt index 77bead834..da174d690 100644 --- a/cython/gtsam_eigency/CMakeLists.txt +++ b/cython/gtsam_eigency/CMakeLists.txt @@ -39,11 +39,11 @@ add_dependencies(cythonize_eigency cythonize_eigency_conversions cythonize_eigen # install install(DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} - DESTINATION "${GTSAM_CYTHON_INSTALL_PATH}${GTSAM_BUILD_TAG}" + DESTINATION "${GTSAM_CYTHON_INSTALL_FULLPATH}" PATTERN "CMakeLists.txt" EXCLUDE PATTERN "__init__.py.in" EXCLUDE) install(TARGETS cythonize_eigency_core cythonize_eigency_conversions - DESTINATION "${GTSAM_CYTHON_INSTALL_PATH}${GTSAM_BUILD_TAG}/gtsam_eigency") -install(FILES ${OUTPUT_DIR}/conversions_api.h DESTINATION ${GTSAM_CYTHON_INSTALL_PATH}${GTSAM_BUILD_TAG}/gtsam_eigency) + DESTINATION "${GTSAM_CYTHON_INSTALL_FULLPATH}/gtsam_eigency") +install(FILES ${OUTPUT_DIR}/conversions_api.h DESTINATION ${GTSAM_CYTHON_INSTALL_FULLPATH}/gtsam_eigency) configure_file(__init__.py.in ${OUTPUT_DIR}/__init__.py) -install(FILES ${OUTPUT_DIR}/__init__.py DESTINATION ${GTSAM_CYTHON_INSTALL_PATH}${GTSAM_BUILD_TAG}/gtsam_eigency) +install(FILES ${OUTPUT_DIR}/__init__.py DESTINATION ${GTSAM_CYTHON_INSTALL_FULLPATH}/gtsam_eigency) From 4f6f8216110a2db9d7a9be3fbdba65a967490793 Mon Sep 17 00:00:00 2001 From: Varun Agrawal Date: Wed, 24 Jun 2020 18:39:49 -0500 Subject: [PATCH 11/29] Vastly improved setup.py template --- cython/setup.py.in | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/cython/setup.py.in b/cython/setup.py.in index df92b564c..98a05c9f6 100644 --- a/cython/setup.py.in +++ b/cython/setup.py.in @@ -7,6 +7,22 @@ except ImportError: packages = find_packages() +package_data = { + package: + [f for f in os.listdir(package.replace('.', os.path.sep)) if os.path.splitext(f)[1] in ('.so', '.pyd')] + for package in packages +} + +cython_install_requirements = open("${CYTHON_INSTALL_REQUIREMENTS_FILE}").readlines() + +install_requires = [line.strip() \ + for line in cython_install_requirements \ + if len(line.strip()) > 0 and not line.strip().startswith('#') +] + +# Cleaner to read in the contents rather than copy them over. +readme_contents = open("${PROJECT_SOURCE_DIR}/README.md").read() + setup( name='gtsam', description='Georgia Tech Smoothing And Mapping library', @@ -16,7 +32,7 @@ setup( author_email='frank.dellaert@gtsam.org', license='Simplified BSD license', keywords='slam sam robotics localization mapping optimization', - long_description='''${README_CONTENTS}''', + long_description=readme_contents, long_description_content_type='text/markdown', python_requires='>=2.7', # https://pypi.org/pypi?%3Aaction=list_classifiers @@ -34,11 +50,6 @@ setup( ], packages=packages, - package_data={package: - [f for f in os.listdir(package.replace('.', os.path.sep)) if os.path.splitext(f)[1] in ('.so', '.pyd')] - for package in packages - }, - install_requires=[line.strip() for line in ''' -${CYTHON_INSTALL_REQUIREMENTS} -'''.splitlines() if len(line.strip()) > 0 and not line.strip().startswith('#')] + package_data=package_data, + install_requires=install_requires ) From 192184b3b7c1efacb5a7608a7da4205fbba3536f Mon Sep 17 00:00:00 2001 From: Varun Agrawal Date: Wed, 24 Jun 2020 18:40:03 -0500 Subject: [PATCH 12/29] Specify working directory from where to call setup.py --- cython/CMakeLists.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cython/CMakeLists.txt b/cython/CMakeLists.txt index 0d2af6a33..01b6c06d4 100644 --- a/cython/CMakeLists.txt +++ b/cython/CMakeLists.txt @@ -45,6 +45,8 @@ if (GTSAM_INSTALL_CYTHON_TOOLBOX) install_cython_scripts("${PROJECT_SOURCE_DIR}/cython/gtsam_unstable" "${GTSAM_CYTHON_INSTALL_PATH}" "*.py") # Add the new make target command - add_custom_target(python-install COMMAND ${PYTHON_EXECUTABLE} ${GTSAM_CYTHON_INSTALL_PATH}${GTSAM_BUILD_TAG}/setup.py install) + add_custom_target(python-install + COMMAND ${PYTHON_EXECUTABLE} ${GTSAM_CYTHON_INSTALL_PATH}${GTSAM_BUILD_TAG}/setup.py install + WORKING_DIRECTORY ${GTSAM_CYTHON_INSTALL_FULLPATH}) endif () From 54c29031839fe00d559e97e4b2014927667180d2 Mon Sep 17 00:00:00 2001 From: Varun Agrawal Date: Mon, 29 Jun 2020 16:53:42 -0500 Subject: [PATCH 13/29] make python-install command depends on gtsam target --- cython/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/cython/CMakeLists.txt b/cython/CMakeLists.txt index 01b6c06d4..2bfa8ae7c 100644 --- a/cython/CMakeLists.txt +++ b/cython/CMakeLists.txt @@ -47,6 +47,7 @@ if (GTSAM_INSTALL_CYTHON_TOOLBOX) # Add the new make target command add_custom_target(python-install COMMAND ${PYTHON_EXECUTABLE} ${GTSAM_CYTHON_INSTALL_PATH}${GTSAM_BUILD_TAG}/setup.py install + DEPENDS gtsam WORKING_DIRECTORY ${GTSAM_CYTHON_INSTALL_FULLPATH}) endif () From 806e5b12a37462481f1d1e656835b0beab89a6f1 Mon Sep 17 00:00:00 2001 From: Varun Agrawal Date: Mon, 29 Jun 2020 19:29:52 -0500 Subject: [PATCH 14/29] cleaner version of execution script which only needs 'make install' --- cython/CMakeLists.txt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cython/CMakeLists.txt b/cython/CMakeLists.txt index 2bfa8ae7c..75cbfea8a 100644 --- a/cython/CMakeLists.txt +++ b/cython/CMakeLists.txt @@ -45,9 +45,9 @@ if (GTSAM_INSTALL_CYTHON_TOOLBOX) install_cython_scripts("${PROJECT_SOURCE_DIR}/cython/gtsam_unstable" "${GTSAM_CYTHON_INSTALL_PATH}" "*.py") # Add the new make target command - add_custom_target(python-install - COMMAND ${PYTHON_EXECUTABLE} ${GTSAM_CYTHON_INSTALL_PATH}${GTSAM_BUILD_TAG}/setup.py install - DEPENDS gtsam - WORKING_DIRECTORY ${GTSAM_CYTHON_INSTALL_FULLPATH}) + install(CODE + "execute_process(COMMAND ${PYTHON_EXECUTABLE} setup.py install + WORKING_DIRECTORY ${GTSAM_CYTHON_INSTALL_FULLPATH})") + endif () From 16532bff37a5eb991f639d682ae733d2f27650b2 Mon Sep 17 00:00:00 2001 From: Varun Agrawal Date: Mon, 29 Jun 2020 21:37:07 -0500 Subject: [PATCH 15/29] run setup.py after installing the gtsam_eigency module --- cython/CMakeLists.txt | 10 ++++++---- cython/gtsam_eigency/CMakeLists.txt | 22 ++++++++++++---------- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/cython/CMakeLists.txt b/cython/CMakeLists.txt index 75cbfea8a..5569c0e47 100644 --- a/cython/CMakeLists.txt +++ b/cython/CMakeLists.txt @@ -44,10 +44,12 @@ if (GTSAM_INSTALL_CYTHON_TOOLBOX) install_cython_scripts("${PROJECT_SOURCE_DIR}/cython/gtsam" "${GTSAM_CYTHON_INSTALL_PATH}" "*.py") install_cython_scripts("${PROJECT_SOURCE_DIR}/cython/gtsam_unstable" "${GTSAM_CYTHON_INSTALL_PATH}" "*.py") - # Add the new make target command + # Adding custom function here so that gtsam_eigency is installed before + # the below execute_process runs. + install_gtsam_eigency(${PROJECT_BINARY_DIR}/cython/gtsam_eigency) + + # Automatically run the python installation via the setup.py install(CODE "execute_process(COMMAND ${PYTHON_EXECUTABLE} setup.py install - WORKING_DIRECTORY ${GTSAM_CYTHON_INSTALL_FULLPATH})") - - + WORKING_DIRECTORY ${GTSAM_CYTHON_INSTALL_FULLPATH})") endif () diff --git a/cython/gtsam_eigency/CMakeLists.txt b/cython/gtsam_eigency/CMakeLists.txt index da174d690..5ea1c337c 100644 --- a/cython/gtsam_eigency/CMakeLists.txt +++ b/cython/gtsam_eigency/CMakeLists.txt @@ -8,7 +8,7 @@ set(OUTPUT_DIR "${PROJECT_BINARY_DIR}/cython/gtsam_eigency") set(EIGENCY_INCLUDE_DIR ${OUTPUT_DIR}) # This is to make the build/cython/gtsam_eigency folder a python package -configure_file(__init__.py.in ${PROJECT_BINARY_DIR}/cython/gtsam_eigency/__init__.py) +configure_file(__init__.py.in ${OUTPUT_DIR}/__init__.py) # include eigency headers include_directories(${EIGENCY_INCLUDE_DIR}) @@ -38,12 +38,14 @@ add_custom_target(cythonize_eigency) add_dependencies(cythonize_eigency cythonize_eigency_conversions cythonize_eigency_core) # install -install(DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} - DESTINATION "${GTSAM_CYTHON_INSTALL_FULLPATH}" - PATTERN "CMakeLists.txt" EXCLUDE - PATTERN "__init__.py.in" EXCLUDE) -install(TARGETS cythonize_eigency_core cythonize_eigency_conversions - DESTINATION "${GTSAM_CYTHON_INSTALL_FULLPATH}/gtsam_eigency") -install(FILES ${OUTPUT_DIR}/conversions_api.h DESTINATION ${GTSAM_CYTHON_INSTALL_FULLPATH}/gtsam_eigency) -configure_file(__init__.py.in ${OUTPUT_DIR}/__init__.py) -install(FILES ${OUTPUT_DIR}/__init__.py DESTINATION ${GTSAM_CYTHON_INSTALL_FULLPATH}/gtsam_eigency) +function(install_gtsam_eigency source_directory) + install(DIRECTORY ${source_directory} + DESTINATION "${GTSAM_CYTHON_INSTALL_FULLPATH}" + PATTERN "CMakeLists.txt" EXCLUDE + PATTERN "__init__.py.in" EXCLUDE) + install(TARGETS cythonize_eigency_core cythonize_eigency_conversions + DESTINATION "${GTSAM_CYTHON_INSTALL_FULLPATH}/gtsam_eigency") + install(FILES ${source_directory}/conversions_api.h DESTINATION ${GTSAM_CYTHON_INSTALL_FULLPATH}/gtsam_eigency) + install(FILES ${source_directory}/__init__.py DESTINATION ${GTSAM_CYTHON_INSTALL_FULLPATH}/gtsam_eigency) + +endfunction() \ No newline at end of file From 192bf870af28156de5808074f6a45aa36e962410 Mon Sep 17 00:00:00 2001 From: Varun Agrawal Date: Tue, 30 Jun 2020 13:16:09 -0500 Subject: [PATCH 16/29] newline added to end of CMake file --- cython/gtsam_eigency/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cython/gtsam_eigency/CMakeLists.txt b/cython/gtsam_eigency/CMakeLists.txt index 5ea1c337c..6cff534c5 100644 --- a/cython/gtsam_eigency/CMakeLists.txt +++ b/cython/gtsam_eigency/CMakeLists.txt @@ -48,4 +48,4 @@ function(install_gtsam_eigency source_directory) install(FILES ${source_directory}/conversions_api.h DESTINATION ${GTSAM_CYTHON_INSTALL_FULLPATH}/gtsam_eigency) install(FILES ${source_directory}/__init__.py DESTINATION ${GTSAM_CYTHON_INSTALL_FULLPATH}/gtsam_eigency) -endfunction() \ No newline at end of file +endfunction() From 9cbabb2cb6e8bf7407f67cfa8026cd227f069f5d Mon Sep 17 00:00:00 2001 From: Varun Agrawal Date: Tue, 30 Jun 2020 20:45:55 -0500 Subject: [PATCH 17/29] Set high level Cython/Eigency variables to reduce duplication --- CMakeLists.txt | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 2cbdbf00c..f8deebfcd 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -454,13 +454,14 @@ endif() if (GTSAM_INSTALL_CYTHON_TOOLBOX) set(GTSAM_INSTALL_CYTHON_TOOLBOX 1) # Set up cache options - set(GTSAM_CYTHON_INSTALL_PATH "" CACHE PATH "Cython toolbox destination, blank defaults to CMAKE_INSTALL_PREFIX/cython") - if(NOT GTSAM_CYTHON_INSTALL_PATH) - set(GTSAM_CYTHON_INSTALL_PATH "${CMAKE_INSTALL_PREFIX}/cython") - endif() + set(GTSAM_CYTHON_PATH "${PROJECT_BINARY_DIR}/cython" CACHE PATH "Cython source files directory path") # Cython install path appended with Build type (e.g. cython, cythonDebug, etc). - set(GTSAM_CYTHON_INSTALL_FULLPATH "${GTSAM_CYTHON_INSTALL_PATH}${GTSAM_BUILD_TAG}") - set(GTSAM_EIGENCY_INSTALL_PATH ${GTSAM_CYTHON_INSTALL_FULLPATH}/gtsam_eigency) + set(GTSAM_CYTHON_INSTALL_PATH "" CACHE PATH "Cython toolbox destination, blank defaults to PROJECT_BINARY_DIR/cython.build") + if(NOT GTSAM_CYTHON_INSTALL_PATH) + set(GTSAM_CYTHON_INSTALL_PATH "${PROJECT_BINARY_DIR}/cython.build${GTSAM_BUILD_TAG}") + endif() + set(GTSAM_EIGENCY_PATH ${GTSAM_CYTHON_PATH}/gtsam_eigency) + set(GTSAM_EIGENCY_INSTALL_PATH ${GTSAM_CYTHON_INSTALL_PATH}/gtsam_eigency) add_subdirectory(cython) else() set(GTSAM_INSTALL_CYTHON_TOOLBOX 0) # This will go into config.h From 06476c8ee745deca1b188730b4cf881d48a0f668 Mon Sep 17 00:00:00 2001 From: Varun Agrawal Date: Tue, 30 Jun 2020 20:46:55 -0500 Subject: [PATCH 18/29] Create and use cython build directory --- cython/CMakeLists.txt | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/cython/CMakeLists.txt b/cython/CMakeLists.txt index 5569c0e47..b0eb43c50 100644 --- a/cython/CMakeLists.txt +++ b/cython/CMakeLists.txt @@ -1,3 +1,6 @@ +# Create directory where cython build files will be placed +file(MAKE_DIRECTORY ${GTSAM_CYTHON_INSTALL_PATH}) + # Install cython components include(GtsamCythonWrap) @@ -5,7 +8,7 @@ include(GtsamCythonWrap) if (GTSAM_INSTALL_CYTHON_TOOLBOX) # build and include the eigency version of eigency add_subdirectory(gtsam_eigency) - include_directories(${PROJECT_BINARY_DIR}/cython/gtsam_eigency) + include_directories(${GTSAM_EIGENCY_PATH}) # Fix for error "C1128: number of sections exceeded object file format limit" if(MSVC) @@ -44,12 +47,13 @@ if (GTSAM_INSTALL_CYTHON_TOOLBOX) install_cython_scripts("${PROJECT_SOURCE_DIR}/cython/gtsam" "${GTSAM_CYTHON_INSTALL_PATH}" "*.py") install_cython_scripts("${PROJECT_SOURCE_DIR}/cython/gtsam_unstable" "${GTSAM_CYTHON_INSTALL_PATH}" "*.py") - # Adding custom function here so that gtsam_eigency is installed before - # the below execute_process runs. - install_gtsam_eigency(${PROJECT_BINARY_DIR}/cython/gtsam_eigency) + # Install gtsam_eigency. + # The paths are picked up directly from the parent CMakeLists.txt. + install_gtsam_eigency() # Automatically run the python installation via the setup.py install(CODE "execute_process(COMMAND ${PYTHON_EXECUTABLE} setup.py install - WORKING_DIRECTORY ${GTSAM_CYTHON_INSTALL_FULLPATH})") + WORKING_DIRECTORY ${GTSAM_CYTHON_INSTALL_PATH})") + endif () From c84060acea3321150f7fd5e652d0b8b8c5c90f53 Mon Sep 17 00:00:00 2001 From: Varun Agrawal Date: Tue, 30 Jun 2020 20:47:32 -0500 Subject: [PATCH 19/29] Use the high level cython variables, improve install process --- cython/gtsam_eigency/CMakeLists.txt | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/cython/gtsam_eigency/CMakeLists.txt b/cython/gtsam_eigency/CMakeLists.txt index 6cff534c5..f64e45cdb 100644 --- a/cython/gtsam_eigency/CMakeLists.txt +++ b/cython/gtsam_eigency/CMakeLists.txt @@ -4,7 +4,7 @@ include(GtsamCythonWrap) # so that the cython-generated header "conversions_api.h" can be found when cythonizing eigency's core # and eigency's cython pxd headers can be found when cythonizing gtsam file(COPY "." DESTINATION ".") -set(OUTPUT_DIR "${PROJECT_BINARY_DIR}/cython/gtsam_eigency") +set(OUTPUT_DIR "${GTSAM_CYTHON_PATH}/gtsam_eigency") set(EIGENCY_INCLUDE_DIR ${OUTPUT_DIR}) # This is to make the build/cython/gtsam_eigency folder a python package @@ -16,8 +16,8 @@ include_directories(${EIGENCY_INCLUDE_DIR}) # Cythonize and build eigency message(STATUS "Cythonize and build eigency") # Important trick: use "../gtsam_eigency/conversions.pyx" to let cython know that the conversions module is -# a part of the gtsam_eigency package and generate the function call import_gtsam_igency__conversions() -# in conversions_api.h correctly!!! +# a part of the gtsam_eigency package and generate the function call import_gtsam_eigency__conversions() +# in conversions_api.h correctly! cythonize(cythonize_eigency_conversions "../gtsam_eigency/conversions.pyx" "conversions" "${OUTPUT_DIR}" "${EIGENCY_INCLUDE_DIR}" "" "" "") cythonize(cythonize_eigency_core "../gtsam_eigency/core.pyx" "core" @@ -38,14 +38,16 @@ add_custom_target(cythonize_eigency) add_dependencies(cythonize_eigency cythonize_eigency_conversions cythonize_eigency_core) # install -function(install_gtsam_eigency source_directory) - install(DIRECTORY ${source_directory} - DESTINATION "${GTSAM_CYTHON_INSTALL_FULLPATH}" +function(install_gtsam_eigency) + install(DIRECTORY ${GTSAM_EIGENCY_PATH} + DESTINATION "${GTSAM_CYTHON_INSTALL_PATH}" PATTERN "CMakeLists.txt" EXCLUDE - PATTERN "__init__.py.in" EXCLUDE) + PATTERN "__init__.py.in" EXCLUDE + PATTERN "*.dir" EXCLUDE + PATTERN "*.make" EXCLUDE) install(TARGETS cythonize_eigency_core cythonize_eigency_conversions - DESTINATION "${GTSAM_CYTHON_INSTALL_FULLPATH}/gtsam_eigency") - install(FILES ${source_directory}/conversions_api.h DESTINATION ${GTSAM_CYTHON_INSTALL_FULLPATH}/gtsam_eigency) - install(FILES ${source_directory}/__init__.py DESTINATION ${GTSAM_CYTHON_INSTALL_FULLPATH}/gtsam_eigency) + DESTINATION "${GTSAM_EIGENCY_INSTALL_PATH}") + install(FILES ${GTSAM_EIGENCY_PATH}/conversions_api.h DESTINATION ${GTSAM_EIGENCY_INSTALL_PATH}) + install(FILES ${GTSAM_EIGENCY_PATH}/__init__.py DESTINATION ${GTSAM_EIGENCY_INSTALL_PATH}) endfunction() From 7a725bf46af9fc069f4db772e4f191956a783e89 Mon Sep 17 00:00:00 2001 From: Varun Agrawal Date: Tue, 30 Jun 2020 20:48:01 -0500 Subject: [PATCH 20/29] Remove redundant postfix checking since the postfix is already added at the top level --- cmake/GtsamCythonWrap.cmake | 75 ++++++------------------------------- 1 file changed, 12 insertions(+), 63 deletions(-) diff --git a/cmake/GtsamCythonWrap.cmake b/cmake/GtsamCythonWrap.cmake index 851f53cfe..3ce7f4454 100644 --- a/cmake/GtsamCythonWrap.cmake +++ b/cmake/GtsamCythonWrap.cmake @@ -184,35 +184,16 @@ function(install_cython_wrapped_library interface_header generated_files_path in # Split up filename to strip trailing '/' in GTSAM_CYTHON_INSTALL_PATH/subdirectory if there is one get_filename_component(location "${install_path}" PATH) get_filename_component(name "${install_path}" NAME) - message(STATUS "Installing Cython Toolbox to ${location}${GTSAM_BUILD_TAG}/${name}") #${GTSAM_CYTHON_INSTALL_FULLPATH}" + message(STATUS "Installing Cython Toolbox to ${location}/${name}") #${GTSAM_CYTHON_INSTALL_PATH}" - if(GTSAM_BUILD_TYPE_POSTFIXES) - foreach(build_type ${CMAKE_CONFIGURATION_TYPES}) - string(TOUPPER "${build_type}" build_type_upper) - if(${build_type_upper} STREQUAL "RELEASE") - set(build_type_tag "") # Don't create release mode tag on installed directory - else() - set(build_type_tag "${build_type}") - endif() - - install(DIRECTORY "${generated_files_path}/" DESTINATION "${location}${build_type_tag}/${name}" - CONFIGURATIONS "${build_type}" - PATTERN "build" EXCLUDE - PATTERN "CMakeFiles" EXCLUDE - PATTERN "Makefile" EXCLUDE - PATTERN "*.cmake" EXCLUDE - PATTERN "*.cpp" EXCLUDE - PATTERN "*.py" EXCLUDE) - endforeach() - else() - install(DIRECTORY "${generated_files_path}/" DESTINATION ${install_path} - PATTERN "build" EXCLUDE - PATTERN "CMakeFiles" EXCLUDE - PATTERN "Makefile" EXCLUDE - PATTERN "*.cmake" EXCLUDE - PATTERN "*.cpp" EXCLUDE - PATTERN "*.py" EXCLUDE) - endif() + install(DIRECTORY "${generated_files_path}/" DESTINATION ${install_path} + CONFIGURATIONS "${CMAKE_BUILD_TYPE}" + PATTERN "build" EXCLUDE + PATTERN "CMakeFiles" EXCLUDE + PATTERN "Makefile" EXCLUDE + PATTERN "*.cmake" EXCLUDE + PATTERN "*.cpp" EXCLUDE + PATTERN "*.py" EXCLUDE) endfunction() # Helper function to install Cython scripts and handle multiple build types where the scripts @@ -232,24 +213,9 @@ function(install_cython_scripts source_directory dest_directory patterns) foreach(pattern ${patterns}) list(APPEND patterns_args PATTERN "${pattern}") endforeach() - if(GTSAM_BUILD_TYPE_POSTFIXES) - foreach(build_type ${CMAKE_CONFIGURATION_TYPES}) - string(TOUPPER "${build_type}" build_type_upper) - if(${build_type_upper} STREQUAL "RELEASE") - set(build_type_tag "") # Don't create release mode tag on installed directory - else() - set(build_type_tag "${build_type}") - endif() - # Split up filename to strip trailing '/' in GTSAM_CYTHON_INSTALL_PATH if there is one - get_filename_component(location "${dest_directory}" PATH) - get_filename_component(name "${dest_directory}" NAME) - install(DIRECTORY "${source_directory}" DESTINATION "${location}/${name}${build_type_tag}" CONFIGURATIONS "${build_type}" - FILES_MATCHING ${patterns_args} PATTERN "${exclude_patterns}" EXCLUDE) - endforeach() - else() - install(DIRECTORY "${source_directory}" DESTINATION "${dest_directory}" FILES_MATCHING ${patterns_args} PATTERN "${exclude_patterns}" EXCLUDE) - endif() + install(DIRECTORY "${source_directory}" DESTINATION "${dest_directory}" CONFIGURATIONS "${CMAKE_BUILD_TYPE}" + FILES_MATCHING ${patterns_args} PATTERN "${exclude_patterns}" EXCLUDE) endfunction() # Helper function to install specific files and handle multiple build types where the scripts @@ -259,22 +225,5 @@ endfunction() # source_files: The source files to be installed. # dest_directory: The destination directory to install to. function(install_cython_files source_files dest_directory) - - if(GTSAM_BUILD_TYPE_POSTFIXES) - foreach(build_type ${CMAKE_CONFIGURATION_TYPES}) - string(TOUPPER "${build_type}" build_type_upper) - if(${build_type_upper} STREQUAL "RELEASE") - set(build_type_tag "") # Don't create release mode tag on installed directory - else() - set(build_type_tag "${build_type}") - endif() - # Split up filename to strip trailing '/' in GTSAM_CYTHON_INSTALL_PATH if there is one - get_filename_component(location "${dest_directory}" PATH) - get_filename_component(name "${dest_directory}" NAME) - install(FILES "${source_files}" DESTINATION "${location}/${name}${build_type_tag}" CONFIGURATIONS "${build_type}") - endforeach() - else() - install(FILES "${source_files}" DESTINATION "${dest_directory}") - endif() - + install(FILES "${source_files}" DESTINATION "${dest_directory}" CONFIGURATIONS "${CMAKE_BUILD_TYPE}") endfunction() From 54f2acd521b0531c9a727ee585cae7afcf5ef2bd Mon Sep 17 00:00:00 2001 From: Varun Agrawal Date: Tue, 30 Jun 2020 20:57:31 -0500 Subject: [PATCH 21/29] updated cython wrapper README --- cython/README.md | 89 +++++++++++++++++++++--------------------------- 1 file changed, 38 insertions(+), 51 deletions(-) diff --git a/cython/README.md b/cython/README.md index bc6e346d9..0c59915a0 100644 --- a/cython/README.md +++ b/cython/README.md @@ -1,33 +1,36 @@ # Python Wrapper -This is the Cython/Python wrapper around the GTSAM C++ library. +This is the Python wrapper around the GTSAM C++ library. We use Cython to generate the bindings to the underlying C++ code. + +## Requirements + +- If you want to build the gtsam python library for a specific python version (eg 2.7), use the `-DGTSAM_PYTHON_VERSION=2.7` option when running `cmake` otherwise the default interpreter will be used. + - If the interpreter is inside an environment (such as an anaconda environment or virtualenv environment) then the environment should be active while building gtsam. +- This wrapper needs `Cython(>=0.25.2)`, `backports_abc(>=0.5)`, and `numpy(>=1.11.0)`. These can be installed as follows: + + ```bash + pip install -r /cython/requirements.txt + ``` + +- For compatibility with gtsam's Eigen version, it contains its own cloned version of [Eigency](https://github.com/wouterboomsma/eigency.git), +named `gtsam_eigency`, to interface between C++'s Eigen and Python's numpy. ## Install -- if you want to build the gtsam python library for a specific python version (eg 2.7), use the `-DGTSAM_PYTHON_VERSION=2.7` option when running `cmake` otherwise the default interpreter will be used. - - If the interpreter is inside an environment (such as an anaconda environment or virtualenv environment) then the environment should be active while building gtsam. -- This wrapper needs Cython(>=0.25.2), backports_abc>=0.5, and numpy. These can be installed as follows: - -```bash - pip install -r /cython/requirements.txt -``` - -- For compatibility with gtsam's Eigen version, it contains its own cloned version of [Eigency](https://github.com/wouterboomsma/eigency.git), -named **gtsam_eigency**, to interface between C++'s Eigen and Python's numpy. - -- Build and install gtsam using cmake with `GTSAM_INSTALL_CYTHON_TOOLBOX` enabled. -The wrapped module will be installed to `GTSAM_CYTHON_INSTALL_PATH`, which is -by default: `/cython` +- Run cmake with the `GTSAM_INSTALL_CYTHON_TOOLBOX` cmake flag enabled to configure building the wrapper. The wrapped module will be built and copied to the directory defined by `GTSAM_CYTHON_INSTALL_PATH`, which is by default: `/cython.build`. - To use the library without installing system-wide: modify your `PYTHONPATH` to include the `GTSAM_CYTHON_INSTALL_PATH`: -```bash -export PYTHONPATH=$PYTHONPATH: -``` -- To install system-wide: run `make install` then navigate to `GTSAM_CYTHON_INSTALL_PATH` and run `python setup.py install` - - (the same command can be used to install into a virtual environment if it is active) - - note: if you don't want gtsam to install to a system directory such as `/usr/local`, pass `-DCMAKE_INSTALL_PREFIX="./install"` to cmake to install gtsam to a subdirectory of the build directory. - - if you run `setup.py` from the build directory rather than the installation directory, the script will warn you with the message: `setup.py is being run from an unexpected location`. - Before `make install` is run, not all the components of the package have been copied across, so running `setup.py` from the build directory would result in an incomplete package. + + ```bash + export PYTHONPATH=$PYTHONPATH: + ``` + +- Build GTSAM and the wrapper with `make`. + +- To install system-wide, simply run `make install`. + - The same command can be used to install into a virtual environment if it is active. + - **NOTE**: if you don't want gtsam to install to a system directory such as `/usr/local`, pass `-DCMAKE_INSTALL_PREFIX="./install"` to cmake to install gtsam to a subdirectory of the build directory. + - If you run `setup.py` from the build directory rather than the installation directory, the script will warn you with the message: `setup.py is being run from an unexpected location`. ## Unit Tests @@ -47,48 +50,32 @@ See the tests for examples. - Vector/Matrix: + GTSAM expects double-precision floating point vectors and matrices. - Hence, you should pass numpy matrices with dtype=float, or 'float64'. + Hence, you should pass numpy matrices with `dtype=float`, or `float64`. + Also, GTSAM expects *column-major* matrices, unlike the default storage scheme in numpy. Hence, you should pass column-major matrices to gtsam using the flag order='F'. And you always get column-major matrices back. - For more details, see: https://github.com/wouterboomsma/eigency#storage-layout---why-arrays-are-sometimes-transposed - + Passing row-major matrices of different dtype, e.g. 'int', will also work + For more details, see [this link](https://github.com/wouterboomsma/eigency#storage-layout---why-arrays-are-sometimes-transposed). + + Passing row-major matrices of different dtype, e.g. `int`, will also work as the wrapper converts them to column-major and dtype float for you, using numpy.array.astype(float, order='F', copy=False). However, this will result a copy if your matrix is not in the expected type and storage order. - Inner namespace: Classes in inner namespace will be prefixed by _ in Python. -Examples: noiseModel_Gaussian, noiseModel_mEstimator_Tukey + + Examples: `noiseModel_Gaussian`, `noiseModel_mEstimator_Tukey` - Casting from a base class to a derive class must be done explicitly. -Examples: -```Python - noiseBase = factor.noiseModel() - noiseGaussian = dynamic_cast_noiseModel_Gaussian_noiseModel_Base(noiseBase) -``` -## Wrapping Your Own Project That Uses GTSAM + Examples: + ```python + noiseBase = factor.noiseModel() + noiseGaussian = dynamic_cast_noiseModel_Gaussian_noiseModel_Base(noiseBase) + ``` -- Set PYTHONPATH to include ${GTSAM_CYTHON_INSTALL_PATH} - + so that it can find gtsam Cython header: gtsam/gtsam.pxd +## Wrapping Custom GTSAM-based Project -- In your CMakeList.txt -```cmake -find_package(GTSAM REQUIRED) # Make sure gtsam's install folder is in your PATH -set(CMAKE_MODULE_PATH "${CMAKE_MODULE_PATH}" "${GTSAM_DIR}/../GTSAMCMakeTools") - -# Wrap -include(GtsamCythonWrap) -include_directories(${GTSAM_EIGENCY_INSTALL_PATH}) -wrap_and_install_library_cython("your_project_interface.h" - "from gtsam.gtsam cimport *" # extra import of gtsam/gtsam.pxd Cython header - "your_install_path" - "libraries_to_link_with_the_cython_module" - "dependencies_which_need_to_be_built_before_the_wrapper" - ) -#Optional: install_cython_scripts and install_cython_files. See GtsamCythonWrap.cmake. -``` +Please refer to the template project and the corresponding tutorial available [here](https://github.com/borglab/gtsam-project-python). ## KNOWN ISSUES From 8859b963a24db117fc51ea9532c310b6721c0fc1 Mon Sep 17 00:00:00 2001 From: Varun Agrawal Date: Wed, 1 Jul 2020 12:13:53 -0500 Subject: [PATCH 22/29] In-place cython build Build everything inside the build/cython{BuildType} directory directly, so we can bypass the `make install` step and introduce the `make python-install` step which allows cmake to handle all dependencies. --- CMakeLists.txt | 8 ++--- cmake/GtsamCythonWrap.cmake | 6 ++-- cython/CMakeLists.txt | 51 ++++++++++++++--------------- cython/gtsam_eigency/CMakeLists.txt | 17 +--------- 4 files changed, 31 insertions(+), 51 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index f8deebfcd..a9966f5d3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -454,15 +454,13 @@ endif() if (GTSAM_INSTALL_CYTHON_TOOLBOX) set(GTSAM_INSTALL_CYTHON_TOOLBOX 1) # Set up cache options - set(GTSAM_CYTHON_PATH "${PROJECT_BINARY_DIR}/cython" CACHE PATH "Cython source files directory path") # Cython install path appended with Build type (e.g. cython, cythonDebug, etc). - set(GTSAM_CYTHON_INSTALL_PATH "" CACHE PATH "Cython toolbox destination, blank defaults to PROJECT_BINARY_DIR/cython.build") + set(GTSAM_CYTHON_INSTALL_PATH "" CACHE PATH "Cython toolbox destination, blank defaults to PROJECT_BINARY_DIR/cython") if(NOT GTSAM_CYTHON_INSTALL_PATH) - set(GTSAM_CYTHON_INSTALL_PATH "${PROJECT_BINARY_DIR}/cython.build${GTSAM_BUILD_TAG}") + set(GTSAM_CYTHON_INSTALL_PATH "${PROJECT_BINARY_DIR}/cython${GTSAM_BUILD_TAG}" CACHE PATH "") endif() - set(GTSAM_EIGENCY_PATH ${GTSAM_CYTHON_PATH}/gtsam_eigency) set(GTSAM_EIGENCY_INSTALL_PATH ${GTSAM_CYTHON_INSTALL_PATH}/gtsam_eigency) - add_subdirectory(cython) + add_subdirectory(cython ${GTSAM_CYTHON_INSTALL_PATH}) else() set(GTSAM_INSTALL_CYTHON_TOOLBOX 0) # This will go into config.h endif() diff --git a/cmake/GtsamCythonWrap.cmake b/cmake/GtsamCythonWrap.cmake index 3ce7f4454..797745acf 100644 --- a/cmake/GtsamCythonWrap.cmake +++ b/cmake/GtsamCythonWrap.cmake @@ -41,9 +41,9 @@ execute_process(COMMAND "${PYTHON_EXECUTABLE}" "-c" function(wrap_and_install_library_cython interface_header extra_imports install_path libs dependencies) # Paths for generated files get_filename_component(module_name "${interface_header}" NAME_WE) - set(generated_files_path "${PROJECT_BINARY_DIR}/cython/${module_name}") + set(generated_files_path "${GTSAM_CYTHON_INSTALL_PATH}/${module_name}") wrap_library_cython("${interface_header}" "${generated_files_path}" "${extra_imports}" "${libs}" "${dependencies}") - install_cython_wrapped_library("${interface_header}" "${generated_files_path}" "${install_path}") + # install_cython_wrapped_library("${interface_header}" "${generated_files_path}" "${install_path}") endfunction() function(set_up_required_cython_packages) @@ -214,7 +214,7 @@ function(install_cython_scripts source_directory dest_directory patterns) list(APPEND patterns_args PATTERN "${pattern}") endforeach() - install(DIRECTORY "${source_directory}" DESTINATION "${dest_directory}" CONFIGURATIONS "${CMAKE_BUILD_TYPE}" + file(COPY "${source_directory}" DESTINATION "${dest_directory}" FILES_MATCHING ${patterns_args} PATTERN "${exclude_patterns}" EXCLUDE) endfunction() diff --git a/cython/CMakeLists.txt b/cython/CMakeLists.txt index b0eb43c50..74725c463 100644 --- a/cython/CMakeLists.txt +++ b/cython/CMakeLists.txt @@ -1,21 +1,37 @@ -# Create directory where cython build files will be placed -file(MAKE_DIRECTORY ${GTSAM_CYTHON_INSTALL_PATH}) - # Install cython components include(GtsamCythonWrap) # Create the cython toolbox for the gtsam library if (GTSAM_INSTALL_CYTHON_TOOLBOX) + # Add the new make target command + add_custom_target(python-install + COMMAND ${PYTHON_EXECUTABLE} ${GTSAM_CYTHON_INSTALL_PATH}/setup.py install + WORKING_DIRECTORY ${GTSAM_CYTHON_INSTALL_FULLPATH}) + # build and include the eigency version of eigency add_subdirectory(gtsam_eigency) - include_directories(${GTSAM_EIGENCY_PATH}) + include_directories(${GTSAM_EIGENCY_INSTALL_PATH}) # Fix for error "C1128: number of sections exceeded object file format limit" if(MSVC) add_compile_options(/bigobj) endif() - # wrap gtsam + # First set up all the package related files. + # This also ensures the below wrap operations work correctly. + set(CYTHON_INSTALL_REQUIREMENTS_FILE "${PROJECT_SOURCE_DIR}/cython/requirements.txt") + + # Install the custom-generated __init__.py + # This makes the cython (sub-)directories into python packages, so gtsam can be found while wrapping gtsam_unstable + configure_file(${PROJECT_SOURCE_DIR}/cython/gtsam/__init__.py ${GTSAM_CYTHON_INSTALL_PATH}/gtsam/__init__.py COPYONLY) + configure_file(${PROJECT_SOURCE_DIR}/cython/gtsam_unstable/__init__.py ${GTSAM_CYTHON_INSTALL_PATH}/gtsam_unstable/__init__.py COPYONLY) + configure_file(${PROJECT_SOURCE_DIR}/cython/setup.py.in ${GTSAM_CYTHON_INSTALL_PATH}/setup.py) + install_cython_files("${PROJECT_BINARY_DIR}/cython/setup.py" "${GTSAM_CYTHON_INSTALL_PATH}") + # install scripts and tests + install_cython_scripts("${PROJECT_SOURCE_DIR}/cython/gtsam" "${GTSAM_CYTHON_INSTALL_PATH}" "*.py") + install_cython_scripts("${PROJECT_SOURCE_DIR}/cython/gtsam_unstable" "${GTSAM_CYTHON_INSTALL_PATH}" "*.py") + + # Wrap gtsam add_custom_target(gtsam_header DEPENDS "../gtsam.h") wrap_and_install_library_cython("../gtsam.h" # interface_header "" # extra imports @@ -23,8 +39,9 @@ if (GTSAM_INSTALL_CYTHON_TOOLBOX) gtsam # library to link with "wrap;cythonize_eigency;gtsam;gtsam_header" # dependencies which need to be built before wrapping ) + add_dependencies(python-install gtsam gtsam_header) - # wrap gtsam_unstable + # Wrap gtsam_unstable if(GTSAM_BUILD_UNSTABLE) add_custom_target(gtsam_unstable_header DEPENDS "../gtsam_unstable/gtsam_unstable.h") wrap_and_install_library_cython("../gtsam_unstable/gtsam_unstable.h" # interface_header @@ -33,27 +50,7 @@ if (GTSAM_INSTALL_CYTHON_TOOLBOX) gtsam_unstable # library to link with "gtsam_unstable;gtsam_unstable_header;cythonize_gtsam" # dependencies to be built before wrapping ) + add_dependencies(python-install gtsam_unstable gtsam_unstable_header) endif() - set(CYTHON_INSTALL_REQUIREMENTS_FILE "${PROJECT_SOURCE_DIR}/cython/requirements.txt") - - # Install the custom-generated __init__.py - # 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 ${PROJECT_BINARY_DIR}/cython/gtsam/__init__.py COPYONLY) - configure_file(${PROJECT_SOURCE_DIR}/cython/gtsam_unstable/__init__.py ${PROJECT_BINARY_DIR}/cython/gtsam_unstable/__init__.py COPYONLY) - configure_file(${PROJECT_SOURCE_DIR}/cython/setup.py.in ${PROJECT_BINARY_DIR}/cython/setup.py) - install_cython_files("${PROJECT_BINARY_DIR}/cython/setup.py" "${GTSAM_CYTHON_INSTALL_PATH}") - # install scripts and tests - install_cython_scripts("${PROJECT_SOURCE_DIR}/cython/gtsam" "${GTSAM_CYTHON_INSTALL_PATH}" "*.py") - install_cython_scripts("${PROJECT_SOURCE_DIR}/cython/gtsam_unstable" "${GTSAM_CYTHON_INSTALL_PATH}" "*.py") - - # Install gtsam_eigency. - # The paths are picked up directly from the parent CMakeLists.txt. - install_gtsam_eigency() - - # Automatically run the python installation via the setup.py - install(CODE - "execute_process(COMMAND ${PYTHON_EXECUTABLE} setup.py install - WORKING_DIRECTORY ${GTSAM_CYTHON_INSTALL_PATH})") - endif () diff --git a/cython/gtsam_eigency/CMakeLists.txt b/cython/gtsam_eigency/CMakeLists.txt index f64e45cdb..7c215e89c 100644 --- a/cython/gtsam_eigency/CMakeLists.txt +++ b/cython/gtsam_eigency/CMakeLists.txt @@ -4,7 +4,7 @@ include(GtsamCythonWrap) # so that the cython-generated header "conversions_api.h" can be found when cythonizing eigency's core # and eigency's cython pxd headers can be found when cythonizing gtsam file(COPY "." DESTINATION ".") -set(OUTPUT_DIR "${GTSAM_CYTHON_PATH}/gtsam_eigency") +set(OUTPUT_DIR "${GTSAM_CYTHON_INSTALL_PATH}/gtsam_eigency") set(EIGENCY_INCLUDE_DIR ${OUTPUT_DIR}) # This is to make the build/cython/gtsam_eigency folder a python package @@ -36,18 +36,3 @@ target_include_directories(cythonize_eigency_core PUBLIC add_dependencies(cythonize_eigency_core cythonize_eigency_conversions) add_custom_target(cythonize_eigency) add_dependencies(cythonize_eigency cythonize_eigency_conversions cythonize_eigency_core) - -# install -function(install_gtsam_eigency) - install(DIRECTORY ${GTSAM_EIGENCY_PATH} - DESTINATION "${GTSAM_CYTHON_INSTALL_PATH}" - PATTERN "CMakeLists.txt" EXCLUDE - PATTERN "__init__.py.in" EXCLUDE - PATTERN "*.dir" EXCLUDE - PATTERN "*.make" EXCLUDE) - install(TARGETS cythonize_eigency_core cythonize_eigency_conversions - DESTINATION "${GTSAM_EIGENCY_INSTALL_PATH}") - install(FILES ${GTSAM_EIGENCY_PATH}/conversions_api.h DESTINATION ${GTSAM_EIGENCY_INSTALL_PATH}) - install(FILES ${GTSAM_EIGENCY_PATH}/__init__.py DESTINATION ${GTSAM_EIGENCY_INSTALL_PATH}) - -endfunction() From 74591eece60b45342a865474fef452a7683430c7 Mon Sep 17 00:00:00 2001 From: Varun Agrawal Date: Wed, 1 Jul 2020 14:36:16 -0500 Subject: [PATCH 23/29] fixed CYTHON_INSTALL_PATH cmake variable wrt cache --- CMakeLists.txt | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index a9966f5d3..f5b9c5e22 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -455,10 +455,8 @@ if (GTSAM_INSTALL_CYTHON_TOOLBOX) set(GTSAM_INSTALL_CYTHON_TOOLBOX 1) # Set up cache options # Cython install path appended with Build type (e.g. cython, cythonDebug, etc). - set(GTSAM_CYTHON_INSTALL_PATH "" CACHE PATH "Cython toolbox destination, blank defaults to PROJECT_BINARY_DIR/cython") - if(NOT GTSAM_CYTHON_INSTALL_PATH) - set(GTSAM_CYTHON_INSTALL_PATH "${PROJECT_BINARY_DIR}/cython${GTSAM_BUILD_TAG}" CACHE PATH "") - endif() + # This does not override custom values set from the command line + set(GTSAM_CYTHON_INSTALL_PATH "${PROJECT_BINARY_DIR}/cython${GTSAM_BUILD_TAG}" CACHE PATH "Cython toolbox destination, blank defaults to PROJECT_BINARY_DIR/cython") set(GTSAM_EIGENCY_INSTALL_PATH ${GTSAM_CYTHON_INSTALL_PATH}/gtsam_eigency) add_subdirectory(cython ${GTSAM_CYTHON_INSTALL_PATH}) else() From 59968fddc5d7e0ded0102a8ac310602b9dd5c4b9 Mon Sep 17 00:00:00 2001 From: Varun Agrawal Date: Wed, 1 Jul 2020 14:36:57 -0500 Subject: [PATCH 24/29] Python Wrapper CMake update - Added python-install target variable for easy updating. - Fixed/Added all dependencies so that everything is built automatically. - Removed unnecessary install commands --- cmake/GtsamCythonWrap.cmake | 26 +++----------------------- cython/CMakeLists.txt | 14 ++++++++------ cython/gtsam_eigency/CMakeLists.txt | 2 ++ 3 files changed, 13 insertions(+), 29 deletions(-) diff --git a/cmake/GtsamCythonWrap.cmake b/cmake/GtsamCythonWrap.cmake index 797745acf..7597834c9 100644 --- a/cmake/GtsamCythonWrap.cmake +++ b/cmake/GtsamCythonWrap.cmake @@ -43,7 +43,6 @@ function(wrap_and_install_library_cython interface_header extra_imports install_ get_filename_component(module_name "${interface_header}" NAME_WE) set(generated_files_path "${GTSAM_CYTHON_INSTALL_PATH}/${module_name}") wrap_library_cython("${interface_header}" "${generated_files_path}" "${extra_imports}" "${libs}" "${dependencies}") - # install_cython_wrapped_library("${interface_header}" "${generated_files_path}" "${install_path}") endfunction() function(set_up_required_cython_packages) @@ -170,32 +169,13 @@ function(wrap_library_cython interface_header generated_files_path extra_imports cythonize(cythonize_${module_name} ${generated_pyx} ${module_name} ${generated_files_path} "${include_dirs}" "${libs}" ${interface_header} cython_wrap_${module_name}_pyx) + add_dependencies(${python_install_target} cython_wrap_${module_name}_pyx) + # distclean add_custom_target(wrap_${module_name}_cython_distclean COMMAND cmake -E remove_directory ${generated_files_path}) endfunction() -# Internal function that installs a wrap toolbox -function(install_cython_wrapped_library interface_header generated_files_path install_path) - get_filename_component(module_name "${interface_header}" NAME_WE) - - # NOTE: only installs .pxd and .pyx and binary files (not .cpp) - the trailing slash on the directory name - # here prevents creating the top-level module name directory in the destination. - # Split up filename to strip trailing '/' in GTSAM_CYTHON_INSTALL_PATH/subdirectory if there is one - get_filename_component(location "${install_path}" PATH) - get_filename_component(name "${install_path}" NAME) - message(STATUS "Installing Cython Toolbox to ${location}/${name}") #${GTSAM_CYTHON_INSTALL_PATH}" - - install(DIRECTORY "${generated_files_path}/" DESTINATION ${install_path} - CONFIGURATIONS "${CMAKE_BUILD_TYPE}" - PATTERN "build" EXCLUDE - PATTERN "CMakeFiles" EXCLUDE - PATTERN "Makefile" EXCLUDE - PATTERN "*.cmake" EXCLUDE - PATTERN "*.cpp" EXCLUDE - PATTERN "*.py" EXCLUDE) -endfunction() - # Helper function to install Cython scripts and handle multiple build types where the scripts # should be installed to all build type toolboxes # @@ -225,5 +205,5 @@ endfunction() # source_files: The source files to be installed. # dest_directory: The destination directory to install to. function(install_cython_files source_files dest_directory) - install(FILES "${source_files}" DESTINATION "${dest_directory}" CONFIGURATIONS "${CMAKE_BUILD_TYPE}") + file(COPY "${source_files}" DESTINATION "${dest_directory}") endfunction() diff --git a/cython/CMakeLists.txt b/cython/CMakeLists.txt index 74725c463..ce93120c2 100644 --- a/cython/CMakeLists.txt +++ b/cython/CMakeLists.txt @@ -4,7 +4,8 @@ include(GtsamCythonWrap) # Create the cython toolbox for the gtsam library if (GTSAM_INSTALL_CYTHON_TOOLBOX) # Add the new make target command - add_custom_target(python-install + set(python_install_target python-install) + add_custom_target(${python_install_target} COMMAND ${PYTHON_EXECUTABLE} ${GTSAM_CYTHON_INSTALL_PATH}/setup.py install WORKING_DIRECTORY ${GTSAM_CYTHON_INSTALL_FULLPATH}) @@ -27,9 +28,6 @@ if (GTSAM_INSTALL_CYTHON_TOOLBOX) configure_file(${PROJECT_SOURCE_DIR}/cython/gtsam_unstable/__init__.py ${GTSAM_CYTHON_INSTALL_PATH}/gtsam_unstable/__init__.py COPYONLY) configure_file(${PROJECT_SOURCE_DIR}/cython/setup.py.in ${GTSAM_CYTHON_INSTALL_PATH}/setup.py) install_cython_files("${PROJECT_BINARY_DIR}/cython/setup.py" "${GTSAM_CYTHON_INSTALL_PATH}") - # install scripts and tests - install_cython_scripts("${PROJECT_SOURCE_DIR}/cython/gtsam" "${GTSAM_CYTHON_INSTALL_PATH}" "*.py") - install_cython_scripts("${PROJECT_SOURCE_DIR}/cython/gtsam_unstable" "${GTSAM_CYTHON_INSTALL_PATH}" "*.py") # Wrap gtsam add_custom_target(gtsam_header DEPENDS "../gtsam.h") @@ -39,7 +37,7 @@ if (GTSAM_INSTALL_CYTHON_TOOLBOX) gtsam # library to link with "wrap;cythonize_eigency;gtsam;gtsam_header" # dependencies which need to be built before wrapping ) - add_dependencies(python-install gtsam gtsam_header) + add_dependencies(${python_install_target} gtsam gtsam_header) # Wrap gtsam_unstable if(GTSAM_BUILD_UNSTABLE) @@ -50,7 +48,11 @@ if (GTSAM_INSTALL_CYTHON_TOOLBOX) gtsam_unstable # library to link with "gtsam_unstable;gtsam_unstable_header;cythonize_gtsam" # dependencies to be built before wrapping ) - add_dependencies(python-install gtsam_unstable gtsam_unstable_header) + add_dependencies(${python_install_target} gtsam_unstable gtsam_unstable_header) endif() + # install scripts and tests + install_cython_scripts("${PROJECT_SOURCE_DIR}/cython/gtsam" "${GTSAM_CYTHON_INSTALL_PATH}" "*.py") + install_cython_scripts("${PROJECT_SOURCE_DIR}/cython/gtsam_unstable" "${GTSAM_CYTHON_INSTALL_PATH}" "*.py") + endif () diff --git a/cython/gtsam_eigency/CMakeLists.txt b/cython/gtsam_eigency/CMakeLists.txt index 7c215e89c..663ea0a32 100644 --- a/cython/gtsam_eigency/CMakeLists.txt +++ b/cython/gtsam_eigency/CMakeLists.txt @@ -36,3 +36,5 @@ target_include_directories(cythonize_eigency_core PUBLIC add_dependencies(cythonize_eigency_core cythonize_eigency_conversions) add_custom_target(cythonize_eigency) add_dependencies(cythonize_eigency cythonize_eigency_conversions cythonize_eigency_core) + +add_dependencies(${python_install_target} cythonize_eigency) From a6908cd1cb003ee184d0018a559075b319d65afe Mon Sep 17 00:00:00 2001 From: Varun Agrawal Date: Wed, 1 Jul 2020 16:23:24 -0500 Subject: [PATCH 25/29] removed unneeded install commands and updated README --- cmake/GtsamCythonWrap.cmake | 10 ----- cython/CMakeLists.txt | 1 - cython/README.md | 83 ++++++++++++++++++++----------------- 3 files changed, 44 insertions(+), 50 deletions(-) diff --git a/cmake/GtsamCythonWrap.cmake b/cmake/GtsamCythonWrap.cmake index 7597834c9..c155cbbd8 100644 --- a/cmake/GtsamCythonWrap.cmake +++ b/cmake/GtsamCythonWrap.cmake @@ -197,13 +197,3 @@ function(install_cython_scripts source_directory dest_directory patterns) file(COPY "${source_directory}" DESTINATION "${dest_directory}" FILES_MATCHING ${patterns_args} PATTERN "${exclude_patterns}" EXCLUDE) endfunction() - -# Helper function to install specific files and handle multiple build types where the scripts -# should be installed to all build type toolboxes -# -# Arguments: -# source_files: The source files to be installed. -# dest_directory: The destination directory to install to. -function(install_cython_files source_files dest_directory) - file(COPY "${source_files}" DESTINATION "${dest_directory}") -endfunction() diff --git a/cython/CMakeLists.txt b/cython/CMakeLists.txt index ce93120c2..65a9e9c62 100644 --- a/cython/CMakeLists.txt +++ b/cython/CMakeLists.txt @@ -27,7 +27,6 @@ if (GTSAM_INSTALL_CYTHON_TOOLBOX) configure_file(${PROJECT_SOURCE_DIR}/cython/gtsam/__init__.py ${GTSAM_CYTHON_INSTALL_PATH}/gtsam/__init__.py COPYONLY) configure_file(${PROJECT_SOURCE_DIR}/cython/gtsam_unstable/__init__.py ${GTSAM_CYTHON_INSTALL_PATH}/gtsam_unstable/__init__.py COPYONLY) configure_file(${PROJECT_SOURCE_DIR}/cython/setup.py.in ${GTSAM_CYTHON_INSTALL_PATH}/setup.py) - install_cython_files("${PROJECT_BINARY_DIR}/cython/setup.py" "${GTSAM_CYTHON_INSTALL_PATH}") # Wrap gtsam add_custom_target(gtsam_header DEPENDS "../gtsam.h") diff --git a/cython/README.md b/cython/README.md index 0c59915a0..f69b7a5a6 100644 --- a/cython/README.md +++ b/cython/README.md @@ -4,43 +4,48 @@ This is the Python wrapper around the GTSAM C++ library. We use Cython to genera ## Requirements -- If you want to build the gtsam python library for a specific python version (eg 2.7), use the `-DGTSAM_PYTHON_VERSION=2.7` option when running `cmake` otherwise the default interpreter will be used. - - If the interpreter is inside an environment (such as an anaconda environment or virtualenv environment) then the environment should be active while building gtsam. +- If you want to build the GTSAM python library for a specific python version (eg 3.6), + use the `-DGTSAM_PYTHON_VERSION=3.6` option when running `cmake` otherwise the default interpreter will be used. +- If the interpreter is inside an environment (such as an anaconda environment or virtualenv environment), + then the environment should be active while building GTSAM. - This wrapper needs `Cython(>=0.25.2)`, `backports_abc(>=0.5)`, and `numpy(>=1.11.0)`. These can be installed as follows: ```bash pip install -r /cython/requirements.txt ``` -- For compatibility with gtsam's Eigen version, it contains its own cloned version of [Eigency](https://github.com/wouterboomsma/eigency.git), -named `gtsam_eigency`, to interface between C++'s Eigen and Python's numpy. +- For compatibility with GTSAM's Eigen version, it contains its own cloned version of [Eigency](https://github.com/wouterboomsma/eigency.git), + named `gtsam_eigency`, to interface between C++'s Eigen and Python's numpy. ## Install -- Run cmake with the `GTSAM_INSTALL_CYTHON_TOOLBOX` cmake flag enabled to configure building the wrapper. The wrapped module will be built and copied to the directory defined by `GTSAM_CYTHON_INSTALL_PATH`, which is by default: `/cython.build`. - -- To use the library without installing system-wide: modify your `PYTHONPATH` to include the `GTSAM_CYTHON_INSTALL_PATH`: - - ```bash - export PYTHONPATH=$PYTHONPATH: - ``` +- Run cmake with the `GTSAM_INSTALL_CYTHON_TOOLBOX` cmake flag enabled to configure building the wrapper. The wrapped module will be built and copied to the directory defined by `GTSAM_CYTHON_INSTALL_PATH`, which is by default `/cython` in Release mode and `/cython` for other modes. - Build GTSAM and the wrapper with `make`. -- To install system-wide, simply run `make install`. - - The same command can be used to install into a virtual environment if it is active. - - **NOTE**: if you don't want gtsam to install to a system directory such as `/usr/local`, pass `-DCMAKE_INSTALL_PREFIX="./install"` to cmake to install gtsam to a subdirectory of the build directory. - - If you run `setup.py` from the build directory rather than the installation directory, the script will warn you with the message: `setup.py is being run from an unexpected location`. +- To install, simply run `make python-install`. + - The same command can be used to install into a virtual environment if it is active. + - **NOTE**: if you don't want GTSAM to install to a system directory such as `/usr/local`, pass `-DCMAKE_INSTALL_PREFIX="./install"` to cmake to install GTSAM to a subdirectory of the build directory. + +- You can also directly run `make python-install` without running `make`, and it will compile all the dependencies accordingly. ## Unit Tests The Cython toolbox also has a small set of unit tests located in the test directory. To run them: -```bash - cd - python -m unittest discover -``` + ```bash + cd + python -m unittest discover + ``` + +## Utils + +TODO + +## Examples + +TODO ## Writing Your Own Scripts @@ -49,25 +54,27 @@ See the tests for examples. ### Some Important Notes: - Vector/Matrix: - + GTSAM expects double-precision floating point vectors and matrices. + + - GTSAM expects double-precision floating point vectors and matrices. Hence, you should pass numpy matrices with `dtype=float`, or `float64`. - + Also, GTSAM expects *column-major* matrices, unlike the default storage - scheme in numpy. Hence, you should pass column-major matrices to gtsam using + - Also, GTSAM expects _column-major_ matrices, unlike the default storage + scheme in numpy. Hence, you should pass column-major matrices to GTSAM using the flag order='F'. And you always get column-major matrices back. For more details, see [this link](https://github.com/wouterboomsma/eigency#storage-layout---why-arrays-are-sometimes-transposed). - + Passing row-major matrices of different dtype, e.g. `int`, will also work + - Passing row-major matrices of different dtype, e.g. `int`, will also work as the wrapper converts them to column-major and dtype float for you, using numpy.array.astype(float, order='F', copy=False). However, this will result a copy if your matrix is not in the expected type and storage order. -- Inner namespace: Classes in inner namespace will be prefixed by _ in Python. +- Inner namespace: Classes in inner namespace will be prefixed by \_ in Python. Examples: `noiseModel_Gaussian`, `noiseModel_mEstimator_Tukey` - Casting from a base class to a derive class must be done explicitly. Examples: + ```python noiseBase = factor.noiseModel() noiseGaussian = dynamic_cast_noiseModel_Gaussian_noiseModel_Base(noiseBase) @@ -75,37 +82,35 @@ See the tests for examples. ## Wrapping Custom GTSAM-based Project -Please refer to the template project and the corresponding tutorial available [here](https://github.com/borglab/gtsam-project-python). +Please refer to the template project and the corresponding tutorial available [here](https://github.com/borglab/GTSAM-project-python). ## KNOWN ISSUES - - Doesn't work with python3 installed from homebrew - - size-related issue: can only wrap up to a certain number of classes: up to mEstimator! - - Guess: 64 vs 32b? disutils Compiler flags? - - Bug with Cython 0.24: instantiated factor classes return FastVector for keys(), which can't be casted to FastVector - - Upgrading to 0.25 solves the problem - - Need default constructor and default copy constructor for almost every classes... :( - - support these constructors by default and declare "delete" for special classes? - +- Doesn't work with python3 installed from homebrew + - size-related issue: can only wrap up to a certain number of classes: up to mEstimator! + - Guess: 64 vs 32b? disutils Compiler flags? +- Bug with Cython 0.24: instantiated factor classes return FastVector for keys(), which can't be casted to FastVector + - Upgrading to 0.25 solves the problem +- Need default constructor and default copy constructor for almost every classes... :( + - support these constructors by default and declare "delete" for special classes? ### TODO - [ ] allow duplication of parent' functions in child classes. Not allowed for now due to conflicts in Cython. -- [ ] a common header for boost shared_ptr? (Or wait until everything is switched to std::shared_ptr in gtsam?) +- [ ] a common header for boost shared_ptr? (Or wait until everything is switched to std::shared_ptr in GTSAM?) - [ ] inner namespaces ==> inner packages? - [ ] Wrap fixed-size Matrices/Vectors? - ### Completed/Cancelled: -- [x] Fix Python tests: don't use " import * ": Bad style!!! (18-03-17 19:50) +- [x] Fix Python tests: don't use " import \* ": Bad style!!! (18-03-17 19:50) - [x] Unit tests for cython wrappers @done (18-03-17 18:45) -- simply compare generated files - [x] Wrap unstable @done (18-03-17 15:30) -- [x] Unify cython/gtsam.h and the original gtsam.h @done (18-03-17 15:30) +- [x] Unify cython/GTSAM.h and the original GTSAM.h @done (18-03-17 15:30) - [x] 18-03-17: manage to unify the two versions by removing std container stubs from the matlab version,and keeping KeyList/KeyVector/KeySet as in the matlab version. Probably Cython 0.25 fixes the casting problem. - [x] 06-03-17: manage to remove the requirements for default and copy constructors - [ ] 25-11-16: Try to unify but failed. Main reasons are: Key/size_t, std containers, KeyVector/KeyList/KeySet. Matlab doesn't need to know about Key, but I can't make Cython to ignore Key as it couldn't cast KeyVector, i.e. FastVector, to FastVector. -- [ ] Marginal and JointMarginal: revert changes @failed (17-03-17 11:00) -- Cython does need a default constructor! It produces cpp code like this: ```gtsam::JointMarginal __pyx_t_1;``` Users don't have to wrap this constructor, however. +- [ ] Marginal and JointMarginal: revert changes @failed (17-03-17 11:00) -- Cython does need a default constructor! It produces cpp code like this: `GTSAM::JointMarginal __pyx_t_1;` Users don't have to wrap this constructor, however. - [x] Convert input numpy Matrix/Vector to float dtype and storage order 'F' automatically, cannot crash! @done (15-03-17 13:00) - [x] Remove requirements.txt - Frank: don't bother with only 2 packages and a special case for eigency! @done (08-03-17 10:30) - [x] CMake install script @done (25-11-16 02:30) @@ -119,7 +124,7 @@ Please refer to the template project and the corresponding tutorial available [h - [x] Casting from parent and grandparents @done (16-11-16 17:00) - [x] Allow overloading constructors. The current solution is annoying!!! @done (16-11-16 17:00) - [x] Support "print obj" @done (16-11-16 17:00) -- [x] methods for FastVector: at, [], ... @done (16-11-16 17:00) +- [x] methods for FastVector: at, [], ... @done (16-11-16 17:00) - [x] Cython: Key and size_t: traits doesn't exist @done (16-09-12 18:34) - [x] KeyVector, KeyList, KeySet... @done (16-09-13 17:19) - [x] [Nice to have] parse typedef @done (16-09-13 17:19) From d2f69eeab41044219f4a40314eaf98d62254d183 Mon Sep 17 00:00:00 2001 From: Varun Agrawal Date: Wed, 1 Jul 2020 17:07:31 -0500 Subject: [PATCH 26/29] Add python-install dependency for gtsam_unstable as well --- cmake/GtsamCythonWrap.cmake | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmake/GtsamCythonWrap.cmake b/cmake/GtsamCythonWrap.cmake index c155cbbd8..2f5582513 100644 --- a/cmake/GtsamCythonWrap.cmake +++ b/cmake/GtsamCythonWrap.cmake @@ -137,6 +137,8 @@ function(cythonize target pyx_file output_lib_we output_dir include_dirs libs in target_link_libraries(${target} "${libs}") endif() add_dependencies(${target} ${target}_pyx2cpp) + + add_dependencies(${python_install_target} ${target}) endfunction() # Internal function that wraps a library and compiles the wrapper @@ -169,8 +171,6 @@ function(wrap_library_cython interface_header generated_files_path extra_imports cythonize(cythonize_${module_name} ${generated_pyx} ${module_name} ${generated_files_path} "${include_dirs}" "${libs}" ${interface_header} cython_wrap_${module_name}_pyx) - add_dependencies(${python_install_target} cython_wrap_${module_name}_pyx) - # distclean add_custom_target(wrap_${module_name}_cython_distclean COMMAND cmake -E remove_directory ${generated_files_path}) From cb151dd9ee9426f60c371c4a49c043726f7a4afa Mon Sep 17 00:00:00 2001 From: Varun Agrawal Date: Sat, 4 Jul 2020 20:42:15 -0400 Subject: [PATCH 27/29] update python build location in travis script --- .travis.python.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.python.sh b/.travis.python.sh index 1ef5799aa..772311f38 100644 --- a/.travis.python.sh +++ b/.travis.python.sh @@ -34,7 +34,7 @@ cmake $CURRDIR -DCMAKE_BUILD_TYPE=Release \ make -j$(nproc) install -cd $CURRDIR/../gtsam_install/cython +cd cython sudo $PYTHON setup.py install From e08e39202074fed0b0a40fdb821e5f0726ec4799 Mon Sep 17 00:00:00 2001 From: Varun Agrawal Date: Sun, 5 Jul 2020 21:57:18 -0500 Subject: [PATCH 28/29] Improved paths and added checks --- cmake/GtsamCythonWrap.cmake | 13 +++++++++---- cython/gtsam_eigency/CMakeLists.txt | 4 +++- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/cmake/GtsamCythonWrap.cmake b/cmake/GtsamCythonWrap.cmake index 2f5582513..c8f876895 100644 --- a/cmake/GtsamCythonWrap.cmake +++ b/cmake/GtsamCythonWrap.cmake @@ -41,7 +41,7 @@ execute_process(COMMAND "${PYTHON_EXECUTABLE}" "-c" function(wrap_and_install_library_cython interface_header extra_imports install_path libs dependencies) # Paths for generated files get_filename_component(module_name "${interface_header}" NAME_WE) - set(generated_files_path "${GTSAM_CYTHON_INSTALL_PATH}/${module_name}") + set(generated_files_path "${install_path}") wrap_library_cython("${interface_header}" "${generated_files_path}" "${extra_imports}" "${libs}" "${dependencies}") endfunction() @@ -138,7 +138,9 @@ function(cythonize target pyx_file output_lib_we output_dir include_dirs libs in endif() add_dependencies(${target} ${target}_pyx2cpp) - add_dependencies(${python_install_target} ${target}) + if(TARGET ${python_install_target}) + add_dependencies(${python_install_target} ${target}) + endif() endfunction() # Internal function that wraps a library and compiles the wrapper @@ -151,9 +153,12 @@ function(wrap_library_cython interface_header generated_files_path extra_imports get_filename_component(module_name "${interface_header}" NAME_WE) # Wrap module to Cython pyx - message(STATUS "Cython wrapper generating ${module_name}.pyx") + message(STATUS "Cython wrapper generating ${generated_files_path}/${module_name}.pyx") set(generated_pyx "${generated_files_path}/${module_name}.pyx") - file(MAKE_DIRECTORY "${generated_files_path}") + if(NOT EXISTS ${generated_files_path}) + file(MAKE_DIRECTORY "${generated_files_path}") + endif() + add_custom_command( OUTPUT ${generated_pyx} DEPENDS ${interface_header} wrap diff --git a/cython/gtsam_eigency/CMakeLists.txt b/cython/gtsam_eigency/CMakeLists.txt index 663ea0a32..a0cf0fbde 100644 --- a/cython/gtsam_eigency/CMakeLists.txt +++ b/cython/gtsam_eigency/CMakeLists.txt @@ -37,4 +37,6 @@ add_dependencies(cythonize_eigency_core cythonize_eigency_conversions) add_custom_target(cythonize_eigency) add_dependencies(cythonize_eigency cythonize_eigency_conversions cythonize_eigency_core) -add_dependencies(${python_install_target} cythonize_eigency) +if(TARGET ${python_install_target}) + add_dependencies(${python_install_target} cythonize_eigency) +endif() From 66570469c5f890af0245ec0b1334c9a9c1294782 Mon Sep 17 00:00:00 2001 From: Varun Agrawal Date: Tue, 7 Jul 2020 17:38:27 -0400 Subject: [PATCH 29/29] fix working directory for python install target --- cython/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cython/CMakeLists.txt b/cython/CMakeLists.txt index 65a9e9c62..221025575 100644 --- a/cython/CMakeLists.txt +++ b/cython/CMakeLists.txt @@ -7,7 +7,7 @@ if (GTSAM_INSTALL_CYTHON_TOOLBOX) set(python_install_target python-install) add_custom_target(${python_install_target} COMMAND ${PYTHON_EXECUTABLE} ${GTSAM_CYTHON_INSTALL_PATH}/setup.py install - WORKING_DIRECTORY ${GTSAM_CYTHON_INSTALL_FULLPATH}) + WORKING_DIRECTORY ${GTSAM_CYTHON_INSTALL_PATH}) # build and include the eigency version of eigency add_subdirectory(gtsam_eigency)