Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix GTSAM MATLAB Wrapper #698

Merged
merged 4 commits into from
Feb 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions .github/workflows/build-macos.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,8 @@ jobs:
- name: Install (macOS)
if: runner.os == 'macOS'
run: |
brew tap ProfFan/robotics
brew install cmake ninja
brew install ProfFan/robotics/boost
brew install boost
if [ "${{ matrix.compiler }}" = "gcc" ]; then
brew install gcc@${{ matrix.version }}
echo "CC=gcc-${{ matrix.version }}" >> $GITHUB_ENV
Expand Down
15 changes: 13 additions & 2 deletions cmake/GtsamMatlabWrap.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@ if(GTSAM_INSTALL_MATLAB_TOOLBOX)
endif()
endif()

# Fixup the Python paths
if(GTWRAP_DIR)
# packaged
set(GTWRAP_PACKAGE_DIR ${GTWRAP_DIR})
else()
set(GTWRAP_PACKAGE_DIR ${CMAKE_SOURCE_DIR}/wrap)
endif()

# Set up cache options
option(GTSAM_MEX_BUILD_STATIC_MODULE "Build MATLAB wrapper statically (increases build time)" OFF)
set(GTSAM_BUILD_MEX_BINARY_FLAGS "" CACHE STRING "Extra flags for running Matlab MEX compilation")
Expand Down Expand Up @@ -239,15 +247,18 @@ function(wrap_library_internal interfaceHeader linkLibraries extraIncludeDirs ex


set(_ignore gtsam::Point2
gtsam::Point3)
gtsam::Point3
gtsam::BearingRangeFactor
gtsam::BearingRangeFactor2D
gtsam::BearingRangeFactorPose2)

# set the matlab wrapping script variable
set(MATLAB_WRAP_SCRIPT "${GTSAM_SOURCE_DIR}/wrap/scripts/matlab_wrap.py")

add_custom_command(
OUTPUT ${generated_cpp_file}
DEPENDS ${interfaceHeader} ${module_library_target} ${otherLibraryTargets} ${otherSourcesAndObjects}
COMMAND
COMMAND ${CMAKE_COMMAND} -E env "PYTHONPATH=${GTWRAP_PACKAGE_DIR}${GTWRAP_PATH_SEPARATOR}$ENV{PYTHONPATH}"
${PYTHON_EXECUTABLE}
${MATLAB_WRAP_SCRIPT}
--src ${interfaceHeader}
Expand Down
33 changes: 7 additions & 26 deletions wrap/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -26,34 +26,15 @@ install(FILES cmake/gtwrapConfig.cmake cmake/PybindWrap.cmake
cmake/GtwrapUtils.cmake
DESTINATION "${SCRIPT_INSTALL_DIR}/gtwrap")

include(GNUInstallDirs)
# Install the gtwrap python package as a directory so it can be found for
# wrapping.
install(DIRECTORY gtwrap DESTINATION "${CMAKE_INSTALL_DATADIR}/gtwrap")

# Install wrapping scripts as binaries to `CMAKE_INSTALL_PREFIX/bin` so they can
# be invoked for wrapping.
# We use DESTINATION (instead of TYPE) so we can support older CMake versions.
install(PROGRAMS scripts/pybind_wrap.py scripts/matlab_wrap.py
DESTINATION ${CMAKE_INSTALL_BINDIR})
install(PROGRAMS scripts/pybind_wrap.py scripts/matlab_wrap.py TYPE BIN)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be using TYPE? I left a comment saying that we need DESTINATION for older versions of CMake.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed :) I'll push a change to this to wrap (but let's first merge this)


# Install pybind11 directory to `CMAKE_INSTALL_PREFIX/lib/pybind11` This will
# allow the gtwrapConfig.cmake file to load it later.
# We use DESTINATION (instead of TYPE) so we can support older CMake versions.
install(DIRECTORY pybind11
DESTINATION ${CMAKE_INSTALL_LIBDIR})

# ##############################################################################
# Install the Python package
find_package(
Python ${WRAP_PYTHON_VERSION}
COMPONENTS Interpreter
EXACT)

# Detect virtualenv and set Pip args accordingly
# https://www.scivision.dev/cmake-install-python-package/
if(DEFINED ENV{VIRTUAL_ENV} OR DEFINED ENV{CONDA_PREFIX})
set(_pip_args)
else()
set(_pip_args "--user")
endif()
#TODO add correct flags for virtualenv

# Finally install the gtwrap python package.
execute_process(COMMAND ${Python_EXECUTABLE} -m pip install . ${_pip_args}
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR})
install(DIRECTORY pybind11 DESTINATION "${CMAKE_INSTALL_LIBDIR}/gtwrap")
8 changes: 4 additions & 4 deletions wrap/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,18 @@ cmake ..
make install # use sudo if needed
```

Using `wrap` in your project is straightforward from here. In you `CMakeLists.txt` file, you just need to add the following:
Using `wrap` in your project is straightforward from here. In your `CMakeLists.txt` file, you just need to add the following:

```cmake
include(PybindWrap)
find_package(gtwrap)

