Merge pull request #695 from bccp/fixes

This PR fixes the build for recent numpy and scipy versions, fixes compatibility with the new classylss, fixes a variety of bugs found by running a linter, and fixes the github action to successfully run the tests.

Author: sbird

.github/workflows/main.yaml

@@ -1,11 +1,8 @@
 # main test workflow; ported from .travis.yaml
 
-
 name: main
 
 on:
-  schedule:
-    - cron:  '0 0 * * 5'   # Runs on every Friday.
   push:
     branches: [ '*', $default-branch ]
     tags: ['[0-9]*']   # anything looks like a version.
@@ -17,79 +14,64 @@ jobs:
     runs-on: ${{ matrix.os }}
     env:
       OMP_NUM_THREADS: 1
-
     defaults:
       run:
         shell: bash -l {0}   # for conda.
-
     strategy:
-      fail-fast: false
       matrix:
-        os: [ ubuntu-latest, macos-latest ]
-        numpy-version: [ '1.20' ]
-        python-version: [ 3.8 ]
-        include:
-          - {deploy: true, os: ubuntu-latest, numpy-version: '1.20', python-version: 3.8 }
+        os: [ubuntu-latest]
+        python-version: [ '3.8', '3.10', '3.13' ]
     steps:
-
     - name: Checkout source code
-      uses: actions/checkout@v2
-
-    - name: Cache conda
-      uses: actions/cache@v1
-      env:
-        # Increase this value to reset cache.
-        CACHE_NUMBER: 0
-      with:
-        path: ~/conda_pkgs_dir
-        key:
-          ${{ runner.os }}-conda-${{ env.CACHE_NUMBER }}
-
-    - name: Setup Conda Environment
-      uses: conda-incubator/[email protected]
-      with:
-        activate-environment: test
-        channels: bccp
-        show-channel-urls: true
-        use-only-tar-bz2: false
-        auto-update-conda: true
-        python-version: ${{ matrix.python-version }}
-
+      uses: actions/checkout@v4
     - name: Setup test env
       run: |
-        conda install -q -y \
-               python=${{ matrix.python-version }} \
-               numpy=${{ matrix.numpy-version }} \
-               nose cython mpi4py \
-               --file requirements.txt \
-               --file requirements-extras.txt
-
-        # submitting coverage to coveralls
-        pip install coveralls
-        # install nbodykit itself
-        pip install .[extras]
-        conda list --explicit
-
-    - name: Install Compilers
+        sudo apt update
+        sudo apt install -y build-essential libopenmpi-dev openmpi-bin python3-numpy python3-scipy python3-mpi4py python3-sympy python3-astropy cython3 python3-dask python3-pandas python3-h5py libgsl-dev
+        python -m pip install --upgrade pip
+        pip install build pytest pytest-mpi
+        pip install -r requirements.txt
+        pip install -r requirements-extras.txt
+    - name: Build
       run: |
-        if [ "$RUNNER_OS" == "macOS" ]; then
-          conda install -q -y clang_osx-64 clangxx_osx-64 gfortran_osx-64
-        else
-          conda install -q -y gcc_linux-64 gxx_linux-64 gfortran_linux-64
-        fi
-
+        python -m build
+    - name: Install
+      run: |
+        python -m pip install --user dist/nbodykit*whl
+    - name: Build in place for tests
+      run: |
+        python setup.py build_ext --inplace
     - name: Python Unit tests
       run: |
-        python run-tests.py -v --mpirun='mpirun -n 4' --with-coverage
-
-    - name: Build Python sdist
-      if: startsWith(github.ref, 'refs/tags') && matrix.deploy
+        cd nbodykit
+        python -m pytest --with-mpi
+    - name: MPI Unit tests
       run: |
-        bash check_tag.sh "${GITHUB_REF##*/}" bigfile/version.py
-        python setup.py sdist
+        cd nbodykit
+        mpirun -n 4 --oversubscribe python -m pytest --with-mpi
+    - name: Store the distribution packages
+      if: matrix.os == 'ubuntu-latest' && matrix.python-version == '3.10' #Only do this once!
+      uses: actions/upload-artifact@v4
+      with:
+        name: python-package-distributions
+        path: dist/nbodykit*.tar.gz
 
