Skip to content

Commit

Permalink
Remove legacy Cython features
Browse files Browse the repository at this point in the history
Several Cython features have been deprecated in Cython 3.0 and will
generate errors in future Cython releases. This change removes .pxi
include files, all `IF` conditional statements, and cpdef functions.
Almost all Cython 3.0 deprecation warnings were addressed, a silent
API change from Python 3.11 was mitigated, and some of the custom
Cython code for type checking was replaced by equivalent C++ code.
  • Loading branch information
jngrad committed Jan 10, 2024
1 parent 9550aa1 commit d5b68bc
Show file tree
Hide file tree
Showing 23 changed files with 139 additions and 267 deletions.
2 changes: 1 addition & 1 deletion .github/actions/build_and_check/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ runs:
- run: |
brew install boost boost-mpi fftw
brew install hdf5-mpi
pip3 install -c requirements.txt numpy cython h5py scipy
pip3 install -c requirements.txt numpy "cython<3.0" h5py scipy
shell: bash
if: runner.os == 'macOS'
- run: |
Expand Down
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ endif()
# Python interpreter and Cython interface library
if(ESPRESSO_BUILD_WITH_PYTHON)
find_package(Python 3.9 REQUIRED COMPONENTS Interpreter Development NumPy)
find_package(Cython 0.29.21...<3.0 REQUIRED)
find_package(Cython 0.29.21...<3.0.8 REQUIRED)
find_program(IPYTHON_EXECUTABLE NAMES jupyter ipython3 ipython)
endif()

Expand Down
2 changes: 1 addition & 1 deletion maintainer/CI/build_cmake.sh
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ if [ "${with_coverage}" = true ] || [ "${with_coverage_python}" = true ]; then
lcov --gcov-tool "${GCOV:-gcov}" -q --remove coverage.info '/usr/*' --output-file coverage.info # filter out system
lcov --gcov-tool "${GCOV:-gcov}" -q --remove coverage.info '*/doc/*' --output-file coverage.info # filter out docs
if [ -d _deps/ ]; then
lcov --gcov-tool "${GCOV:-gcov}" -q --remove coverage.info $(realpath _deps/)'/*' --output-file coverage.info # filter out docs
lcov --gcov-tool "${GCOV:-gcov}" -q --remove coverage.info $(realpath _deps/)'/*' --output-file coverage.info # filter out external projects
fi
fi
if [ "${with_coverage_python}" = true ]; then
Expand Down
2 changes: 1 addition & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# build system
cython>=0.29.21,<3.0
cython>=0.29.21,<=3.0.7
setuptools>=59.6.0
# required scientific packages
numpy>=1.23
Expand Down
17 changes: 10 additions & 7 deletions src/core/electrostatics/icc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@
#include <boost/mpi/operations.hpp>

#include <algorithm>
#include <cassert>
#include <cmath>
#include <limits>
#include <stdexcept>
Expand Down Expand Up @@ -212,6 +211,8 @@ void ICCStar::iteration(CellStructure &cell_structure,
}

