Refactor Python Bindings (#105)

* refactor: modernize python bindings * refactor: workflows * fix: expose vtk system * fix: re-add deleted line * fix: only disable versioned soname * refactor: sync viennaps changes * Also add build path in Python bindings --------- Co-authored-by: Tobias Reiter <[email protected]>
ViennaTools · Mar 22, 2024 · 8cff5ff · 8cff5ff
1 parent aa693ce
commit 8cff5ff
Show file tree

Hide file tree

Showing 8 changed files with 140 additions and 333 deletions.
diff --git a/.github/workflows/test.yml → .github/workflows/build.yml b/.github/workflows/test.yml → .github/workflows/build.yml
@@ -11,9 +11,16 @@ jobs:
       fail-fast: false
 
       matrix:
-        config: [Release, Debug]
+        config:
+          - Release
+          - Debug
+
+        os:
+          - macos-latest
+          - ubuntu-latest
+          - windows-latest
+
         vtk-system: [true, false]
-        os: [ubuntu-latest, windows-latest, macos-latest]
 
         exclude:
           - os: macos-latest
@@ -25,7 +32,7 @@ jobs:
     runs-on: ${{ matrix.os }}
     container: ${{ matrix.os == 'ubuntu-latest' && 'fedora:38' || null }}
 
-    name: '🧪 Test on ${{ matrix.os }} [Config: "${{ matrix.config }}", VTK installed: "${{ matrix.vtk-system }}"]'
+    name: '🧪 Test on ${{ matrix.os }} ["${{ matrix.config }}", VTK: "${{ matrix.vtk-system }}"]'
 
     steps:
       - name: 📥 Checkout
@@ -40,13 +47,11 @@ jobs:
       - name: 🦥 Cache Dependencies
         uses: actions/cache@v3
         with:
-          key: ${{ matrix.os }}-${{ matrix.config }}-cache
-          path: |
-            deps-cache
-            build
+          key: test-${{ matrix.os }}-${{ matrix.config }}-${{ matrix.vtk-system }}
+          path: build
 
       - name: 🏗️ Compile
-        run: cmake -DCMAKE_BUILD_TYPE=${{ matrix.config }} -DCPM_SOURCE_CACHE=deps-cache -DVIENNALS_BUILD_TESTS=ON -B build && cmake --build build
+        run: cmake -DVIENNALS_BUILD_TESTS=ON -B build && cmake --build build --config ${{ matrix.config }}
 
       - name: 🧪 Test
         run: ctest -E "Benchmark|Performance" -C ${{ matrix.config }} --test-dir build
diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml
@@ -11,20 +11,12 @@ jobs:
       fail-fast: false
 
       matrix:
-        vtk-system: [true, false]
         os: [ubuntu-latest, windows-latest, macos-latest]
 
-        exclude:
-          - os: macos-latest
-            vtk-system: true
-
-          - os: windows-latest
-            vtk-system: true
-
     runs-on: ${{ matrix.os }}
     container: ${{ matrix.os == 'ubuntu-latest' && 'fedora:38' || null }}
 
-    name: '🐍 Build Bindings on ${{ matrix.os }} [VTK installed: "${{ matrix.vtk-system }}"]'
+    name: "🐍 Build Bindings on ${{ matrix.os }}"
 
     steps:
       - name: 📥 Checkout
@@ -34,7 +26,31 @@ jobs:
         uses: ./.github/actions/setup
         with:
           container: ${{ matrix.os }}
-          install-vtk: ${{ matrix.vtk-system }}
+          install-vtk: false
 
-      - name: 🐍 Build Python Module
-        run: pip3 install --user . -v
+      - name: 🦥 Cache Dependencies
+        uses: actions/cache@v3
+        with:
+          key: python-${{ matrix.os }}
+          path: build
+
+      - name: 🐍 Build and check Python Module (Windows)
+        if: ${{ matrix.os == 'windows-latest' }}
+        run: |
+          python3 -m venv venv
+          ./venv/Scripts/activate.bat
+          pip install .
+          python3 -c "import viennals2d; print(viennals2d.__doc__)"
+
+      - name: 🐍 Build and check Python Module (Other)
+        if: ${{ matrix.os != 'windows-latest' }}
+        run: |
+          python3 -m venv venv
+          ./venv/bin/pip install .
+          ./venv/bin/python -c "import viennals2d; print(viennals2d.__doc__)"
+
+      - name: 📦 Upload Artifact
+        uses: actions/upload-artifact@v3
+        with:
+          name: Pre-Built (${{ matrix.os }})
+          path: venv
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -2,7 +2,7 @@ cmake_minimum_required(VERSION 3.20 FATAL_ERROR)
 project(
   ViennaLS
   LANGUAGES CXX
-  VERSION 3.0.0)
+  VERSION 3.1.0)
 
 # --------------------------------------------------------------------------------------------------------
 # Library options