-    - name: Publish distribution to PyPI
-      uses: pypa/gh-action-pypi-publish@master
-      if: startsWith(github.ref, 'refs/tags') && matrix.deploy
+  publish-to-pypi:
+    name: Publish to PyPI
+    if: startsWith(github.ref, 'refs/tags/')  # only publish to PyPI on tag pushes
+    needs:
+    - build
+    runs-on: ubuntu-latest
+    environment:
+      name: pypi
+      url: https://pypi.org/p/nbodykit
+    permissions:
+      id-token: write  # IMPORTANT: mandatory for trusted publishing
+    steps:
+    - name: Download the source dist
+      uses: actions/download-artifact@v4
       with:
-        password: ${{ secrets.PYPI_SECRET }}
+        name: python-package-distributions
+        path: dist/
+    - name: Publish distribution to PyPI
+      uses: pypa/gh-action-pypi-publish@release/v1

docs/environment.yml

@@ -11,12 +11,11 @@ channels:
 - astropy
 
 dependencies:
-- python=3.6
+- python
 - numpy
 - scipy
 - astropy
 - mpi4py
-- runtests
 - pmesh
 - kdcount
 - mpsort

nbodykit/__init__.py

@@ -2,15 +2,19 @@
 
 from mpi4py import MPI
 
+from contextlib import contextmanager
+import logging
+
 import dask
+import dask.cache
 
 import warnings
 
 
 try:
     # prevents too many threads exception when using MPI and dask
     # by disabling threading in dask.
-    dask.config.set(scheduler='synchronous') 
+    dask.config.set(scheduler='synchronous')
 except:
     # deprecated since 0.18.1
     dask.set_options(get=dask.get)
@@ -20,9 +24,6 @@
 _global_options['dask_chunk_size'] = 100000
 _global_options['paint_chunk_size'] = 1024 * 1024 * 4
 
-from contextlib import contextmanager
-import logging
-
 def _unpickle(name):
     return getattr(MPI, name)
 
@@ -97,7 +98,7 @@ def use_mpi(comm=None):
         This function shall only be used before any nbodykit object is created.
 
     """
-    dask.config.set(scheduler='synchronous') 
+    dask.config.set(scheduler='synchronous')
     if comm is None:
         comm = MPI.COMM_WORLD
     CurrentMPIComm._stack[-1] = comm
@@ -139,7 +140,7 @@ def enter(cls, comm):
             with CurrentMPIComm.enter(comm):
                 cat = UniformCatalog(...)
 
-        is identical to 
+        is identical to
 
         .. code:: python
 
@@ -189,7 +190,6 @@ def set(cls, comm):
         cls._stack[-1] = comm
         cls._stack[-1].barrier()
 
-import dask.cache
 class GlobalCache(dask.cache.Cache):
     """
         A Cache object.
@@ -270,7 +270,6 @@ def setup_logging(log_level="info"):
     #
     # [ 000000.43 ]   0: 06-28 14:49  measurestats    INFO     Nproc = [2, 1, 1]
     # [ 000000.43 ]   0: 06-28 14:49  measurestats    INFO     Rmax = 120
-    import logging
 
     levels = {
             "info" : logging.INFO,
@@ -279,7 +278,7 @@ def setup_logging(log_level="info"):
             }
 
     import time
-    logger = logging.getLogger();
+    logger = logging.getLogger()
     t0 = time.time()
 
     rank = MPI.COMM_WORLD.rank

nbodykit/algorithms/convpower/tests/test_catalog.py

@@ -1,13 +1,14 @@
-from runtests.mpi import MPITest
 from nbodykit.lab import *
 from nbodykit import setup_logging
 
 from numpy.testing import assert_allclose
 import pytest
+from mpi4py import MPI
 
 setup_logging()
 
-@MPITest([1])
+@pytest.mark.parametrize("comm", [MPI.COMM_WORLD,])
+@pytest.mark.mpi
 def test_missing_columns(comm):
 
     # create FKP catalog
@@ -17,7 +18,8 @@ def test_missing_columns(comm):
     with pytest.raises(ValueError):
         cat = FKPCatalog(source1, source2, BoxSize=512.0, BoxPad=0.02)
 
-@MPITest([1])
+@pytest.mark.parametrize("comm", [MPI.COMM_WORLD,])
+@pytest.mark.mpi
 def test_boxsize(comm):
 
     # data and randoms