void icc_data::sanity_checks() const {
if (n_icc <= 0)
throw std::domain_error("Parameter 'n_icc' must be >= 1");
if (convergence <= 0.)
throw std::domain_error("Parameter 'convergence' must be > 0");
if (relaxation < 0. or relaxation > 2.)
Expand All @@ -222,12 +223,14 @@ void icc_data::sanity_checks() const {
throw std::domain_error("Parameter 'first_id' must be >= 0");
if (eps_out <= 0.)
throw std::domain_error("Parameter 'eps_out' must be > 0");

assert(n_icc >= 1);
assert(areas.size() == n_icc);
assert(epsilons.size() == n_icc);
assert(sigmas.size() == n_icc);
assert(normals.size() == n_icc);
if (areas.size() != n_icc)
throw std::invalid_argument("Parameter 'areas' has incorrect shape");
if (epsilons.size() != n_icc)
throw std::invalid_argument("Parameter 'epsilons' has incorrect shape");
if (sigmas.size() != n_icc)
throw std::invalid_argument("Parameter 'sigmas' has incorrect shape");
if (normals.size() != n_icc)
throw std::invalid_argument("Parameter 'normals' has incorrect shape");
}

ICCStar::ICCStar(icc_data data) {
Expand Down
21 changes: 1 addition & 20 deletions src/python/espressomd/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -17,24 +17,6 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
#

add_custom_command(
OUTPUT gen_pxiconfig.cpp
COMMAND
${Python_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/gen_pxiconfig.py
${CMAKE_SOURCE_DIR}/src/config/features.def
${CMAKE_CURRENT_BINARY_DIR}/gen_pxiconfig.cpp
DEPENDS ${CMAKE_SOURCE_DIR}/src/config/features.def)

add_executable(gen_pxiconfig gen_pxiconfig.cpp)
target_link_libraries(gen_pxiconfig espresso::config)
set_target_properties(gen_pxiconfig PROPERTIES CXX_CLANG_TIDY
"${ESPRESSO_CXX_CLANG_TIDY}")

add_custom_command(
OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/myconfig.pxi
COMMAND ${CMAKE_CURRENT_BINARY_DIR}/gen_pxiconfig >
${CMAKE_CURRENT_BINARY_DIR}/myconfig.pxi DEPENDS gen_pxiconfig)

add_custom_target(espressomd)

# Make the cython_SRC, cython_HEADER and cython_AUX a cached variable to be able
Expand Down Expand Up @@ -93,8 +75,7 @@ foreach(cython_file ${cython_SRC})
${CMAKE_CURRENT_SOURCE_DIR} -I ${CMAKE_CURRENT_BINARY_DIR}
${cython_file} -o ${outputpath}
WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/..
DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/myconfig.pxi ${cython_file}
${cython_HEADER})
DEPENDS ${cython_file} ${cython_HEADER})
set(target "espressomd_${basename}")
add_library(${target} SHARED ${outputpath})
if(NOT "${relpath}" STREQUAL "")
Expand Down
43 changes: 0 additions & 43 deletions src/python/espressomd/electrostatic_extensions.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,8 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
#

from . import utils
from .script_interface import ScriptInterfaceHelper, script_interface_register
from .code_features import has_features
import numpy as np


class ElectrostaticExtensions(ScriptInterfaceHelper):
Expand All @@ -33,7 +31,6 @@ def __init__(self, **kwargs):
if 'sip' not in kwargs:
params = self.default_params()
params.update(kwargs)
self.validate_params(params)
super().__init__(**params)
else:
super().__init__(**kwargs)
Expand All @@ -42,9 +39,6 @@ def _check_required_features(self):
if not has_features("ELECTROSTATICS"):
raise NotImplementedError("Feature ELECTROSTATICS not compiled in")

def validate_params(self, params):
raise NotImplementedError("Derived classes must implement this method")

def default_params(self):
raise NotImplementedError("Derived classes must implement this method")

Expand Down Expand Up @@ -93,43 +87,6 @@ class ICC(ElectrostaticExtensions):
_so_name = "Coulomb::ICCStar"
_so_creation_policy = "GLOBAL"

def validate_params(self, params):
utils.check_type_or_throw_except(
params["n_icc"], 1, int, "Invalid parameter 'n_icc'")
utils.check_type_or_throw_except(
params["first_id"], 1, int, "Invalid parameter 'first_id'")
utils.check_type_or_throw_except(
params["convergence"], 1, float, "Invalid parameter 'convergence'")
utils.check_type_or_throw_except(
params["relaxation"], 1, float, "Invalid parameter 'relaxation'")
utils.check_type_or_throw_except(
params["ext_field"], 3, float, "Invalid parameter 'ext_field'")
utils.check_type_or_throw_except(
params["max_iterations"], 1, int, "Invalid parameter 'max_iterations'")
utils.check_type_or_throw_except(
params["eps_out"], 1, float, "Invalid parameter 'eps_out'")

n_icc = params["n_icc"]
if n_icc <= 0:
raise ValueError("Parameter 'n_icc' must be >= 1")

if n_icc:
if np.shape(params["normals"]) != (n_icc, 3):
raise ValueError("Parameter 'normals' has incorrect shape")
utils.check_array_type_or_throw_except(
np.reshape(params["normals"], (-1,)), 3 * n_icc, float,
"Parameter 'normals' has incorrect type")

if "sigmas" not in params:
params["sigmas"] = np.zeros(n_icc)

for key in ("areas", "sigmas", "epsilons"):
if np.shape(params[key]) != (n_icc,):
raise ValueError(f"Parameter '{key}' has incorrect shape")
utils.check_array_type_or_throw_except(
np.reshape(params[key], (-1,)), n_icc, float,
f"Parameter '{key}' has incorrect type")

def valid_keys(self):
return {"n_icc", "convergence", "relaxation", "ext_field",
"max_iterations", "first_id", "eps_out", "normals",
Expand Down
69 changes: 0 additions & 69 deletions src/python/espressomd/gen_pxiconfig.py

This file was deleted.

3 changes: 1 addition & 2 deletions src/python/espressomd/particle_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import functools
from .interactions import BondedInteraction
from .interactions import BondedInteractions
from .utils import nesting_level, array_locked, is_valid_type, handle_errors
from .utils import nesting_level, array_locked, is_valid_type
from .utils import check_type_or_throw_except
from .code_features import assert_features, has_features
from .script_interface import script_interface_register, ScriptInterfaceHelper
Expand Down Expand Up @@ -598,7 +598,6 @@ def vs_auto_relate_to(self, rel_to, override_cutoff_check=False,
"vs_relate_to",
pid=rel_to,
override_cutoff_check=override_cutoff_check)
handle_errors("vs_auto_relate_to")
if self.propagation != Propagation.NONE:
if couple_to_lb:
self.propagation |= Propagation.TRANS_LB_MOMENTUM_EXCHANGE
Expand Down
8 changes: 4 additions & 4 deletions src/python/espressomd/script_interface.pxd
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ from libcpp.unordered_map cimport unordered_map
from libcpp.string cimport string
from libcpp.memory cimport shared_ptr
from libcpp.vector cimport vector
from libcpp cimport bool
from libcpp cimport bool as cbool

from boost cimport string_ref

Expand All @@ -37,8 +37,8 @@ cdef extern from "script_interface/ScriptInterface.hpp" namespace "ScriptInterfa
Variant(const Variant & )
Variant & operator = (const Variant &)

bool is_type[T](const Variant &)
bool is_none(const Variant &)
cbool is_type[T](const Variant &)
cbool is_none(const Variant &)
ctypedef unordered_map[string, Variant] VariantMap

Variant make_variant[T](const T & x)
Expand Down Expand Up @@ -71,7 +71,7 @@ cdef extern from "script_interface/initialize.hpp" namespace "ScriptInterface":
void initialize(Factory[ObjectHandle] *)

cdef extern from "script_interface/get_value.hpp" namespace "ScriptInterface":
T get_value[T](const Variant T)
T get_value[T](const Variant T) except +

cdef extern from "script_interface/code_info/CodeInfo.hpp" namespace "ScriptInterface::CodeInfo":
void check_features(const vector[string] & features) except +
Expand Down
10 changes: 6 additions & 4 deletions src/python/espressomd/script_interface.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ from libcpp.memory cimport shared_ptr, make_shared
from libcpp.vector cimport vector
from libcpp.utility cimport pair
from libcpp.unordered_map cimport unordered_map
from libcpp cimport bool as cbool

cdef shared_ptr[ContextManager] _om

Expand Down Expand Up @@ -192,6 +193,7 @@ cdef class PScriptInterface:
for name, value in kwargs.items():
self.sip.get().set_parameter(utils.to_char_pointer(name),
python_object_to_variant(value))
utils.handle_errors(f"while setting parameter '{name}'")

def get_parameter(self, name):
cdef Variant value = self.sip.get().get_parameter(utils.to_char_pointer(name))
Expand Down Expand Up @@ -299,7 +301,7 @@ cdef Variant python_object_to_variant(value) except *:
vec_variant.push_back(python_object_to_variant(e))
return make_variant[vector[Variant]](vec_variant)
elif isinstance(value, (type(True), np.bool_)):
return make_variant[bool](value)
return make_variant[cbool](value)
elif np.issubdtype(np.dtype(type(value)), np.signedinteger):
return make_variant[int](value)
elif np.issubdtype(np.dtype(type(value)), np.floating):
Expand All @@ -308,7 +310,7 @@ cdef Variant python_object_to_variant(value) except *:
raise TypeError(
f"No conversion from type '{type(value).__name__}' to 'Variant'")

cdef variant_to_python_object(const Variant & value) except +:
cdef variant_to_python_object(const Variant & value):
"""Convert C++ Variant objects to Python objects."""

cdef vector[Variant] vec
Expand All @@ -324,8 +326,8 @@ cdef variant_to_python_object(const Variant & value) except +:
cdef Vector4d vec4d
if is_none(value):
return None
if is_type[bool](value):
return get_value[bool](value)
if is_type[cbool](value):
return get_value[cbool](value)
if is_type[int](value):
return get_value[int](value)
if is_type[double](value):
Expand Down
16 changes: 0 additions & 16 deletions src/python/espressomd/system.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@
from . import thermostat

from .code_features import has_features, assert_features
from . import utils

from .script_interface import script_interface_register, ScriptInterfaceHelper


Expand Down Expand Up @@ -152,16 +150,6 @@ class System(ScriptInterfaceHelper):
_so_creation_policy = "GLOBAL"
_so_bind_methods = _System._so_bind_methods

def __setattr__(self, attr, value):
if attr == "periodicity":
utils.check_type_or_throw_except(
value, 3, bool, "Attribute 'periodicity' must be a list of 3 bools")
if attr == "box_l":
utils.check_type_or_throw_except(
value, 3, float, "Attribute 'box_l' must be a list of 3 floats")
super().__setattr__(attr, value)
utils.handle_errors(f"while assigning system parameter '{attr}'")

def __init__(self, **kwargs):
if "sip" in kwargs:
super().__init__(**kwargs)
Expand Down Expand Up @@ -428,14 +416,10 @@ def distance_vec(self, p1, p2):
if isinstance(p1, particle_data.ParticleHandle):
pos1 = p1.pos_folded
else:
utils.check_type_or_throw_except(
p1, 3, float, "p1 must be a particle or 3 floats")
pos1 = p1
if isinstance(p2, particle_data.ParticleHandle):
pos2 = p2.pos_folded
else:
utils.check_type_or_throw_except(
p2, 3, float, "p2 must be a particle or 3 floats")
pos2 = p2

return self.call_method("distance_vec", pos1=pos1, pos2=pos2)
Expand Down
Loading

0 comments on commit d5b68bc

Please sign in to comment.