@@ -16,23 +16,30 @@ option(VIENNALS_STATIC_BUILD "Build dependencies as static libraries" OFF)
 option(VIENNALS_ENABLE_SANITIZER "Enable Sanitizer for debug buidlds" OFF)
 option(VIENNALS_USE_SANITIZER "Enable Sanitizer (Requires GCC or Clang)" OFF)
 
+option(VIENNALS_BUILD_PYTHON "Build python bindings" OFF)
 option(VIENNALS_BUILD_EXAMPLES "Build examples" OFF)
 option(VIENNALS_BUILD_TESTS "Build tests" OFF)
 
 # --------------------------------------------------------------------------------------------------------
-# Conditional Library options
+# Global CMake Configuration └ See:
+# https://github.com/ViennaTools/ViennaPS/blob/c76e371817a797dfe2800691f00cb93317b731fa/CMakeLists.txt#L30
 # --------------------------------------------------------------------------------------------------------
 
-include(CMakeDependentOption)
-
-cmake_dependent_option(VIENNALS_BUILD_PYTHON "Build for Python (>=3)" OFF "VIENNALS_USE_VTK" OFF)
+if(VIENNALS_BUILD_PYTHON)
+  set(CMAKE_PLATFORM_NO_VERSIONED_SONAME ON)
+endif()
 
 # --------------------------------------------------------------------------------------------------------
 # Configuration
 # --------------------------------------------------------------------------------------------------------
 
 include("cmake/openmp.cmake")
 
+if(VIENNALS_BUILD_PYTHON)
+  message(STATUS "[ViennaLS] Using VTK for python bindings")
+  set(VIENNALS_USE_VTK ON)
+endif()
+
 if(VIENNALS_STATIC_BUILD)
   message(STATUS "[ViennaLS] Enabling precompiled headers for static build")
   set(VIENNALS_PRECOMPILE_HEADERS ON)
@@ -90,12 +97,14 @@ include("cmake/vtk.cmake")
 CPMAddPackage(
   NAME PackageProject
   VERSION 1.11.1
-  GIT_REPOSITORY "https://github.com/TheLartians/PackageProject.cmake")
+  GIT_REPOSITORY "https://github.com/TheLartians/PackageProject.cmake"
+  EXCLUDE_FROM_ALL ${VIENNALS_BUILD_PYTHON})
 
 CPMFindPackage(
   NAME ViennaHRLE
   VERSION 0.4.0
-  GIT_REPOSITORY "https://github.com/ViennaTools/ViennaHRLE")
+  GIT_REPOSITORY "https://github.com/ViennaTools/ViennaHRLE"
+  EXCLUDE_FROM_ALL ${VIENNALS_BUILD_PYTHON})
 
 find_package(OpenMP REQUIRED)
 target_link_libraries(${PROJECT_NAME} INTERFACE OpenMP::OpenMP_CXX ViennaHRLE)
@@ -104,7 +113,9 @@ if(VIENNALS_USE_VTK)
   enable_language(C)
 
   find_package(VTK QUIET)
