From bb16b7e8267b86147877cf3b37f6ddd4752718bd Mon Sep 17 00:00:00 2001 From: Nick Papior Date: Thu, 21 Dec 2023 12:44:30 +0100 Subject: [PATCH] small clean-ups, and fixed requirements future.annotations requires Cython>=3 Signed-off-by: Nick Papior --- ci/doc.yml | 2 +- pyproject.toml | 5 +- src/sisl/geom/neighs/CMakeLists.txt | 8 +-- src/sisl/geom/neighs/_neigh_operations.py | 62 ++++++++++--------- .../geom/neighs/tests/test_neighfinder.py | 35 ++++------- 5 files changed, 50 insertions(+), 62 deletions(-) diff --git a/ci/doc.yml b/ci/doc.yml index 3cab8b1e59..33e2c30aa8 100644 --- a/ci/doc.yml +++ b/ci/doc.yml @@ -12,7 +12,7 @@ dependencies: - pyproject-metadata - pyparsing>=1.5.7 - numpy>=1.13 - - cython>=0.29.28 + - cython>=3.0.0 - scipy>=1.5.0 - netcdf4 - xarray>=0.10.0 diff --git a/pyproject.toml b/pyproject.toml index ceef61e383..3c555c9387 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -12,7 +12,7 @@ requires = [ "setuptools_scm[toml]>=6.2", "scikit-build-core[pyproject]", - "Cython>=0.29.28", + "Cython>=3", # see https://github.com/scipy/oldest-supported-numpy/ # should fix #310 "oldest-supported-numpy; sys_platform != 'win32'", @@ -49,7 +49,8 @@ dependencies = [ ] authors = [ - {name = "Nick Papior", email = "nickpapior@gmail.com"} + {name = "Nick Papior", email = "nickpapior@gmail.com"}, + {name = "Pol Febrer"} ] maintainers = [{name="sisl developers"}] diff --git a/src/sisl/geom/neighs/CMakeLists.txt b/src/sisl/geom/neighs/CMakeLists.txt index 0c2a94aebf..4e37c6334f 100644 --- a/src/sisl/geom/neighs/CMakeLists.txt +++ b/src/sisl/geom/neighs/CMakeLists.txt @@ -1,11 +1,5 @@ # In this directory we have a set of libraries # We will need to link to the Numpy includes -set_property(DIRECTORY - APPEND - PROPERTY INCLUDE_DIRECTORIES - ${CMAKE_CURRENT_SOURCE_DIR}/.. - ) - foreach(source _neigh_operations) add_cython_library( SOURCE ${source}.py @@ -14,4 +8,4 @@ foreach(source _neigh_operations) ) install(TARGETS ${source} LIBRARY DESTINATION ${SKBUILD_PROJECT_NAME}/geom/neighs) -endforeach() \ No newline at end of file +endforeach() diff --git a/src/sisl/geom/neighs/_neigh_operations.py b/src/sisl/geom/neighs/_neigh_operations.py index 6dac016573..ac65efcb11 100644 --- a/src/sisl/geom/neighs/_neigh_operations.py +++ b/src/sisl/geom/neighs/_neigh_operations.py @@ -38,9 +38,12 @@ def build_table(nbins: cnp.int64_t, bin_indices: cnp.int64_t[:]): """ n_atoms = bin_indices.shape[0] - list_array: cnp.int64_t[:] = np.zeros(n_atoms, dtype=np.int64) - counts: cnp.int64_t[:] = np.zeros(nbins, dtype=np.int64) - heads: cnp.int64_t[:] = np.full(nbins, -1, dtype=np.int64) + list_array_obj = np.zeros(n_atoms, dtype=np.int64) + list_array: cnp.int64_t[:] = list_array_obj + counts_obj = np.zeros(nbins, dtype=np.int64) + counts: cnp.int64_t[:] = counts_obj + heads_obj = np.full(nbins, -1, dtype=np.int64) + heads: cnp.int64_t[:] = heads_obj # Loop through all atoms for at in range(n_atoms): @@ -54,10 +57,10 @@ def build_table(nbins: cnp.int64_t, bin_indices: cnp.int64_t[:]): heads[bin_index] = at # Update the count of this bin (increment it by 1). - counts[bin_index] = counts[bin_index] + 1 + counts[bin_index] += 1 # Return the memory views as numpy arrays - return np.asarray(list_array), np.asarray(heads), np.asarray(counts) + return list_array_obj, heads_obj, counts_obj @cython.boundscheck(False) @@ -135,20 +138,22 @@ def get_pairs( """ N_ind = at_indices.shape[0] - i_pair: cython.int = 0 + i_pair: cython.size_t = 0 ref_xyz: cnp.float64_t[:] = np.zeros(3, dtype=np.float64) neigh_isc: cnp.int64_t[:] = np.zeros(3, dtype=np.int64) - neighs: cnp.int64_t[:, :] = np.zeros((max_npairs, 5), dtype=np.int64) - split_indices: cnp.int64_t[:] = np.zeros(N_ind, dtype=np.int64) + neighs_obj: cnp.ndarray = np.zeros([max_npairs, 5], dtype=np.int64) + neighs: cnp.int64_t[:, :] = neighs_obj + split_indices_obj: cnp.ndarray = np.zeros(N_ind, dtype=np.int64) + split_indices: cnp.int64_t[:] = split_indices_obj for search_index in range(N_ind): at = at_indices[search_index] for j in range(8): # Find the bin index. - bin_index = indices[search_index, j] + bin_index: cnp.int64_t = indices[search_index, j] # Get the first atom index in this bin neigh_at: cnp.int64_t = heads[bin_index] @@ -181,11 +186,10 @@ def get_pairs( # instead of moving potential neighbors to the unit cell # because in this way we reduce the number of operations. for i in range(3): - ref_xyz[i] = ( - ref_xyz[i] - - cell[0, i] * neigh_isc[0] - - cell[1, i] * neigh_isc[1] - - cell[2, i] * neigh_isc[2] + ref_xyz[i] -= ( + cell[0, i] * neigh_isc[0] + + cell[1, i] * neigh_isc[1] + + cell[2, i] * neigh_isc[2] ) # Loop through all atoms that are in this bin. @@ -228,7 +232,7 @@ def get_pairs( # We have finished this search, store the breakpoint. split_indices[search_index] = i_pair - return np.asarray(neighs), np.asarray(split_indices) + return neighs_obj, split_indices_obj @cython.boundscheck(False) @@ -306,12 +310,12 @@ def get_all_unique_pairs( N_ats = list_array.shape[0] - i_pair: cython.int = 0 + i_pair: cython.size_t = 0 ref_xyz: cnp.float64_t[:] = np.zeros(3, dtype=np.float64) neigh_isc: cnp.int64_t[:] = np.zeros(3, dtype=np.int64) - neighs: cnp.int64_t[:, :] = np.zeros((max_npairs, 5), dtype=np.int64) + neighs: cnp.int64_t[:, :] = np.zeros([max_npairs, 5], dtype=np.int64) for at in range(N_ats): if self_interaction: @@ -325,7 +329,7 @@ def get_all_unique_pairs( for j in range(8): # Find the bin index. - bin_index = indices[at, j] + bin_index: cnp.int64_t = indices[at, j] # Get the first atom index in this bin neigh_at: cnp.int64_t = heads[bin_index] @@ -358,11 +362,10 @@ def get_all_unique_pairs( # instead of moving potential neighbors to the unit cell # because in this way we reduce the number of operations. for i in range(3): - ref_xyz[i] = ( - ref_xyz[i] - - cell[0, i] * neigh_isc[0] - - cell[1, i] * neigh_isc[1] - - cell[2, i] * neigh_isc[2] + ref_xyz[i] -= ( + cell[0, i] * neigh_isc[0] + + cell[1, i] * neigh_isc[1] + + cell[2, i] * neigh_isc[2] ) # Loop through all atoms that are in this bin. @@ -483,7 +486,7 @@ def get_close( N_ind = search_xyz.shape[0] - i_pair: cython.int = 0 + i_pair: cython.size_t = 0 ref_xyz: cnp.float64_t[:] = np.zeros(3, dtype=np.float64) neigh_isc: cnp.int64_t[:] = np.zeros(3, dtype=np.int64) @@ -494,7 +497,7 @@ def get_close( for search_index in range(N_ind): for j in range(8): # Find the bin index. - bin_index = indices[search_index, j] + bin_index: cnp.int64_t = indices[search_index, j] # Get the first atom index in this bin neigh_at: cnp.int64_t = heads[bin_index] @@ -527,11 +530,10 @@ def get_close( # instead of moving potential neighbors to the unit cell # because in this way we reduce the number of operations. for i in range(3): - ref_xyz[i] = ( - ref_xyz[i] - - cell[0, i] * neigh_isc[0] - - cell[1, i] * neigh_isc[1] - - cell[2, i] * neigh_isc[2] + ref_xyz[i] -= ( + cell[0, i] * neigh_isc[0] + + cell[1, i] * neigh_isc[1] + + cell[2, i] * neigh_isc[2] ) # Loop through all atoms that are in this bin. diff --git a/src/sisl/geom/neighs/tests/test_neighfinder.py b/src/sisl/geom/neighs/tests/test_neighfinder.py index 5e1bfb656a..bc1cb4935a 100644 --- a/src/sisl/geom/neighs/tests/test_neighfinder.py +++ b/src/sisl/geom/neighs/tests/test_neighfinder.py @@ -1,38 +1,31 @@ +from functools import partial + import numpy as np import pytest from sisl import Geometry from sisl.geom import NeighFinder +pytestmark = [pytest.mark.neighbour] -@pytest.fixture(scope="module", params=[True, False]) -def sphere_overlap(request): - return request.param +tr_fixture = partial(pytest.fixture, scope="module", params=[True, False]) -@pytest.fixture(scope="module", params=[True, False]) -def multiR(request): - return request.param - -@pytest.fixture(scope="module", params=[True, False]) -def self_interaction(request): +def request_param(request): return request.param -@pytest.fixture(scope="module", params=[False, True]) -def post_setup(request): - return request.param - - -@pytest.fixture(scope="module", params=[False, True]) -def pbc(request): - return request.param +sphere_overlap = tr_fixture()(request_param) +multiR = tr_fixture()(request_param) +self_interaction = tr_fixture()(request_param) +post_setup = tr_fixture()(request_param) +pbc = tr_fixture()(request_param) @pytest.fixture(scope="module") def neighfinder(sphere_overlap, multiR): - geom = Geometry([[0, 0, 0], [1.2, 0, 0], [9, 0, 0]], lattice=np.diag([10, 10, 7])) + geom = Geometry([[0, 0, 0], [1.2, 0, 0], [9, 0, 0]], lattice=[10, 10, 7]) R = np.array([1.1, 1.5, 1.2]) if multiR else 1.5 @@ -75,7 +68,7 @@ def expected_neighs(sphere_overlap, multiR, self_interaction, pbc): def test_neighfinder_setup(sphere_overlap, multiR, post_setup): - geom = Geometry([[0, 0, 0], [1, 0, 0]], lattice=np.diag([10, 10, 7])) + geom = Geometry([[0, 0, 0], [1, 0, 0]], lattice=[10, 10, 7]) R = np.array([0.9, 1.5]) if multiR else 1.5 @@ -243,7 +236,7 @@ def test_R_too_big(pbc): """Test the case when R is so big that it needs a bigger bin than the unit cell.""" - geom = Geometry([[0, 0, 0], [1, 0, 0]], lattice=np.diag([2, 10, 10])) + geom = Geometry([[0, 0, 0], [1, 0, 0]], lattice=[2, 10, 10]) neighfinder = NeighFinder(geom, R=1.5) @@ -265,7 +258,5 @@ def test_R_too_big(pbc): if pbc: expected_neighs.insert(0, [0, 1, -1, 0, 0]) - print(neighs, expected_neighs) - assert neighs.shape == (len(expected_neighs), 5) assert np.all(neighs == expected_neighs)