Merge pull request #551 from borglab/fix/remove-cython

Remove all Cython references

.github/scripts/python.sh

@@ -43,11 +43,6 @@ if [ -z ${PYTHON_VERSION+x} ]; then
     exit 127
 fi
 
-if [ -z ${WRAPPER+x} ]; then
-    echo "Please provide the wrapper to build!"
-    exit 126
-fi
-
 PYTHON="python${PYTHON_VERSION}"
 
 if [[ $(uname) == "Darwin" ]]; then
@@ -61,25 +56,11 @@ PATH=$PATH:$($PYTHON -c "import site; print(site.USER_BASE)")/bin
 
 [ "${GTSAM_WITH_TBB:-OFF}" = "ON" ] && install_tbb
 
-case $WRAPPER in
-"cython")
-    BUILD_CYTHON="ON"
-    BUILD_PYBIND="OFF"
-    TYPEDEF_POINTS_TO_VECTORS="OFF"
 
-    sudo $PYTHON -m pip install -r $GITHUB_WORKSPACE/cython/requirements.txt
-    ;;
-"pybind")
-    BUILD_CYTHON="OFF"
-    BUILD_PYBIND="ON"
-    TYPEDEF_POINTS_TO_VECTORS="ON"
+BUILD_PYBIND="ON"
+TYPEDEF_POINTS_TO_VECTORS="ON"
 
-    sudo $PYTHON -m pip install -r $GITHUB_WORKSPACE/python/requirements.txt
-    ;;
-*)
-    exit 126
-    ;;
-esac
+sudo $PYTHON -m pip install -r $GITHUB_WORKSPACE/python/requirements.txt
 
 mkdir $GITHUB_WORKSPACE/build
 cd $GITHUB_WORKSPACE/build
@@ -90,7 +71,6 @@ cmake $GITHUB_WORKSPACE -DCMAKE_BUILD_TYPE=Release \
     -DGTSAM_WITH_TBB=${GTSAM_WITH_TBB:-OFF} \
     -DGTSAM_BUILD_EXAMPLES_ALWAYS=OFF \
     -DGTSAM_BUILD_WITH_MARCH_NATIVE=OFF \
-    -DGTSAM_INSTALL_CYTHON_TOOLBOX=${BUILD_CYTHON} \
     -DGTSAM_BUILD_PYTHON=${BUILD_PYBIND} \
     -DGTSAM_TYPEDEF_POINTS_TO_VECTORS=${TYPEDEF_POINTS_TO_VECTORS} \
     -DGTSAM_PYTHON_VERSION=$PYTHON_VERSION \
@@ -98,30 +78,10 @@ cmake $GITHUB_WORKSPACE -DCMAKE_BUILD_TYPE=Release \
     -DGTSAM_ALLOW_DEPRECATED_SINCE_V41=OFF \
     -DCMAKE_INSTALL_PREFIX=$GITHUB_WORKSPACE/gtsam_install
 
-make -j$(nproc) install &
+make -j$(nproc) install
 
-while ps -p $! > /dev/null
-do
-  sleep 60
-  now=$(date +%s)
-  printf "%d seconds have elapsed\n" $(( (now - start) ))
-done
 
-case $WRAPPER in
-"cython")
-    cd $GITHUB_WORKSPACE/build/cython
-    $PYTHON setup.py install --user --prefix=
-    cd $GITHUB_WORKSPACE/build/cython/gtsam/tests
-    $PYTHON -m unittest discover
-    ;;
-"pybind")
-    cd $GITHUB_WORKSPACE/build/python
-    $PYTHON setup.py install --user --prefix=
-    cd $GITHUB_WORKSPACE/python/gtsam/tests
-    $PYTHON -m unittest discover
-    ;;
-*)
-    echo "THIS SHOULD NEVER HAPPEN!"
-    exit 125
-    ;;
-esac
+cd $GITHUB_WORKSPACE/build/python
+$PYTHON setup.py install --user --prefix=
+cd $GITHUB_WORKSPACE/python/gtsam/tests
+$PYTHON -m unittest discover

.github/workflows/build-python.yml

