From 12bfc78c833340ce081dd920db8b99f4f20b5a4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-No=C3=ABl=20Grad?= Date: Tue, 22 Dec 2020 15:51:28 +0100 Subject: [PATCH 1/3] python: Simplify code --- src/python/espressomd/scafacos.pyx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/python/espressomd/scafacos.pyx b/src/python/espressomd/scafacos.pyx index d258c267e81..3a4c3af057a 100644 --- a/src/python/espressomd/scafacos.pyx +++ b/src/python/espressomd/scafacos.pyx @@ -108,9 +108,8 @@ IF SCAFACOS == 1: param_string = "" for k in method_params: param_string += k + " " + str(method_params[k]) + " " - # Remove trailing whitespace - param_string = param_string[0:-1] - param_string = param_string.replace(" ", ",") + # Format list as a single string + param_string = param_string.rstrip().replace(" ", ",") set_parameters(to_char_pointer(self._params["method_name"]), to_char_pointer(param_string), self.dipolar) From 5e4b9daf9bdc4c39e7d5604547a5de1daf4d144d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-No=C3=ABl=20Grad?= Date: Tue, 22 Dec 2020 17:06:20 +0100 Subject: [PATCH 2/3] core: Use standard exception mechanism Fatal errors from ScaFaCoS no longer crash ESPResSo. --- src/core/electrostatics_magnetostatics/coulomb.cpp | 8 +++----- src/core/electrostatics_magnetostatics/coulomb.hpp | 2 +- src/core/electrostatics_magnetostatics/dipole.cpp | 1 + src/python/espressomd/electrostatics.pxd | 2 +- src/python/espressomd/scafacos.pxd | 2 +- testsuite/python/scafacos_interface.py | 13 +++++++++++++ 6 files changed, 20 insertions(+), 8 deletions(-) diff --git a/src/core/electrostatics_magnetostatics/coulomb.cpp b/src/core/electrostatics_magnetostatics/coulomb.cpp index 1fdf502f41d..8ffbe210253 100644 --- a/src/core/electrostatics_magnetostatics/coulomb.cpp +++ b/src/core/electrostatics_magnetostatics/coulomb.cpp @@ -50,6 +50,7 @@ #include #include #include +#include Coulomb_parameters coulomb; @@ -439,16 +440,13 @@ void bcast_coulomb_params() { } } -int set_prefactor(double prefactor) { +void set_prefactor(double prefactor) { if (prefactor < 0.0) { - runtimeErrorMsg() << "Coulomb prefactor has to be >=0"; - return ES_ERROR; + throw std::invalid_argument("Coulomb prefactor has to be >= 0"); } coulomb.prefactor = prefactor; mpi_bcast_coulomb_params(); - - return ES_OK; } void deactivate_method() { diff --git a/src/core/electrostatics_magnetostatics/coulomb.hpp b/src/core/electrostatics_magnetostatics/coulomb.hpp index c60c351888b..c6cc7015fa9 100644 --- a/src/core/electrostatics_magnetostatics/coulomb.hpp +++ b/src/core/electrostatics_magnetostatics/coulomb.hpp @@ -82,7 +82,7 @@ int elc_sanity_check(); void bcast_coulomb_params(); /** @brief Set the electrostatics prefactor */ -int set_prefactor(double prefactor); +void set_prefactor(double prefactor); /** @brief Deactivates the current %Coulomb method. */ void deactivate_method(); diff --git a/src/core/electrostatics_magnetostatics/dipole.cpp b/src/core/electrostatics_magnetostatics/dipole.cpp index e8e683c21f3..d1cfa51b9f6 100644 --- a/src/core/electrostatics_magnetostatics/dipole.cpp +++ b/src/core/electrostatics_magnetostatics/dipole.cpp @@ -45,6 +45,7 @@ #include #include +#include Dipole_parameters dipole = { 0.0, diff --git a/src/python/espressomd/electrostatics.pxd b/src/python/espressomd/electrostatics.pxd index ca6bd85482b..ba28946ddee 100644 --- a/src/python/espressomd/electrostatics.pxd +++ b/src/python/espressomd/electrostatics.pxd @@ -61,7 +61,7 @@ IF ELECTROSTATICS: cdef extern from "electrostatics_magnetostatics/coulomb.hpp" namespace "Coulomb": - int set_prefactor(double prefactor) + int set_prefactor(double prefactor) except+ void deactivate_method() IF P3M: diff --git a/src/python/espressomd/scafacos.pxd b/src/python/espressomd/scafacos.pxd index d215ddea7fa..a14e1bc4656 100644 --- a/src/python/espressomd/scafacos.pxd +++ b/src/python/espressomd/scafacos.pxd @@ -24,7 +24,7 @@ from libcpp cimport bool from libcpp.list cimport list IF SCAFACOS: cdef extern from "electrostatics_magnetostatics/scafacos.hpp" namespace "Scafacos": - void set_parameters(string & method_name, string & params, bool dipolar) + void set_parameters(string & method_name, string & params, bool dipolar) except+ string get_method_and_parameters(bool dipolar) except+ void free_handle(bool dipolar) list[string] available_methods_core "Scafacos::available_methods" () diff --git a/testsuite/python/scafacos_interface.py b/testsuite/python/scafacos_interface.py index 6a1653c6523..600a2579bdd 100644 --- a/testsuite/python/scafacos_interface.py +++ b/testsuite/python/scafacos_interface.py @@ -50,6 +50,19 @@ def test_available_methods(self): for method in available_methods: self.assertIn(method, scafacos_methods) + def test_actor_exceptions(self): + system = self.system + + if espressomd.has_features('SCAFACOS_DIPOLES'): + with self.assertRaisesRegex(ValueError, "Dipolar prefactor has to be >= 0"): + system.actors.add(espressomd.magnetostatics.Scafacos( + prefactor=-1, method_name="p3m", method_params={"p3m_cao": 7})) + system.actors.clear() + with self.assertRaisesRegex(ValueError, "Coulomb prefactor has to be >= 0"): + system.actors.add(espressomd.electrostatics.Scafacos( + prefactor=-1, method_name="p3m", method_params={"p3m_cao": 7})) + system.actors.clear() + def test_actor_coulomb(self): system = self.system From 8782a2173617575eaff0cee49e55e1335117d1e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-No=C3=ABl=20Grad?= Date: Tue, 22 Dec 2020 17:17:32 +0100 Subject: [PATCH 3/3] python: Validate ScaFaCoS parameters --- src/python/espressomd/scafacos.pyx | 7 ++++++- testsuite/python/scafacos_interface.py | 8 ++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/python/espressomd/scafacos.pyx b/src/python/espressomd/scafacos.pyx index 3a4c3af057a..62726cf3fa2 100644 --- a/src/python/espressomd/scafacos.pyx +++ b/src/python/espressomd/scafacos.pyx @@ -54,7 +54,12 @@ IF SCAFACOS == 1: return {"method_name", "method_params", "prefactor"} def validate_params(self): - return True + if self._params["method_name"] not in available_methods(): + raise ValueError( + f"method '{self._params['method_name']}' is unknown or not compiled in ScaFaCoS") + if not self._params["method_params"]: + raise ValueError( + "ScaFaCoS methods require at least 1 parameter") def _get_params_from_es_core(self): # Parameters are returned as strings diff --git a/testsuite/python/scafacos_interface.py b/testsuite/python/scafacos_interface.py index 600a2579bdd..07d76194bb6 100644 --- a/testsuite/python/scafacos_interface.py +++ b/testsuite/python/scafacos_interface.py @@ -62,6 +62,14 @@ def test_actor_exceptions(self): system.actors.add(espressomd.electrostatics.Scafacos( prefactor=-1, method_name="p3m", method_params={"p3m_cao": 7})) system.actors.clear() + with self.assertRaisesRegex(ValueError, "method 'impossible' is unknown or not compiled in ScaFaCoS"): + system.actors.add(espressomd.electrostatics.Scafacos( + prefactor=1, method_name="impossible", method_params={"p3m_cao": 7})) + system.actors.clear() + with self.assertRaisesRegex(ValueError, "ScaFaCoS methods require at least 1 parameter"): + system.actors.add(espressomd.electrostatics.Scafacos( + prefactor=1, method_name="p3m", method_params={})) + system.actors.clear() def test_actor_coulomb(self): system = self.system