nbodykit/algorithms/convpower/tests/test_catalogmesh.py

@@ -1,4 +1,3 @@
-from runtests.mpi import MPITest
 from nbodykit.lab import *
 from nbodykit import setup_logging
 
@@ -7,7 +6,12 @@
 
 setup_logging()
 
-@MPITest([1, 4])
+from mpi4py import MPI
+
+setup_logging()
+
+@pytest.mark.parametrize("comm", [MPI.COMM_WORLD,])
+@pytest.mark.mpi
 def test_paint(comm):
 
     NBAR1 = 3e-5; WEIGHT1 = 1.05

nbodykit/algorithms/fftpower.py

@@ -7,6 +7,7 @@
 from nbodykit.meshtools import SlabIterator
 from nbodykit.base.catalog import CatalogSourceBase
 from nbodykit.base.mesh import MeshSource
+from mpi4py import MPI
 
 class FFTBase(object):
     """
@@ -143,7 +144,7 @@ def _compute_3d_power(self, first, second):
 
 
 class FFTPower(FFTBase):
-    """
+    r"""
     Algorithm to compute the 1d or 2d power spectrum and/or multipoles
     in a periodic box, using a Fast Fourier Transform (FFT).
 
@@ -227,10 +228,10 @@ def __init__(self, first, mode, Nmesh=None, BoxSize=None, second=None,
         self.attrs.update(self.power.attrs)
 
     def run(self):
-        """
+        r"""
         Compute the power spectrum in a periodic box, using FFTs.
 
-        Returns 
+        Returns
         -------
         power : :class:`~nbodykit.binned_statistic.BinnedStatistic`
             a BinnedStatistic object that holds the measured :math:`P(k)` or
@@ -735,24 +736,27 @@ def _find_unique_edges(x, x0, xmax, comm):
 
         Returns edges and the true centers
     """
-    def find_unique_local(x, x0):
-        fx2 = 0
-        for xi, x0i in zip(x, x0):
-            fx2 = fx2 + xi ** 2
+    fx2 = 0
+    for xi in x:
+        fx2 = fx2 + xi ** 2
 
+    def find_unique_local(fx2, binning):
+        """Find unique values in a floating point array by making integer bins"""
         fx2 = numpy.ravel(fx2)
-        ix2 = numpy.int64(fx2 / (x0.min() * 0.5) ** 2 + 0.5)
+        ix2 = numpy.int64(fx2 / binning + 0.5)
         ix2, ind = numpy.unique(ix2, return_index=True)
         fx2 = fx2[ind]
-        return fx2 ** 0.5
+        return fx2
 
-    fx = find_unique_local(x, x0)
+    binning = (x0.min() * 0.05)**2
+    fx = find_unique_local(fx2, binning)**0.5
 
     fx = fx[fx < xmax]
     fx = numpy.concatenate(comm.allgather(fx), axis=0)
-    # may have duplicates after allgather
-    fx = numpy.unique(fx)
-    fx.sort()
+    # May have duplicates after allgather: need to re-bin.
+    # We want to be picky about duplicates, so use a small bin size
+    minx0 = comm.allreduce(x0.min(), op=MPI.MIN)
+    fx = find_unique_local(fx, minx0 * 1e-5)
 
     # now make some reasonable bins.
     width = numpy.diff(fx)

nbodykit/algorithms/fof.py

@@ -230,8 +230,8 @@ def _assign_labels(minid, comm, thresh):
     # assign origind for recovery of ordering, since
     # we need to work in sorted fofid
     data['fofid'] = minid
-    data['origind'] = numpy.arange(len(data), dtype='u4')
-    data['origind'] += numpy.sum(comm.allgather(len(data))[:comm.rank], dtype='intp') \
+    data['origind'] = numpy.arange(len(data), dtype='u8')
+    data['origind'] += numpy.sum(comm.allgather(int(len(data)))[:comm.rank], dtype='u8')
 
     data = DistributedArray(data, comm)
 

nbodykit/algorithms/kdtree.py

@@ -4,7 +4,7 @@
 
 from nbodykit.utils import split_size_3d
 from pmesh.domain import GridND
-from scipy.spatial.ckdtree import cKDTree as KDTree
+from scipy.spatial import cKDTree as KDTree
 
 class KDDensity(object):
     """