@@ -12,7 +12,6 @@ jobs:
       CTEST_PARALLEL_LEVEL: 2
       CMAKE_BUILD_TYPE: ${{ matrix.build_type }}
       PYTHON_VERSION: ${{ matrix.python_version }}
-      WRAPPER: ${{ matrix.wrapper }}
     strategy:
       fail-fast: false
       matrix:
@@ -28,7 +27,6 @@ jobs:
 
         build_type: [Debug, Release]
         python_version: [3]
-        wrapper: [pybind]
         include:
           - name: ubuntu-18.04-gcc-5
             os: ubuntu-18.04

.github/workflows/trigger-python.yml

@@ -1,11 +1,11 @@
-# This triggers Cython builds on `gtsam-manylinux-build`
+# This triggers Python builds on `gtsam-manylinux-build`
 name: Trigger Python Builds
 on:
   push:
     branches:
       - develop
 jobs:
-  triggerCython:
+  triggerPython:
     runs-on: ubuntu-latest
     steps:
       - name: Repository Dispatch
@@ -13,5 +13,5 @@ jobs:
         with:
           token: ${{ secrets.PYTHON_CI_REPO_ACCESS_TOKEN }}
           repository: borglab/gtsam-manylinux-build
-          event-type: cython-wrapper
+          event-type: python-wrapper
           client-payload: '{"ref": "${{ github.ref }}", "sha": "${{ github.sha }}"}'

.gitignore

@@ -9,12 +9,6 @@
 *.txt.user
 *.txt.user.6d59f0c
 *.pydevproject
-cython/venv
-cython/gtsam.cpp
-cython/gtsam.cpython-35m-darwin.so
-cython/gtsam.pyx
-cython/gtsam.so
-cython/gtsam_wrapper.pxd
 .vscode
 .env
 /.vs/

INSTALL.md

@@ -173,7 +173,7 @@ NOTE:  If _GLIBCXX_DEBUG is used to compile gtsam, anything that links against g
 Intel has a guide for installing MKL on Linux through APT repositories at <https://software.intel.com/en-us/articles/installing-intel-free-libs-and-python-apt-repo>.
 
 After following the instructions, add the following to your `~/.bashrc` (and afterwards, open a new terminal before compiling GTSAM):
