Skip to content

Commit

Permalink
Guard against ScaFaCoS fatal errors (#4068)
Browse files Browse the repository at this point in the history
Description of changes:
- catch runtime errors from ScaFaCoS at the python level instead of crashing ESPResSo
- use standard exception mechanism when setting Coulomb and dipolar prefactors
  • Loading branch information
kodiakhq[bot] authored Dec 24, 2020
2 parents dc06e0a + bf54c4d commit c197c76
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 12 deletions.
8 changes: 3 additions & 5 deletions src/core/electrostatics_magnetostatics/coulomb.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
#include <cassert>
#include <cstdio>
#include <limits>
#include <stdexcept>

Coulomb_parameters coulomb;

Expand Down Expand Up @@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion src/core/electrostatics_magnetostatics/coulomb.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
1 change: 1 addition & 0 deletions src/core/electrostatics_magnetostatics/dipole.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@

#include <cassert>
#include <cstdio>
#include <stdexcept>

Dipole_parameters dipole = {
0.0,
Expand Down
2 changes: 1 addition & 1 deletion src/python/espressomd/electrostatics.pxd
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion src/python/espressomd/scafacos.pxd
Original file line number Diff line number Diff line change
Expand Up @@ -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" ()
12 changes: 8 additions & 4 deletions src/python/espressomd/scafacos.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -108,9 +113,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)
Expand Down
21 changes: 21 additions & 0 deletions testsuite/python/scafacos_interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,27 @@ 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()
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

Expand Down

0 comments on commit c197c76

Please sign in to comment.