Merge pull request #5 from Hekstra-Lab/mac-build

add a real build step to tests + test on mac + win

.github/workflows/deploy.yml

@@ -1,4 +1,4 @@
-# copied nearly verbatim from
+# Based on
 # https://github.com/tlambert03/nd2/blob/174a2b75426631b54aad8c2440b6a5da5d4c5226/.github/workflows/deploy.yml
 name: Build & deploy
 
@@ -18,20 +18,23 @@ jobs:
     steps:
       - uses: actions/checkout@v2
 
-      - uses: actions/setup-python@v2
-        name: Install Python
-        with:
-          python-version: '3.8'
-
-      - name: Install dependencies
+      - name: macos clang things
+        if: matrix.os == 'macos-10.15'
         run: |
-          python -m pip install --upgrade pip
-          pip install build
+          brew install llvm libomp
 
-      - name: Build wheels
+        # https://stackoverflow.com/a/39843038/835607
+        # https://stackoverflow.com/a/60564952/835607
+      - name: build - Mac
+        if: matrix.os == 'macos-10.15'
         env:
-          CC: gcc-9
-          CXX: g++-9
+          CC: /usr/local/opt/llvm/bin/clang
+          CXX: /usr/local/opt/llvm/bin/clang
+          LDFLAGS: -L/usr/local/opt/llvm/lib
+        uses: pypa/cibuildwheel@v2.2.0a1
+
+      - name: build - not mac
+        if: matrix.os != 'macos-10.15'
         uses: pypa/cibuildwheel@v2.2.0a1
 
       - uses: actions/upload-artifact@v2

.github/workflows/test_build.yml

@@ -10,12 +10,15 @@ on:
     branches: [main]
 
 jobs:
-  test-3x:
-    name: Python ${{ matrix.python-version }}
-    runs-on: ubuntu-latest
+  build:
+    name: Build on ${{ matrix.os}}
+    runs-on: ${{ matrix.os }}
     strategy:
+      fail-fast: false
       matrix:
-        python-version: ["3.9.x"]
+        python-version: ["3.8.x"]
+        os: [ubuntu-20.04, windows-2019, macos-10.15]
+        # os: [macos-10.15]
     steps:
       - name: Checkout
         uses: actions/checkout@v2
@@ -26,10 +29,66 @@ jobs:
           python-version: ${{ matrix.python-version }}
           architecture: "x64"
 
-      - name: Install
+      - name: macos clang things
+        if: matrix.os == 'macos-10.15'
         run: |