-`LD_PRELOAD` need only be set if you are building the cython wrapper to use GTSAM from python.
+`LD_PRELOAD` need only be set if you are building the python wrapper to use GTSAM from python.
 ```sh
 source /opt/intel/mkl/bin/mklvars.sh intel64
 export LD_PRELOAD="$LD_PRELOAD:/opt/intel/mkl/lib/intel64/libmkl_core.so:/opt/intel/mkl/lib/intel64/libmkl_sequential.so"
@@ -190,6 +190,6 @@ Failing to specify `LD_PRELOAD` may lead to errors such as:
 `ImportError: /opt/intel/mkl/lib/intel64/libmkl_vml_avx2.so: undefined symbol: mkl_serv_getenv`
 or
 `Intel MKL FATAL ERROR: Cannot load libmkl_avx2.so or libmkl_def.so.`
-when importing GTSAM using the cython wrapper in python.
+when importing GTSAM using the python wrapper.
 
 

cmake/CMakeLists.txt

@@ -19,7 +19,6 @@ install(FILES
   GtsamMatlabWrap.cmake
   GtsamTesting.cmake
   GtsamPrinting.cmake
-  FindCython.cmake
   FindNumPy.cmake
   README.html
   DESTINATION "${SCRIPT_INSTALL_DIR}/GTSAMCMakeTools")

cmake/FindCython.cmake

@@ -1,81 +0,0 @@
-# Modifed from: https://github.com/nest/nest-simulator/blob/master/cmake/FindCython.cmake
-#
-# Find the Cython compiler.
-#
-# This code sets the following variables:
-#
-#  CYTHON_FOUND
-#  CYTHON_PATH
-#  CYTHON_EXECUTABLE
-#  CYTHON_VERSION
-#
-# See also UseCython.cmake
-
-#=============================================================================
-# Copyright 2011 Kitware, Inc.
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-#     http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-#=============================================================================
-
-# Use the Cython executable that lives next to the Python executable
-# if it is a local installation.
-if(GTSAM_PYTHON_VERSION STREQUAL "Default")
-  find_package(PythonInterp)
-else()
-  find_package(PythonInterp ${GTSAM_PYTHON_VERSION} EXACT)
-endif()
-
-if ( PYTHONINTERP_FOUND )
-  execute_process( COMMAND "${PYTHON_EXECUTABLE}" "-c"
-      "import Cython; print(Cython.__path__[0])"
-      RESULT_VARIABLE RESULT
-      OUTPUT_VARIABLE CYTHON_PATH
-      OUTPUT_STRIP_TRAILING_WHITESPACE
-  )
-endif ()
-
-# RESULT=0 means ok
-if ( NOT RESULT )
-  get_filename_component( _python_path ${PYTHON_EXECUTABLE} PATH )
-  find_program( CYTHON_EXECUTABLE
-      NAMES cython cython.bat cython3
-      HINTS ${_python_path}
-   )
-endif ()
-
-# RESULT=0 means ok
-if ( NOT RESULT )
-  execute_process( COMMAND "${PYTHON_EXECUTABLE}" "-c"
-      "import Cython; print(Cython.__version__)"
-      RESULT_VARIABLE RESULT
-      OUTPUT_VARIABLE CYTHON_VAR_OUTPUT
-      ERROR_VARIABLE CYTHON_VAR_OUTPUT
-      OUTPUT_STRIP_TRAILING_WHITESPACE
-  )
-  if ( RESULT EQUAL 0 )
-    string( REGEX REPLACE ".* ([0-9]+\\.[0-9]+(\\.[0-9]+)?).*" "\\1"
-                          CYTHON_VERSION "${CYTHON_VAR_OUTPUT}" )
-  endif ()
-endif ()
-
-include( FindPackageHandleStandardArgs )
-find_package_handle_standard_args( Cython
-  FOUND_VAR
-    CYTHON_FOUND
-  REQUIRED_VARS
-    CYTHON_PATH
-    CYTHON_EXECUTABLE
-  VERSION_VAR
-    CYTHON_VERSION
-    )
-

docker/ubuntu-gtsam-python/Dockerfile

@@ -7,9 +7,9 @@ FROM dellaert/ubuntu-gtsam:bionic
 RUN apt-get install -y python3-pip python3-dev
 
 # Install python wrapper requirements
-RUN python3 -m pip install -U -r /usr/src/gtsam/cython/requirements.txt
+RUN python3 -m pip install -U -r /usr/src/gtsam/python/requirements.txt
 
-# Run cmake again, now with cython toolbox on
+# Run cmake again, now with python toolbox on
 WORKDIR /usr/src/gtsam/build
 RUN cmake \
     -DCMAKE_BUILD_TYPE=Release \
@@ -17,7 +17,7 @@ RUN cmake \
     -DGTSAM_BUILD_EXAMPLES_ALWAYS=OFF \
     -DGTSAM_BUILD_TIMING_ALWAYS=OFF \
     -DGTSAM_BUILD_TESTS=OFF \
-    -DGTSAM_INSTALL_CYTHON_TOOLBOX=ON \
+    -DGTSAM_BUILD_PYTHON=ON \
     -DGTSAM_PYTHON_VERSION=3\
     ..
 
@@ -25,7 +25,7 @@ RUN cmake \
 RUN make -j4 install && make clean
 
 # Needed to run python wrapper:
-RUN echo 'export PYTHONPATH=/usr/local/cython/:$PYTHONPATH' >> /root/.bashrc
+RUN echo 'export PYTHONPATH=/usr/local/python/:$PYTHONPATH' >> /root/.bashrc
 
 # Run bash
 CMD ["bash"]

docker/ubuntu-gtsam/Dockerfile

@@ -23,7 +23,6 @@ RUN cmake \
     -DGTSAM_BUILD_EXAMPLES_ALWAYS=OFF \
     -DGTSAM_BUILD_TIMING_ALWAYS=OFF \
     -DGTSAM_BUILD_TESTS=OFF \
-    -DGTSAM_INSTALL_CYTHON_TOOLBOX=OFF \
     ..
 
 # Build

gtsam/gtsam.i

@@ -93,9 +93,9 @@
  *        - Add "void serializable()" to a class if you only want the class to be serialized as a
  *          part of a container (such as noisemodel). This version does not require a publicly
  *          accessible default constructor.
- *   Forward declarations and class definitions for Cython:
- *     - Need to specify the base class (both this forward class and base class are declared in an external cython header)
- *       This is so Cython can generate proper inheritance.
+ *   Forward declarations and class definitions for Pybind:
+ *     - Need to specify the base class (both this forward class and base class are declared in an external Pybind header)
+ *       This is so Pybind can generate proper inheritance.
  *       Example when wrapping a gtsam-based project:
  *          // forward declarations
  *          virtual class gtsam::NonlinearFactor
@@ -104,7 +104,7 @@
  *          #include <MyFactor.h>
  *          virtual class MyFactor : gtsam::NoiseModelFactor {...};
  *    - *DO NOT* re-define overriden function already declared in the external (forward-declared) base class
- *        - This will cause an ambiguity problem in Cython pxd header file
+ *        - This will cause an ambiguity problem in Pybind header file
  */
 
 /**

gtsam/navigation/AHRSFactor.h

@@ -42,7 +42,7 @@ class GTSAM_EXPORT PreintegratedAhrsMeasurements : public PreintegratedRotation
 
  public:
 
-  /// Default constructor, only for serialization and Cython wrapper
+  /// Default constructor, only for serialization and wrappers
   PreintegratedAhrsMeasurements() {}
 
   /**

gtsam/navigation/CombinedImuFactor.h

@@ -145,7 +145,7 @@ public:
   /// @name Constructors
   /// @{
 
-  /// Default constructor only for serialization and Cython wrapper
+  /// Default constructor only for serialization and wrappers
   PreintegratedCombinedMeasurements() {
     preintMeasCov_.setZero();
   }

gtsam/navigation/ImuFactor.h

@@ -80,7 +80,7 @@ protected:
 
 public:
 
-  /// Default constructor for serialization and Cython wrapper
+  /// Default constructor for serialization and wrappers
   PreintegratedImuMeasurements() {
     preintMeasCov_.setZero();
   }

gtsam/nonlinear/Marginals.h

@@ -48,7 +48,7 @@ protected:
 
 public:
 
-  /// Default constructor only for Cython wrapper
+  /// Default constructor only for wrappers
   Marginals(){}
 
   /** Construct a marginals class from a nonlinear factor graph.
@@ -156,7 +156,7 @@ protected:
   FastMap<Key, size_t> indices_;
 
 public:
-  /// Default constructor only for Cython wrapper
+  /// Default constructor only for wrappers
   JointMarginal() {}
 
   /** Access a block, corresponding to a pair of variables, of the joint

gtsam_extra.cmake.in

@@ -7,8 +7,3 @@ set (GTSAM_VERSION_STRING "@GTSAM_VERSION_STRING@")
 
 set (GTSAM_USE_TBB @GTSAM_USE_TBB@)
 set (GTSAM_DEFAULT_ALLOCATOR @GTSAM_DEFAULT_ALLOCATOR@)
-
-if("@GTSAM_INSTALL_CYTHON_TOOLBOX@")
-  list(APPEND GTSAM_CYTHON_INSTALL_PATH "@GTSAM_CYTHON_INSTALL_PATH@")
-  list(APPEND GTSAM_EIGENCY_INSTALL_PATH "@GTSAM_EIGENCY_INSTALL_PATH@")
-endif()

python/gtsam/tests/test_JacobianFactor.py

@@ -19,7 +19,7 @@ from gtsam.utils.test_case import GtsamTestCase
 class TestJacobianFactor(GtsamTestCase):
 
     def test_eliminate(self):
-        # Recommended way to specify a matrix (see cython/README)
+        # Recommended way to specify a matrix (see python/README)
         Ax2 = np.array(
            [[-5., 0.],
             [+0., -5.],