-  set(VIENNALS_SYSTEM_VTK ${VTK_FOUND})
+  set(VIENNALS_SYSTEM_VTK
+      ${VTK_FOUND}
+      CACHE INTERNAL "")
 
   CPMFindPackage(
     NAME VTK
@@ -113,6 +124,7 @@ if(VIENNALS_USE_VTK)
     GIT_TAG 99bd602bdbe8024c55e8382f7cf1013d42a14601
     VERSION 9.0.0
     GIT_REPOSITORY "https://gitlab.kitware.com/vtk/vtk"
+    EXCLUDE_FROM_ALL ${VIENNALS_BUILD_PYTHON}
     OPTIONS "VTK_LEGACY_REMOVE ON"
             "VTK_SMP_IMPLEMENTATION_TYPE \"OpenMP\""
             "VTK_GROUP_ENABLE_Rendering NO"
@@ -169,12 +181,12 @@ if(VIENNALS_PRECOMPILE_HEADERS)
     set(VIENNALS_LINKAGE STATIC)
   endif()
 
-  add_library(${PROJECT_NAME}_Lib ${VIENNALS_LINKAGE})
-  add_library(${PROJECT_NAME}::Lib ALIAS ${PROJECT_NAME}_Lib)
+  add_library(${PROJECT_NAME}Lib ${VIENNALS_LINKAGE})
+  add_library(${PROJECT_NAME}::Lib ALIAS ${PROJECT_NAME}Lib)
 
-  target_link_libraries(${PROJECT_NAME}_Lib PUBLIC ${PROJECT_NAME})
-  target_sources(${PROJECT_NAME}_Lib PUBLIC "lib/specialisations.cpp")
-  set_target_properties(${PROJECT_NAME}_Lib PROPERTIES WINDOWS_EXPORT_ALL_SYMBOLS ON)
+  target_link_libraries(${PROJECT_NAME}Lib PUBLIC ${PROJECT_NAME})
+  target_sources(${PROJECT_NAME}Lib PUBLIC "lib/specialisations.cpp")
+  set_target_properties(${PROJECT_NAME}Lib PROPERTIES WINDOWS_EXPORT_ALL_SYMBOLS ON)
 endif()
 
 # --------------------------------------------------------------------------------------------------------
@@ -204,6 +216,7 @@ endif()
 if(VIENNALS_BUILD_PYTHON)
   message(STATUS "[ViennaLS] Building Python Bindings")
   add_subdirectory(python)
+  return()
 endif()
 
 # --------------------------------------------------------------------------------------------------------

diff --git a/pyproject.toml b/pyproject.toml
@@ -0,0 +1,21 @@
+[build-system]
+requires = ["scikit-build-core", "pybind11"]
+build-backend = "scikit_build_core.build"
+
+[project]
+version = "3.1.0"
+name = "ViennaLS_Python"
+readme = "README.md"
+license = {file = "LICENSE"}
+description = "A high performance sparse level set library"
+dependencies = ['vtk >= 9.0.0, < 10']
+
+[project.urls]
+Homepage = "https://viennatools.github.io/ViennaLS/"
+Documentation = "https://viennatools.github.io/ViennaLS/"
+Repository = "https://github.com/ViennaTools/ViennaLS"
+Issues = "https://github.com/ViennaTools/ViennaLS/issues"
+
+[tool.scikit-build]
+build-dir = "build"
+cmake.args = ["-DVIENNALS_BUILD_PYTHON=ON"]
diff --git a/python/CMakeLists.txt b/python/CMakeLists.txt
@@ -3,16 +3,31 @@ project(ViennaLS_Python LANGUAGES CXX)
 
 add_custom_target(${PROJECT_NAME} ALL)
 
+# --------------------------------------------------------------------------------------------------------
+# Global CMake Configuration └ See:
+# https://github.com/ViennaTools/ViennaPS/blob/c76e371817a797dfe2800691f00cb93317b731fa/python/CMakeLists.txt#L8
+# --------------------------------------------------------------------------------------------------------
+
+set(CMAKE_MACOSX_RPATH ON)
+set(CMAKE_INSTALL_RPATH_USE_LINK_PATH ON)
+
+if(NOT APPLE)
+  list(APPEND CMAKE_INSTALL_RPATH "$ORIGIN")
+  list(APPEND CMAKE_INSTALL_RPATH "$ORIGIN/../vtkmodules")
+  list(APPEND CMAKE_INSTALL_RPATH "$ORIGIN/../viennals.libs")
+else()
+  list(APPEND CMAKE_INSTALL_RPATH "@loader_path")
+  list(APPEND CMAKE_INSTALL_RPATH "@loader_path/../vtkmodules")
+  list(APPEND CMAKE_INSTALL_RPATH "@loader_path/../viennals.libs")
+endif()
+
 # --------------------------------------------------------------------------------------------------------
 # Setup Dependencies
 # --------------------------------------------------------------------------------------------------------
 
 include("../cmake/cpm.cmake")
 
 set(PYBIND11_FINDPYTHON ON)
-set(PYBIND11_PYTHON_VERSION
-    3
-    CACHE STRING "Python version")
 
 CPMFindPackage(
   NAME pybind11
@@ -32,55 +47,35 @@ if(CMAKE_LIBRARY_OUTPUT_DIRECTORY)
 endif()
 
 # --------------------------------------------------------------------------------------------------------
-# VTK-Setup
+# Binding macro
 # --------------------------------------------------------------------------------------------------------
 
-include("../cmake/vtk.cmake")
-
-if(WIN32 AND NOT VIENNALS_SYSTEM_VTK)
-  set(CMAKE_RUNTIME_OUTPUT_DIRECTORY $<1:${PROJECT_BINARY_DIR}/vtk_env>)
-  setup_vtk_env(${PROJECT_NAME} ${CMAKE_RUNTIME_OUTPUT_DIRECTORY})
-endif()
-
-if(NOT VIENNALS_SYSTEM_VTK)
-  message(STATUS "[ViennaLS] Setting VTK_DIR to previously created environment")
-  # We set `VTK_DIR` to the previously setup environment to ensure that the python script can find
-  # the library files
-  set(VTK_DIR "${CMAKE_RUNTIME_OUTPUT_DIRECTORY}")
-endif()
-
-# --------------------------------------------------------------------------------------------------------
-# Setup 2D-Bindings
-# --------------------------------------------------------------------------------------------------------
+macro(setup_binding NAME FLAGS)
+  set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${VIENNALS_LIBRARY_OUTPUT_DIR}/${NAME})
+  set(MODULE_NAME "_${NAME}")
 
-set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${VIENNALS_LIBRARY_OUTPUT_DIR}/viennals2d)
-set(VIENNALS_PYTHON_MODULE_NAME_2D "_${VIENNALS_PYTHON_MODULE_NAME}2d")
+  pybind11_add_module("${MODULE_NAME}" "pyWrap.cpp")
+  add_dependencies(${PROJECT_NAME} ${MODULE_NAME})
 
