Skip to content

Commit

Permalink
Build using scikit-build-core and nanobind (#7)
Browse files Browse the repository at this point in the history
* remove submodules

* implement nanobind wrapper

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix uninitialized pointer

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
akaszynski and pre-commit-ci[bot] authored Jul 16, 2024
1 parent 07dcca0 commit 946f99e
Show file tree
Hide file tree
Showing 22 changed files with 468 additions and 506 deletions.
23 changes: 0 additions & 23 deletions .flake8

This file was deleted.

58 changes: 29 additions & 29 deletions .github/workflows/build-and-deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ on:
pull_request:
push:
tags:
- "*"
- '*'
branches:
- "main"
- main

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
Expand All @@ -21,44 +21,44 @@ jobs:
os: [ubuntu-latest, windows-latest, macos-14, macos-13]

steps:
- uses: actions/checkout@v4
with:
submodules: true
- uses: actions/checkout@v4
with:
submodules: true

- name: Build wheels
uses: pypa/[email protected]
- name: Build wheels
uses: pypa/[email protected]

- uses: actions/upload-artifact@v4
with:
name: pyminiply-wheels-${{ matrix.os }}
path: ./wheelhouse/*.whl
- uses: actions/upload-artifact@v4
with:
name: pyminiply-wheels-${{ matrix.os }}
path: ./wheelhouse/*.whl

build_sdist:
name: Build source distribution
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
submodules: true
- uses: actions/checkout@v4
with:
submodules: true

- name: Build sdist
run: pipx run build --sdist
- name: Build sdist
run: pipx run build --sdist

- name: Validate wheel
run: |
pip install twine
twine check dist/*
- name: Validate wheel
run: |
pip install twine
twine check dist/*
- name: Validate sdist
run: |
pip install dist/*
pip install pytest pyvista
pytest
- name: Validate sdist
run: |
pip install dist/*
pip install pytest pyvista
pytest
- uses: actions/upload-artifact@v4
with:
name: pyminiply-sdist
path: dist/*.tar.gz
- uses: actions/upload-artifact@v4
with:
name: pyminiply-sdist
path: dist/*.tar.gz

upload_pypi:
needs: [build_wheels, build_sdist]
Expand Down
11 changes: 11 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,14 @@ dist/
pyminiply/_wrapper.cpp
*.so
*.pyc

# cmake
#######
CMakeCache.txt
CMakeFiles/
Makefile
cmake_install.cmake
compile_commands.json

# cache
.cache
4 changes: 2 additions & 2 deletions .gitmodules
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
[submodule "pyminiply/miniply"]
path = pyminiply/miniply
[submodule "src/miniply"]
path = src/miniply
url = https://github.com/vilya/miniply
56 changes: 23 additions & 33 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,72 +1,62 @@
# See https://pre-commit.ci/
ci:
autofix_prs: true
autoupdate_schedule: quarterly
autofix_prs: true
autoupdate_schedule: quarterly

repos:
- repo: https://github.com/psf/black
rev: 23.12.1
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.5.2
hooks:
- id: black
- id: ruff
args: [--fix, --exit-non-zero-on-fix]
exclude: ^(docs/|tests)
- id: ruff-format

- repo: https://github.com/keewis/blackdoc
rev: v0.3.9
hooks:
- id: blackdoc
exclude: README.rst

- repo: https://github.com/pycqa/isort
rev: 5.13.2
hooks:
- id: isort
exclude: "setup.py"
args: [
"--profile", "black",
"--force-sort-within-sections",
"--skip-glob", "*__init__.py",
]

- repo: https://github.com/PyCQA/flake8
rev: 6.1.0
hooks:
- id: flake8

- repo: https://github.com/codespell-project/codespell
rev: v2.2.6
rev: v2.3.0
hooks:
- id: codespell
args: ["--skip=*.vt*"]
args: [--skip=*.vt*]

- repo: https://github.com/pycqa/pydocstyle
rev: 6.3.0
hooks:
- id: pydocstyle
additional_dependencies: [tomli==2.0.1]
files: ^pyminiply/.*\.py
files: ^src/pyminiply/.*\.py

- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.5.0
rev: v4.6.0
hooks:
- id: check-merge-conflict
- id: debug-statements
- id: trailing-whitespace
exclude: .*\.cdb$

- repo: https://github.com/macisamuele/language-formatters-pre-commit-hooks
rev: v2.14.0
hooks:
- id: pretty-format-toml
args: [--autofix]
- id: pretty-format-yaml
args: [--autofix, --indent, '2']

- repo: https://github.com/pre-commit/mirrors-clang-format
rev: v17.0.6
hooks:
- id: clang-format
files: |
(?x)^(
pyminiply/[^_].*\.cpp.*
)$

- repo: https://github.com/python-jsonschema/check-jsonschema
rev: 0.27.3
rev: 0.29.0
hooks:
- id: check-github-workflows
- id: check-github-workflows

- repo: https://github.com/dzhu/rstfmt
rev: v0.0.14
hooks:
- id: rstfmt
- id: rstfmt
53 changes: 53 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
cmake_minimum_required(VERSION 3.15...3.26)

project(nanobind_project LANGUAGES CXX)
# find_package(OpenMP REQUIRED)

# Try to import all Python components potentially needed by nanobind
find_package(Python 3.8
REQUIRED COMPONENTS Interpreter Development.Module
OPTIONAL_COMPONENTS Development.SABIModule)

# Import nanobind through CMake's find_package mechanism
find_package(nanobind CONFIG REQUIRED)

nanobind_add_module(
# Name of the extension
_wrapper

# Target the stable ABI for Python 3.12+, which reduces
# the number of binary wheels that must be built. This
# does nothing on older Python versions
STABLE_ABI

# conserve space by reusing a shared libnanobind across libraries
NB_STATIC

src/wrapper.cpp
src/miniply/miniply.cpp
)

# Link OpenMP
# if(OpenMP_CXX_FOUND)
# target_link_libraries(_wrapper PRIVATE OpenMP::OpenMP_CXX)
# endif()

# Compiler-specific options
if(MSVC)
# Use MSVC optimization levels and OpenMP setup
target_compile_options(_wrapper PRIVATE /O2 /std:c++17)
# /openmp:llvm
else()
# Assuming GCC or Clang
target_compile_options(_wrapper PRIVATE -O3)
# -fopenmp
endif()

# Example debugging
# set solib-search-path /home/user/python/.venv311/lib/python3.11/site-packages/stl_reader/
# set breakpoint with b qual.cpp:4872
# target_compile_options(stl_reader PRIVATE -g -O0)
# target_compile_options(pfh PRIVATE -g -O0)

# Install directive for scikit-build-core
install(TARGETS _wrapper LIBRARY DESTINATION pyminiply)
57 changes: 57 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
### C Extension

This repository interfaces with `miniply/miniply.cpp` via
[nanobind](https://github.com/wjakob/nanobind) to efficiently generate C++
extensions.

#### Emacs configuration

If using emacs and helm, generate the project configuration files using `-DCMAKE_EXPORT_COMPILE_COMMANDS=ON`. Here's a sample configuration for C++11 on Linux:

```
pip install nanobind
export NANOBIND_INCLUDE=$(python -c "import nanobind, os; print(os.path.join(os.path.dirname(nanobind.__file__), 'cmake'))")
cmake -Dnanobind_DIR=$NANOBIND_INCLUDE -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DCMAKE_CXX_STANDARD_INCLUDE_DIRECTORIES="/usr/include/c++/11;/usr/include/x86_64-linux-gnu/c++/11/;/usr/lib/gcc/x86_64-linux-gnu/11/include/"
```

These will be necessary for helm and treesit to determine the locations of the header files.


#### Debug Build

This can be helpful when debugging segfaults since this extension often uses raw pointers.


Set the cmake build type to debug in `pyproject.toml``
```
[tool.scikit-build]
cmake.build-type = "Debug"
```

Set the target compile options to build debug symbols with `-g` and `-O0` in `CMakeLists.txt`:

```
target_compile_options(_utilities PRIVATE -g -O0)
```

Finally, run using `gdb`. For example:

```
$ gdb --args python test_ext.py
(gdb) b qual.cpp:4872
Make breakpoint pending on future shared library load? (y or [n]) y
Breakpoint 1 (qual.cpp:4872) pending.
(gdb) run
Thread 1 "python" hit Breakpoint 1, ComputeWeights<float> (offset=0x1fe5830, neigh=0x20432e0,
indices=0x1bb8ec0, points=0x1d6a7a0, n_neigh=108, n_points=27, fac=-0.75, num_threads=4) at /home/user/library-path/src/qual.cpp:4872
4872 T *weights = new T[n_neigh];
(gdb)
```

#### Debugging memory leaks

These can be challenging to find. Use [valgrind](https://valgrind.org/) with the following to identify memory leaks. Be sure to download [valgrind-python.supp](https://github.com/python/cpython/blob/main/Misc/valgrind-python.supp).

```
valgrind --leak-check=full --log-file=val.txt --suppressions=valgrind-python.supp pytest -k clus && grep 'new\[\]' val.txt
```
5 changes: 0 additions & 5 deletions MANIFEST.in

This file was deleted.

Loading

0 comments on commit 946f99e

Please sign in to comment.