diff --git a/.github/actions/build_and_check/action.yml b/.github/actions/build_and_check/action.yml index 6ba63992839..8b41a8ad119 100644 --- a/.github/actions/build_and_check/action.yml +++ b/.github/actions/build_and_check/action.yml @@ -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: | diff --git a/CMakeLists.txt b/CMakeLists.txt index cbc4d8712fc..7c40cb72a7c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -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() diff --git a/maintainer/CI/build_cmake.sh b/maintainer/CI/build_cmake.sh index 66cc2671f5a..7a95caaae96 100755 --- a/maintainer/CI/build_cmake.sh +++ b/maintainer/CI/build_cmake.sh @@ -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 diff --git a/requirements.txt b/requirements.txt index 039abc76e21..ea77f9d6c09 100644 --- a/requirements.txt +++ b/requirements.txt @@ -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 diff --git a/src/core/electrostatics/icc.cpp b/src/core/electrostatics/icc.cpp index 975e76d4f44..cb99d9d2aa3 100644 --- a/src/core/electrostatics/icc.cpp +++ b/src/core/electrostatics/icc.cpp @@ -50,7 +50,6 @@ #include #include -#include #include #include #include @@ -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.) @@ -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) { diff --git a/src/python/espressomd/CMakeLists.txt b/src/python/espressomd/CMakeLists.txt index f130cd5ae12..4c188ce6a14 100644 --- a/src/python/espressomd/CMakeLists.txt +++ b/src/python/espressomd/CMakeLists.txt @@ -17,24 +17,6 @@ # along with this program. If not, see . # -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 @@ -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 "") diff --git a/src/python/espressomd/electrostatic_extensions.py b/src/python/espressomd/electrostatic_extensions.py index 18f5f1cd2c8..5133611e908 100644 --- a/src/python/espressomd/electrostatic_extensions.py +++ b/src/python/espressomd/electrostatic_extensions.py @@ -17,10 +17,8 @@ # along with this program. If not, see . # -from . import utils from .script_interface import ScriptInterfaceHelper, script_interface_register from .code_features import has_features -import numpy as np class ElectrostaticExtensions(ScriptInterfaceHelper): @@ -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) @@ -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") @@ -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", diff --git a/src/python/espressomd/gen_pxiconfig.py b/src/python/espressomd/gen_pxiconfig.py deleted file mode 100644 index eeb72e9e26f..00000000000 --- a/src/python/espressomd/gen_pxiconfig.py +++ /dev/null @@ -1,69 +0,0 @@ -# Copyright (C) 2016-2022 The ESPResSo project -# Copyright (C) 2014 Olaf Lenz -# -# This file is part of ESPResSo. -# -# ESPResSo is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# ESPResSo is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program. If not, see . -# -# This script generates gen_pxiconfig.cpp, which in turn generates myconfig.pxi. -# -import inspect -import sys -import os -# find featuredefs.py -moduledir = os.path.dirname(inspect.getfile(inspect.currentframe())) -sys.path.append(os.path.join(moduledir, '..', '..', 'config')) -import featuredefs - -if len(sys.argv) != 3: - print(f"Usage: {sys.argv[0]} DEFFILE CPPFILE", file=sys.stderr) - exit(2) - -deffilename, cfilename = sys.argv[1:3] - -print("Reading definitions from " + deffilename + "...") -defs = featuredefs.defs(deffilename) -print("Done.") - -# generate cpp-file -print("Writing " + cfilename + "...") -cfile = open(cfilename, 'w') - -cfile.write(""" -#include "config/config.hpp" -#include -int main() { - -std::cout << "# This file was autogenerated." << std::endl - << "# Do not modify it or your changes will be overwritten!" << std::endl; - -""") - -template = """ -#ifdef {0} -std::cout << "DEF {0} = 1" << std::endl; -#else -std::cout << "DEF {0} = 0" << std::endl; -#endif -""" - -for feature in defs.allfeatures: - cfile.write(template.format(feature)) - -cfile.write(""" -} -""") - -cfile.close() -print("Done.") diff --git a/src/python/espressomd/particle_data.py b/src/python/espressomd/particle_data.py index bde3cb5218a..8272834e47a 100644 --- a/src/python/espressomd/particle_data.py +++ b/src/python/espressomd/particle_data.py @@ -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 @@ -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 diff --git a/src/python/espressomd/script_interface.pxd b/src/python/espressomd/script_interface.pxd index bf074a0c542..7a78a2daf77 100644 --- a/src/python/espressomd/script_interface.pxd +++ b/src/python/espressomd/script_interface.pxd @@ -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 @@ -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) @@ -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 + diff --git a/src/python/espressomd/script_interface.pyx b/src/python/espressomd/script_interface.pyx index d663a96adb4..948a070f183 100644 --- a/src/python/espressomd/script_interface.pyx +++ b/src/python/espressomd/script_interface.pyx @@ -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 @@ -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)) @@ -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): @@ -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 @@ -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): diff --git a/src/python/espressomd/system.py b/src/python/espressomd/system.py index ed4e594b655..b4d0dd5f18e 100644 --- a/src/python/espressomd/system.py +++ b/src/python/espressomd/system.py @@ -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 @@ -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) @@ -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) diff --git a/src/python/espressomd/utils.pxd b/src/python/espressomd/utils.pxd index 6217a275c4d..4ae8f9ba0d1 100644 --- a/src/python/espressomd/utils.pxd +++ b/src/python/espressomd/utils.pxd @@ -21,8 +21,6 @@ from libcpp.string cimport string # import std::string as string from libcpp.vector cimport vector # import std::vector as vector from libcpp cimport bool as cbool -cpdef check_type_or_throw_except(x, n, t, msg) - cdef extern from "error_handling/RuntimeError.hpp" namespace "ErrorHandling::RuntimeError": cdef cppclass ErrorLevel: pass @@ -40,8 +38,6 @@ cdef extern from "error_handling/RuntimeError.hpp" namespace "ErrorHandling": cdef extern from "errorhandling.hpp" namespace "ErrorHandling": cdef vector[RuntimeError] mpi_gather_runtime_errors() -cpdef handle_errors(msg) - cdef extern from "utils/Vector.hpp" namespace "Utils": cppclass Vector2d: double & operator[](int i) @@ -62,6 +58,3 @@ cdef extern from "utils/Vector.hpp" namespace "Utils": cppclass Vector3i: int & operator[](int i) int * data() - -cdef make_array_locked(Vector3d) -cdef Vector3d make_Vector3d(a) diff --git a/src/python/espressomd/utils.pyx b/src/python/espressomd/utils.pyx index a63837faccc..2538be38f6d 100644 --- a/src/python/espressomd/utils.pyx +++ b/src/python/espressomd/utils.pyx @@ -20,52 +20,67 @@ cimport numpy as np import numpy as np -cdef _check_type_or_throw_except_assertion(x, t): - return isinstance(x, t) or (t == int and is_valid_type(x, int)) or ( - t == float and (is_valid_type(x, int) or is_valid_type(x, float))) or ( - t == bool and is_valid_type(x, bool)) +def is_valid_type(value, t): + """ + Extended checks for numpy int, float and bool types. + Handles 0-dimensional arrays. + + """ + if value is None: + return False + if isinstance(value, np.ndarray) and value.shape == (): + value = value[()] + if t is int: + return isinstance(value, (int, np.integer)) + elif t is float: + float_types = [ + float, np.float16, np.float32, np.float64, np.longdouble] + if hasattr(np, "float128"): + float_types.append(np.float128) + return isinstance(value, tuple(float_types)) + elif t is bool: + return isinstance(value, (bool, np.bool_)) + else: + return isinstance(value, t) -cpdef check_array_type_or_throw_except(x, n, t, msg): +def check_type_or_throw_except(x, n, t, msg): """ Check that ``x`` is of type ``t`` and that ``n`` values are given, - otherwise raise a ``ValueError`` with message ``msg``. + otherwise raise a ``ValueError`` with message ``msg``. If ``x`` is an + array/list/tuple, the type checking is done on the elements, and all + elements are checked. If ``n`` is 1, ``x`` is assumed to be a scalar. Integers are accepted when a float was asked for. """ + + def check_type(x, t): + return isinstance(x, t) or \ + (t is int and is_valid_type(x, int)) or \ + (t is bool and is_valid_type(x, bool)) or \ + (t is float and (is_valid_type(x, float) or is_valid_type(x, int))) + + if n == 1: + if not check_type(x, t): + raise ValueError(f"{msg} -- Got an {type(x).__name__}") + return + if not hasattr(x, "__getitem__"): raise ValueError( - msg + f" -- A single value was given but {n} were expected.") + f"{msg} -- A single value was given but {n} were expected.") if len(x) != n: raise ValueError( - msg + f" -- {len(x)} values were given but {n} were expected.") + f"{msg} -- {len(x)} values were given but {n} were expected.") if isinstance(x, np.ndarray): value = x.dtype.type() # default-constructed value of the same type - if not _check_type_or_throw_except_assertion(value, t): + if not check_type(value, t): raise ValueError( - msg + f" -- Array was of type {type(value).__name__}") + f"{msg} -- Array was of type {type(value).__name__}") return for i in range(len(x)): - if not _check_type_or_throw_except_assertion(x[i], t): + if not check_type(x[i], t): raise ValueError( - msg + f" -- Item {i} was of type {type(x[i]).__name__}") - - -cpdef check_type_or_throw_except(x, n, t, msg): - """ - Check that ``x`` is of type ``t`` and that ``n`` values are given, - otherwise raise a ``ValueError`` with message ``msg``. If ``x`` is an - array/list/tuple, the type checking is done on the elements, and all - elements are checked. If ``n`` is 1, ``x`` is assumed to be a scalar. - Integers are accepted when a float was asked for. - - """ - # Check whether x is an array/list/tuple or a single value - if n > 1: - check_array_type_or_throw_except(x, n, t, msg) - else: - if not _check_type_or_throw_except_assertion(x, t): - raise ValueError(msg + f" -- Got an {type(x).__name__}") + f"{msg} -- Item {i} was of type {type(x[i]).__name__}") def to_char_pointer(s): @@ -176,18 +191,7 @@ Use numpy.copy() to get a writable copy." raise ValueError(array_locked.ERR_MSG) -cdef make_array_locked(Vector3d v): - return array_locked([v[0], v[1], v[2]]) - - -cdef Vector3d make_Vector3d(a): - cdef Vector3d v - for i, ai in enumerate(a): - v[i] = ai - return v - - -cpdef handle_errors(msg): +def handle_errors(msg): """ Gathers runtime errors. @@ -227,30 +231,6 @@ def nesting_level(obj): return max_level + 1 -def is_valid_type(value, t): - """ - Extended checks for numpy int, float and bool types. - Handles 0-dimensional arrays. - - """ - if value is None: - return False - if isinstance(value, np.ndarray) and value.shape == (): - value = value[()] - if t == int: - return isinstance(value, (int, np.integer)) - elif t == float: - float_types = [ - float, np.float16, np.float32, np.float64, np.longdouble] - if hasattr(np, 'float128'): - float_types.append(np.float128) - return isinstance(value, tuple(float_types)) - elif t == bool: - return isinstance(value, (bool, np.bool_)) - else: - return isinstance(value, t) - - def check_required_keys(required_keys, obtained_keys): a = required_keys b = obtained_keys diff --git a/src/script_interface/electrostatics/ICCStar.hpp b/src/script_interface/electrostatics/ICCStar.hpp index 3e45def0250..9e995e7528e 100644 --- a/src/script_interface/electrostatics/ICCStar.hpp +++ b/src/script_interface/electrostatics/ICCStar.hpp @@ -77,13 +77,21 @@ class ICCStar : public AutoParameters { } void do_construct(VariantMap const ¶ms) override { + auto const n_icc = get_value(params, "n_icc"); + // by default, sigmas are zeros + std::vector sigmas{}; + if (params.count("sigmas")) { + sigmas = get_value>(params, "sigmas"); + } else if (n_icc >= 1) { + sigmas.resize(n_icc); + } auto icc_parameters = ::icc_data{ - get_value(params, "n_icc"), + n_icc, get_value(params, "max_iterations"), get_value(params, "eps_out"), get_value>(params, "areas"), get_value>(params, "epsilons"), - get_value>(params, "sigmas"), + sigmas, get_value(params, "convergence"), get_value>(params, "normals"), get_value(params, "ext_field"), diff --git a/src/script_interface/get_value.hpp b/src/script_interface/get_value.hpp index 5cf71638794..8f1bc914ffd 100644 --- a/src/script_interface/get_value.hpp +++ b/src/script_interface/get_value.hpp @@ -75,22 +75,28 @@ auto simplify_symbol(Utils::Vector const *) { } /** @overload */ -template auto simplify_symbol(std::vector const *) { +template auto simplify_symbol(std::vector const *vec) { auto const name_val = simplify_symbol(static_cast(nullptr)); - return "std::vector<" + name_val + ">"; + std::string metadata{""}; + if (vec) { + metadata += "{.size=" + std::to_string(vec->size()) + "}"; + } + return "std::vector<" + name_val + ">" + metadata; } /** @overload */ inline auto simplify_symbol(std::vector const *vec) { auto value_type_name = std::string("ScriptInterface::Variant"); + std::string metadata{""}; if (vec) { std::set types = {}; for (auto const &v : *vec) { types.insert(simplify_symbol_variant(v)); } value_type_name += "{" + boost::algorithm::join(types, ", ") + "}"; + metadata += "{.size=" + std::to_string(vec->size()) + "}"; } - return "std::vector<" + value_type_name + ">"; + return "std::vector<" + value_type_name + ">" + metadata; } /** @overload */ @@ -214,7 +220,7 @@ struct vector_conversion_visitor : boost::static_visitor> { throw boost::bad_get{}; } - Utils::Vector ret; + Utils::Vector ret{}; boost::transform(vv, ret.begin(), [](const Variant &v) { return get_value_helper{}(v); }); @@ -344,14 +350,18 @@ struct get_value_helper< * is a container. * @tparam T Which type the variant was supposed to convert to */ -template inline void handle_bad_get(Variant const &v) { +template +inline void handle_bad_get(Variant const &v, std::string const &name) { auto const container_name = demangle::simplify_symbol_variant(v); auto const containee_name = demangle::simplify_symbol_containee_variant(v); auto const expected_containee_name = demangle::simplify_symbol_containee(static_cast(nullptr)); auto const from_container = !containee_name.empty(); auto const to_container = !expected_containee_name.empty(); - auto const what = "Provided argument of type '" + container_name + "'"; + auto what = "Provided argument of type '" + container_name + "'"; + if (not name.empty()) { + what += " for parameter '" + name + "'"; + } try { throw; } catch (bad_get_nullptr const &) { @@ -369,6 +379,15 @@ template inline void handle_bad_get(Variant const &v) { } } +template T get_value(Variant const &v, std::string const &name) { + try { + return detail::get_value_helper{}(v); + } catch (...) { + detail::handle_bad_get(v, name); + throw; + } +} + } // namespace detail /** @@ -381,27 +400,20 @@ template inline void handle_bad_get(Variant const &v) { * to a requested type. */ template T get_value(Variant const &v) { - try { - return detail::get_value_helper{}(v); - } catch (...) { - detail::handle_bad_get(v); - throw; - } + return detail::get_value(v, ""); } /** * @brief Get a value from a VariantMap by name, or throw * if it does not exist or is not convertible to * the target type. - * */ template T get_value(VariantMap const &vals, std::string const &name) { - try { - return get_value(vals.at(name)); - } catch (std::out_of_range const &) { + if (vals.count(name) == 0ul) { throw Exception("Parameter '" + name + "' is missing."); } + return detail::get_value(vals.at(name), name); } /** diff --git a/src/script_interface/tests/get_value_test.cpp b/src/script_interface/tests/get_value_test.cpp index 05cc8360025..00baf0cf18f 100644 --- a/src/script_interface/tests/get_value_test.cpp +++ b/src/script_interface/tests/get_value_test.cpp @@ -208,7 +208,7 @@ BOOST_AUTO_TEST_CASE(check_exceptions) { auto const int_variant = Variant{1.5}; auto const vec_variant = Variant{std::vector{{so_obj}}}; auto const vec_variant_pattern = "std::vector<" + variant_sip_name + ">"; - auto const what = msg_prefix + "'" + vec_variant_pattern + "'"; + auto const what = msg_prefix + "'" + vec_variant_pattern + "\\{.size=1\\}'"; auto const predicate_nullptr = exception_message_predicate( what + " contains a value that is a null pointer"); auto const predicate_conversion_containee = exception_message_predicate( diff --git a/src/script_interface/walberla/LatticeWalberla.hpp b/src/script_interface/walberla/LatticeWalberla.hpp index 6c9c99a9de8..999513f0a7b 100644 --- a/src/script_interface/walberla/LatticeWalberla.hpp +++ b/src/script_interface/walberla/LatticeWalberla.hpp @@ -57,7 +57,7 @@ class LatticeWalberla : public AutoParameters { } void do_construct(VariantMap const &args) override { - auto const &box_geo = *System::get_system().box_geo; + auto const &box_geo = *::System::get_system().box_geo; m_agrid = get_value(args, "agrid"); m_box_l = get_value_or(args, "_box_l", box_geo.length()); auto const n_ghost_layers = get_value(args, "n_ghost_layers"); diff --git a/testsuite/python/box_geometry.py b/testsuite/python/box_geometry.py index c27c4880d03..ec950e3b59f 100644 --- a/testsuite/python/box_geometry.py +++ b/testsuite/python/box_geometry.py @@ -36,7 +36,7 @@ def test_box_length_interface(self): # the box length should not be updated np.testing.assert_equal(self.box_l, np.copy(self.system.box_l)) - with self.assertRaisesRegex(ValueError, "Attribute 'box_l' must be a list of 3 floats"): + with self.assertRaisesRegex(RuntimeError, "Provided argument of type 'std::vector{.size=2}' is not convertible to 'Utils::Vector'"): self.system.box_l = self.box_l[:2] def test_periodicity(self): @@ -50,7 +50,7 @@ def test_periodicity(self): default_periodicity = (True, True, True) self.system.periodicity = default_periodicity - with self.assertRaisesRegex(ValueError, "Attribute 'periodicity' must be a list of 3 bools"): + with self.assertRaisesRegex(RuntimeError, "Provided argument of type 'std::vector{.size=2}' is not convertible to 'Utils::Vector'"): self.system.periodicity = (True, True) # the periodicity should not be updated diff --git a/testsuite/python/coulomb_interface.py b/testsuite/python/coulomb_interface.py index b70012b1807..b2257fe49d8 100644 --- a/testsuite/python/coulomb_interface.py +++ b/testsuite/python/coulomb_interface.py @@ -253,7 +253,7 @@ def test_elc_p3m_exceptions(self): self.assertEqual( list(self.system.cell_system.node_grid), list(self.original_node_grid)) - with self.assertRaisesRegex(Exception, "while assigning system parameter 'box_l': ERROR: ELC gap size .+ larger than box length in z-direction"): + with self.assertRaisesRegex(Exception, "while setting parameter 'box_l': ERROR: ELC gap size .+ larger than box length in z-direction"): self.system.box_l = [10., 10., 2.5] self.system.box_l = [10., 10., 10.] self.system.electrostatics.solver = None diff --git a/testsuite/python/icc_interface.py b/testsuite/python/icc_interface.py index e447861077c..2d2cb5c41ae 100644 --- a/testsuite/python/icc_interface.py +++ b/testsuite/python/icc_interface.py @@ -98,13 +98,33 @@ def test_invalid_parameters(self): "Parameter 'relaxation' must be >= 0 and <= 2"), ({"relaxation": 2.1}, "Parameter 'relaxation' must be >= 0 and <= 2"), + ({"eps_out": "1"}, + "parameter 'eps_out' is not convertible to 'double'"), + ({"epsilons": [1.]}, + "Parameter 'epsilons' has incorrect shape"), + ({"areas": [1.]}, + "Parameter 'areas' has incorrect shape"), + ({"sigmas": [1.]}, + "Parameter 'sigmas' has incorrect shape"), + ({"normals": [[1., 2., 3.]]}, + "Parameter 'normals' has incorrect shape"), + ({"epsilons": len(areas) * ["str"]}, + "parameter 'epsilons' is not convertible to 'std::vector'"), + ({"areas": len(areas) * ["str"]}, + "parameter 'areas' is not convertible to 'std::vector'"), + ({"sigmas": len(areas) * ["str"]}, + "parameter 'sigmas' is not convertible to 'std::vector'"), + ({"normals": len(areas) * [3 * ["str"]]}, + "parameter 'normals' is not convertible to 'std::vector>'"), ({"eps_out": -1.}, "Parameter 'eps_out' must be > 0"), - ({"ext_field": 0.}, 'A single value was given but 3 were expected'), ] + ({"ext_field": 0.}, + "parameter 'ext_field' is not convertible to 'Utils::Vector'"), + ] for kwargs, error in invalid_params: params = valid_params.copy() params.update(kwargs) - with self.assertRaisesRegex(ValueError, error): + with self.assertRaisesRegex((ValueError, RuntimeError), error): espressomd.electrostatic_extensions.ICC(**params) @utx.skipIfMissingFeatures(["P3M"]) diff --git a/testsuite/python/pairs.py b/testsuite/python/pairs.py index 5f78a814e5f..b50bc5fa3f2 100644 --- a/testsuite/python/pairs.py +++ b/testsuite/python/pairs.py @@ -83,7 +83,7 @@ def check_pairs(self): def test_input_exceptions(self): with self.assertRaisesRegex(ValueError, "Unknown argument types='none'"): self.system.cell_system.get_pairs(0.1, types="none") - with self.assertRaisesRegex(RuntimeError, "Provided argument of type 'int' is not convertible to 'std::vector'"): + with self.assertRaisesRegex(RuntimeError, "Provided argument of type 'int' for parameter 'types' is not convertible to 'std::vector'"): self.system.cell_system.get_pairs(0.1, types=3) with self.assertRaisesRegex(RuntimeError, "Provided argument of type .+ because it contains a value that is not convertible to 'int'"): self.system.cell_system.get_pairs(0.1, types={'3.': 6.}) diff --git a/testsuite/python/particle.py b/testsuite/python/particle.py index d887bc9814a..75e4f630eab 100644 --- a/testsuite/python/particle.py +++ b/testsuite/python/particle.py @@ -113,6 +113,8 @@ def test_propagation_enum(self): Propagation = espressomd.propagation.Propagation flags_core = self.system.call_method("get_propagation_modes_enum") flags_si = {e.name: e.value for e in Propagation} + # Python 3.11+ skips empty bitfields during enum iteration + flags_si["NONE"] = Propagation.NONE self.assertEqual(flags_si, flags_core) test_bonds_property = generateTestForScalarProperty(