-pybind11_add_module("${VIENNALS_PYTHON_MODULE_NAME_2D}" "pyWrap.cpp")
-add_dependencies(${PROJECT_NAME} ${VIENNALS_PYTHON_MODULE_NAME_2D})
+  target_link_libraries(${MODULE_NAME} PUBLIC ViennaLS)
+  target_compile_definitions(${MODULE_NAME} PRIVATE ${FLAGS} -DVIENNALS_MODULE_NAME=${MODULE_NAME})
 
-target_link_libraries(${VIENNALS_PYTHON_MODULE_NAME_2D} PUBLIC ViennaLS)
-target_compile_definitions(
-  ${VIENNALS_PYTHON_MODULE_NAME_2D}
-  PRIVATE -DVIENNALS_PYTHON_DIMENSION=2 -DVIENNALS_MODULE_NAME=${VIENNALS_PYTHON_MODULE_NAME_2D})
+  configure_file(__init__.py.in ${CMAKE_LIBRARY_OUTPUT_DIRECTORY}/__init__.py)
 
-set(MODULE_NAME ${VIENNALS_PYTHON_MODULE_NAME_2D})
-configure_file(__init__.py.in ${CMAKE_LIBRARY_OUTPUT_DIRECTORY}/__init__.py)
+  install(TARGETS ${MODULE_NAME} LIBRARY DESTINATION ${NAME})
+  install(DIRECTORY "${CMAKE_LIBRARY_OUTPUT_DIRECTORY}/" DESTINATION ${NAME})
+endmacro()
 
 # --------------------------------------------------------------------------------------------------------