pybind_wrap(${PROJECT_NAME}_py # target
${PROJECT_SOURCE_DIR}/cpp/${PROJECT_NAME}.h # interface header file
"${PROJECT_NAME}.cpp" # the generated cpp
"${PROJECT_NAME}" # module_name
"gtsam" # top namespace in the cpp file
"${PROJECT_MODULE_NAME}" # top namespace in the cpp file e.g. gtsam
"${ignore}" # ignore classes
${PROJECT_BINARY_DIR}/${PROJECT_NAME}.tpl
${PROJECT_BINARY_DIR}/${PROJECT_NAME}.tpl # the wrapping template file
${PROJECT_NAME} # libs
"${PROJECT_NAME}" # dependencies
ON # use boost
Expand Down
18 changes: 16 additions & 2 deletions wrap/cmake/PybindWrap.cmake
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
set(PYBIND11_PYTHON_VERSION ${WRAP_PYTHON_VERSION})

if(GTWRAP_PYTHON_PACKAGE_DIR)
# packaged
set(GTWRAP_PACKAGE_DIR "${GTWRAP_PYTHON_PACKAGE_DIR}")
else()
set(GTWRAP_PACKAGE_DIR ${CMAKE_CURRENT_LIST_DIR}/..)
endif()

# User-friendly Pybind11 wrapping and installing function.
# Builds a Pybind11 module from the provided interface_header.
# For example, for the interface header gtsam.h, this will
Expand Down Expand Up @@ -35,9 +42,16 @@ function(pybind_wrap
else(USE_BOOST)
set(_WRAP_BOOST_ARG "")
endif(USE_BOOST)


if (UNIX)
set(GTWRAP_PATH_SEPARATOR ":")
else()
set(GTWRAP_PATH_SEPARATOR ";")
endif()

add_custom_command(OUTPUT ${generated_cpp}
COMMAND ${PYTHON_EXECUTABLE}
COMMAND ${CMAKE_COMMAND} -E env "PYTHONPATH=${GTWRAP_PACKAGE_DIR}${GTWRAP_PATH_SEPARATOR}$ENV{PYTHONPATH}"
${PYTHON_EXECUTABLE}
${PYBIND_WRAP_SCRIPT}
--src
${interface_header}
Expand Down
18 changes: 11 additions & 7 deletions wrap/cmake/gtwrapConfig.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,13 @@ set(GTWRAP_DIR "${CMAKE_CURRENT_LIST_DIR}")
list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}")

if(WIN32 AND NOT CYGWIN)
set(SCRIPT_INSTALL_DIR "${CMAKE_INSTALL_PREFIX}/CMake")
set(GTWRAP_CMAKE_DIR "${GTWRAP_DIR}")
set(GTWRAP_SCRIPT_DIR ${GTWRAP_CMAKE_DIR}/../../../bin)
set(GTWRAP_PYTHON_PACKAGE_DIR ${GTWRAP_CMAKE_DIR}/../../../share/gtwrap)
else()
set(SCRIPT_INSTALL_DIR "${CMAKE_INSTALL_PREFIX}/lib/cmake")
set(GTWRAP_CMAKE_DIR "${GTWRAP_DIR}")
set(GTWRAP_SCRIPT_DIR ${GTWRAP_CMAKE_DIR}/../../../bin)
set(GTWRAP_PYTHON_PACKAGE_DIR ${GTWRAP_CMAKE_DIR}/../../../share/gtwrap)
endif()

# Standard includes
Expand All @@ -16,12 +20,12 @@ include(CMakePackageConfigHelpers)
include(CMakeDependentOption)

# Load all the CMake scripts from the standard location
include(${SCRIPT_INSTALL_DIR}/gtwrap/PybindWrap.cmake)
include(${SCRIPT_INSTALL_DIR}/gtwrap/GtwrapUtils.cmake)
include(${GTWRAP_CMAKE_DIR}/PybindWrap.cmake)
include(${GTWRAP_CMAKE_DIR}/GtwrapUtils.cmake)

# Set the variables for the wrapping scripts to be used in the build.
set(PYBIND_WRAP_SCRIPT "${CMAKE_INSTALL_FULL_BINDIR}/pybind_wrap.py")
set(MATLAB_WRAP_SCRIPT "${CMAKE_INSTALL_FULL_BINDIR}/matlab_wrap.py")
set(PYBIND_WRAP_SCRIPT "${GTWRAP_SCRIPT_DIR}/pybind_wrap.py")
set(MATLAB_WRAP_SCRIPT "${GTWRAP_SCRIPT_DIR}/matlab_wrap.py")

# Load the pybind11 code from the library installation path
add_subdirectory(${CMAKE_INSTALL_FULL_LIBDIR}/pybind11 pybind11)
add_subdirectory(${GTWRAP_CMAKE_DIR}/../../gtwrap/pybind11 pybind11)
9 changes: 7 additions & 2 deletions wrap/gtwrap/matlab_wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -1010,7 +1010,8 @@ def wrap_instantiated_class(self, instantiated_class, namespace_name=''):
file_name = self._clean_class_name(instantiated_class)
namespace_file_name = namespace_name + file_name

if instantiated_class.cpp_class() in self.ignore_classes:
uninstantiated_name = "::".join(instantiated_class.namespaces()[1:]) + "::" + instantiated_class.name
if uninstantiated_name in self.ignore_classes:
return None

# Class comment
Expand Down Expand Up @@ -1518,7 +1519,11 @@ def generate_wrapper(self, namespace):
ptr_ctor_frag = ''

for cls in self.classes:
if cls.cpp_class().strip() in self.ignore_classes:
uninstantiated_name = "::".join(cls.namespaces()[1:]) + "::" + cls.name
self._debug("Cls: {} -> {}".format(cls.name, uninstantiated_name))

if uninstantiated_name in self.ignore_classes:
self._debug("Ignoring: {} -> {}".format(cls.name, uninstantiated_name))
continue

def _has_serialization(cls):
Expand Down