-          pip install cython numpy numba scikit-image
-          python setup.py build_ext -i
+          brew install llvm libomp
+
+      - name: install stuff
+        run: pip install scikit-image numpy build
+
+        # https://stackoverflow.com/a/39843038/835607
+        # https://stackoverflow.com/a/60564952/835607
+      - name: build - Mac
+        if: matrix.os == 'macos-10.15'
+        env:
+          CC: /usr/local/opt/llvm/bin/clang
+          CXX: /usr/local/opt/llvm/bin/clang
+          # CPPFLAGS: -I/usr/local/opt/llvm/include -fopenmp
+          LDFLAGS: -L/usr/local/opt/llvm/lib
+        run: |
+          python -m build .
+
+      - name: build - not mac
+        if: matrix.os != 'macos-10.15'
+        run: |
+          python -m build .
+
+      - uses: actions/upload-artifact@v2
+        with:
+          name: ${{matrix.os}}-${{matrix.python-version}}-wheel
+          path: dist/*.whl
+
+  test:
+    name: Test on ${{ matrix.os}}
+    needs: build
+    runs-on: ${{ matrix.os }}
+    strategy:
+      fail-fast: false
+      matrix:
+        python-version: ["3.8.x"]
+        os: [ubuntu-20.04, macos-10.15]
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v2
+
+      - name: Setup Python ${{ matrix.python-version }}
+        uses: actions/setup-python@v2
+        with:
+          python-version: ${{ matrix.python-version }}
+          architecture: "x64"
+
+      - name: install stuff
+        run: pip install scikit-image numpy
+
+      - name: Download Docker Image (Artifact)
+        uses: actions/download-artifact@v2
+        with:
+          name: ${{matrix.os}}-${{matrix.python-version}}-wheel
+
+      - name: install
+        run: |
+          pip install *.whl
 
       - name: Tests
-        run: python test.py
+        run: pytest --color=yes

README.md

@@ -21,3 +21,7 @@ To really remove stuff and build + test:
 ```
 rm *.so build/ fast_overlap.cpp -rf && python setup.py build_ext -i && python test_speedup.py
 ```
+
+
+### On Mac
+You need to compile python extensions with the same compiler used to compile python. So on mac you should use `clang`. However the apple distributed clang doesn't include openmp so you should either use g++ locally (which seems to work for some reason, but doesn't for built wheels) or use homebrew clang as in the github workflows.

examples/test.py

@@ -0,0 +1,20 @@
+import numpy as np
+from skimage.draw import random_shapes
+from skimage.segmentation import relabel_sequential
+
+from fast_overlap import overlap, overlap_parallel
+
+im_shape = (1024, 1024)
+min_shapes = 5
+im1 = random_shapes(im_shape, 20, min_shapes=min_shapes, random_seed=0)[0]
+im1 = relabel_sequential(im1.sum(axis=-1))[0]
+im2 = random_shapes(im_shape, 20, min_shapes=min_shapes, random_seed=1995)[0]
+im2 = relabel_sequential(im2.sum(axis=-1))[0]
+shape = (int(np.max(im1) + 1), int(np.max(im2) + 1))
+out = overlap_parallel(im1.astype(np.int32), im2.astype(np.int32), shape)
+print(out.sum())
+out_serial = overlap(im1.astype(np.int32), im2.astype(np.int32), shape)
+
+# from IPython import embed
+# embed(colors="Linux")
+assert np.all(out == out_serial)

examples/test_version.py

@@ -0,0 +1,3 @@
+import fast_overlap
+
+print(fast_overlap.__version__)

fast_overlap/fast_overlap.pyx

@@ -25,6 +25,7 @@ cpdef overlap_parallel(int [:,::1] prev, int[:,::1] curr, shape):
     cdef np.ndarray[int, ndim=2, mode="c"] output = np.zeros(shape, dtype=np.dtype("i"))
     cdef Py_ssize_t ncols = shape[1]
 
+    print('about to nogil')
     with nogil:
         overlap_parallel_cpp(&prev[0,0], &curr[0,0], prev.shape, &output[0,0], ncols)
     return output

fast_overlap/parallel_overlap.cpp

@@ -4,14 +4,22 @@
 
 void overlap_parallel_cpp(int *prev, int *curr, Py_ssize_t shape[2],
                           int *output, Py_ssize_t output_cols) {
+#ifdef _WIN32
+#pragma omp parallel for
+#else
 #pragma omp parallel for collapse(2)
+#endif
   for (int i = 0; i < shape[0]; i++) {
     for (int j = 0; j < shape[1]; j++) {
       int prev_id = prev[i * shape[1] + j];
       int curr_id = curr[i * shape[1] + j];
       int idx = prev_id * output_cols + curr_id;
       if (prev_id && curr_id) {
+#ifdef _WIN32
+#pragma omp atomic
+#else
 #pragma omp atomic update
+#endif
         output[idx] += 1;
       }
     }

setup.cfg

@@ -16,8 +16,7 @@ python_requires = >=3.7,<3.10
 
 [options.extras_require]
 test =
-    numba
-    scikit-image
+    pytest
 
 [flake8]
 exclude = docs, _version.py, .eggs, example

setup.py

@@ -1,7 +1,22 @@
+import os
+
 import numpy
 from Cython.Build import cythonize
 from setuptools import Extension, setup
 
+# os.environ["CC"] = "C:/Program Files/LLCM/bin/clang-cl.exe"
+# os.environ["CXX"] = "C:/Program Files/LLCM/bin/clang-cl.exe"
+if os.name == "nt":
+    compile_args = ["/fopenmp", "/Ox"]
+    extra_link_args = []
+else:
+    compile_args = [
+        "-fopenmp",
+        "-O3",
+    ]
+    extra_link_args = ["-fopenmp"]
+# if os.name == "darwin":
+#     compile_args.append("-lomp")
 setup(
     name="fast_overlap",
     use_scm_version={"write_to": "fast_overlap/_version.py"},
@@ -11,8 +26,8 @@
                 "fast_overlap._engine",
                 ["fast_overlap/fast_overlap.pyx"],
                 include_dirs=[numpy.get_include()],
-                extra_compile_args=["-fopenmp", "-O3"],
-                extra_link_args=["-fopenmp"],
+                extra_compile_args=compile_args,
+                extra_link_args=extra_link_args,
             )
         ],
         language_level="3",

test.py

@@ -11,10 +11,13 @@
 im2 = random_shapes(im_shape, 20, min_shapes=min_shapes, random_seed=1995)[0]
 im2 = relabel_sequential(im2.sum(axis=-1))[0]
 shape = (int(np.max(im1) + 1), int(np.max(im2) + 1))
-out = overlap_parallel(im1.astype(np.int32), im2.astype(np.int32), shape)
 # print(out.sum())
+print("overlap")
 out_serial = overlap(im1.astype(np.int32), im2.astype(np.int32), shape)
 
+print("overlap parallel")
+out = overlap_parallel(im1.astype(np.int32), im2.astype(np.int32), shape)
+
 # from IPython import embed
 # embed(colors="Linux")
 assert np.all(out == out_serial)

tests/test_overlap.py

@@ -0,0 +1,21 @@
+from pathlib import Path
+
+import numpy as np
+
+import fast_overlap
+
+ims = np.load(str(Path(__file__).parent / "test-ims.npy"))
+expected = np.load(str(Path(__file__).parent / "expected-overlap.npy"))
+shape = (int(np.max(ims[0]) + 1), int(np.max(ims[1]) + 1))
+
+
+def test_overlap():
+    out = fast_overlap.overlap(ims[0].astype(np.int32), ims[1].astype(np.int32), shape)
+    assert np.all(out == expected)
+
+
+def test_parallel_overlap():
+    out = fast_overlap.overlap_parallel(
+        ims[0].astype(np.int32), ims[1].astype(np.int32), shape
+    )
+    assert np.all(out == expected)