-# Setup 3D-Bindings
+# Setup Bindings
 # --------------------------------------------------------------------------------------------------------
 
-set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${VIENNALS_LIBRARY_OUTPUT_DIR}/viennals3d)
-set(VIENNALS_PYTHON_MODULE_NAME_3D "_${VIENNALS_PYTHON_MODULE_NAME}3d")
-
-pybind11_add_module("${VIENNALS_PYTHON_MODULE_NAME_3D}" "pyWrap.cpp")
-add_dependencies(${PROJECT_NAME} ${VIENNALS_PYTHON_MODULE_NAME_3D})
+setup_binding(viennals2d -DVIENNALS_PYTHON_DIMENSION=2)
+setup_binding(viennals3d -DVIENNALS_PYTHON_DIMENSION=3)
 
-target_link_libraries(${VIENNALS_PYTHON_MODULE_NAME_3D} PUBLIC ViennaLS)
-target_compile_definitions(
-  ${VIENNALS_PYTHON_MODULE_NAME_3D}
-  PRIVATE -DVIENNALS_PYTHON_DIMENSION=3 -DVIENNALS_MODULE_NAME=${VIENNALS_PYTHON_MODULE_NAME_3D})
+set(VIENNALS_LIB_FOLDER "${VIENNALS_LIBRARY_OUTPUT_DIR}/viennals.libs")
 
-set(MODULE_NAME ${VIENNALS_PYTHON_MODULE_NAME_3D})
-configure_file(__init__.py.in ${CMAKE_LIBRARY_OUTPUT_DIRECTORY}/__init__.py)
+if(WIN32 AND NOT VIENNALS_SYSTEM_VTK)
+  setup_vtk_env(${MODULE_NAME} "${VIENNALS_LIB_FOLDER}")
+  install(DIRECTORY "${VIENNALS_LIB_FOLDER}" DESTINATION .)
+endif()
diff --git a/python/__init__.py.in b/python/__init__.py.in
@@ -8,31 +8,26 @@ tailored towards scientific simulations. ViennaLS can also be used for
 visualisation applications, although this is not the main design target.
 """
 
-import os
 import sys
 
-extra_dll_dir = os.path.dirname(__file__)
-
-if sys.platform == 'win32' and os.path.isdir(extra_dll_dir):
-    # Add the VTK dll directory
+def _windows_dll_path():
 
-    vtk_path = "@VTK_DIR@".replace("/lib/cmake/vtk", "")
+    import os
+    import vtk
 
-    if vtk_path:
-        os.add_dll_directory(os.path.abspath("@VTK_DIR@"))
-    else:
-        os.add_dll_directory(os.path.join(os.path.abspath("@VTK_DIR@".replace("/lib/cmake/vtk", "")), "bin"))
+    vtk_path = vtk.__path__[0]
 
-    # Add the directory of the python bindings
-
-    os.add_dll_directory(extra_dll_dir)
+    additional_paths = [
+        vtk_path,
+        os.path.join(os.path.dirname(vtk_path), 'vtk.libs'),
+        os.path.join(os.path.dirname(os.path.dirname(__file__)), 'viennals.libs')
+    ]
 
-    # Remove internal symbols
+    for path in additional_paths:
+        os.add_dll_directory(path)
+        os.environ["PATH"] = path + os.pathsep + os.environ["PATH"]
 
-    del vtk_path
+if sys.platform == "win32":
+    _windows_dll_path()
 
 from .@MODULE_NAME@ import *
-
-# Remove internal symbols
-
-del extra_dll_dir
diff --git a/requirements.txt b/requirements.txt