diff --git a/samples/grand_canonical.py b/samples/grand_canonical.py index 8326e9c8d76..7fe1c2b4ad1 100644 --- a/samples/grand_canonical.py +++ b/samples/grand_canonical.py @@ -38,7 +38,7 @@ import espressomd.reaction_ensemble import espressomd.electrostatics -required_features = ["P3M", "EXTERNAL_FORCES", "WCA"] +required_features = ["P3M", "WCA"] espressomd.assert_features(required_features) parser = argparse.ArgumentParser(epilog=__doc__ + epilog) diff --git a/samples/lj-demo.py b/samples/lj-demo.py index 46173f31cbf..eeab45f217a 100644 --- a/samples/lj-demo.py +++ b/samples/lj-demo.py @@ -454,7 +454,7 @@ def main_loop(): new_box = np.ones(3) * controls.volume**(1. / 3.) if np.any(np.array(system.box_l) != new_box): for p in system.part: - p.pos *= new_box / system.box_l[0] + p.pos = p.pos * new_box / system.box_l[0] print("volume changed") system.force_cap = lj_cap system.box_l = new_box diff --git a/src/core/grid_based_algorithms/lb_collective_interface.cpp b/src/core/grid_based_algorithms/lb_collective_interface.cpp index 4ea9ddcd5ef..2638f091602 100644 --- a/src/core/grid_based_algorithms/lb_collective_interface.cpp +++ b/src/core/grid_based_algorithms/lb_collective_interface.cpp @@ -109,14 +109,14 @@ auto mpi_lb_get_populations(Utils::Vector3i const &index) { REGISTER_CALLBACK_ONE_RANK(mpi_lb_get_populations) -auto mpi_lb_get_boundary_flag(Utils::Vector3i const &index) { +boost::optional mpi_lb_get_boundary_flag(Utils::Vector3i const &index) { return detail::lb_calc(index, [&](auto index) { #ifdef LB_BOUNDARIES auto const linear_index = get_linear_index(lblattice.local_index(index), lblattice.halo_grid); return lbfields[linear_index].boundary; #else - return false; + return 0; #endif }); } diff --git a/src/core/integrate.cpp b/src/core/integrate.cpp index d3777995dec..b56f36c8b3a 100644 --- a/src/core/integrate.cpp +++ b/src/core/integrate.cpp @@ -61,6 +61,7 @@ #include #include +#include #include #include #include @@ -419,6 +420,9 @@ int integrate(int n_steps, int reuse_forces) { int python_integrate(int n_steps, bool recalc_forces_par, bool reuse_forces_par) { + + assert(n_steps >= 0); + // Override the signal handler so that the integrator obeys Ctrl+C SignalHandler sa(SIGINT, [](int) { ctrl_C = 1; }); @@ -431,12 +435,6 @@ int python_integrate(int n_steps, bool recalc_forces_par, reuse_forces = -1; } - /* go on with integrate */ - if (n_steps < 0) { - runtimeErrorMsg() << "illegal number of steps (must be >0)"; - return ES_ERROR; - } - /* if skin wasn't set, do an educated guess now */ if (!skin_set) { auto const max_cut = maximal_cutoff(); @@ -552,7 +550,7 @@ REGISTER_CALLBACK(mpi_set_time_step_local) void mpi_set_time_step(double time_s) { if (time_s <= 0.) - throw std::invalid_argument("time_step must be > 0."); + throw std::domain_error("time_step must be > 0."); if (lb_lbfluid_get_lattice_switch() != ActiveLB::NONE) check_tau_time_step_consistency(lb_lbfluid_get_tau(), time_s); mpi_call_all(mpi_set_time_step_local, time_s); diff --git a/src/core/io/writer/h5md_core.hpp b/src/core/io/writer/h5md_core.hpp index 30f4e4b15b4..31c08497375 100644 --- a/src/core/io/writer/h5md_core.hpp +++ b/src/core/io/writer/h5md_core.hpp @@ -217,9 +217,8 @@ struct incompatible_h5mdfile : public std::exception { struct left_backupfile : public std::exception { const char *what() const noexcept override { - return "A backup of the .h5 file exists. This usually means \ -that either you forgot to call the 'close' method or your simulation \ -crashed."; + return "A backup of the .h5 file exists. This usually means that either " + "you forgot to call the 'close' method or your simulation crashed."; } }; diff --git a/src/core/particle_data.cpp b/src/core/particle_data.cpp index ff011db2825..17d8864a258 100644 --- a/src/core/particle_data.cpp +++ b/src/core/particle_data.cpp @@ -487,8 +487,9 @@ void build_particle_node() { mpi_who_has(); } * @brief Get the mpi rank which owns the particle with id. */ int get_particle_node(int id) { - if (id < 0) - throw std::runtime_error("Invalid particle id!"); + if (id < 0) { + throw std::domain_error("Invalid particle id: " + std::to_string(id)); + } if (particle_node.empty()) build_particle_node(); @@ -724,6 +725,9 @@ void mpi_place_particle(int node, int p_id, const Utils::Vector3d &pos) { } int place_particle(int p_id, Utils::Vector3d const &pos) { + if (p_id < 0) { + throw std::domain_error("Invalid particle id: " + std::to_string(p_id)); + } if (particle_exists(p_id)) { mpi_place_particle(get_particle_node(p_id), p_id, pos); diff --git a/src/core/reaction_methods/ReactionAlgorithm.hpp b/src/core/reaction_methods/ReactionAlgorithm.hpp index c99a1109ef2..2444fe6112e 100644 --- a/src/core/reaction_methods/ReactionAlgorithm.hpp +++ b/src/core/reaction_methods/ReactionAlgorithm.hpp @@ -61,18 +61,7 @@ class ReactionAlgorithm { } this->kT = kT; this->exclusion_range = exclusion_range; - for (auto const &item : exclusion_radius_per_type) { - auto type = item.first; - auto exclusion_radius = item.second; - if (exclusion_radius < 0) { - std::stringstream ss; - ss << "Invalid excluded_radius value for type " << type - << " value: " << exclusion_radius; - std::string error_message = ss.str(); - throw std::domain_error(error_message); - } - } - this->exclusion_radius_per_type = exclusion_radius_per_type; + set_exclusion_radius_per_type(exclusion_radius_per_type); update_volume(); } @@ -109,6 +98,20 @@ class ReactionAlgorithm { volume = new_volume; } void update_volume(); + void + set_exclusion_radius_per_type(std::unordered_map const &map) { + for (auto const &item : map) { + auto const type = item.first; + auto const exclusion_radius = item.second; + if (exclusion_radius < 0.) { + throw std::domain_error("Invalid excluded_radius value for type " + + std::to_string(type) + ": radius " + + std::to_string(exclusion_radius)); + } + } + exclusion_radius_per_type = map; + } + virtual int do_reaction(int reaction_steps); void check_reaction_method() const; void remove_constraint() { m_reaction_constraint = ReactionConstraint::NONE; } diff --git a/src/core/reaction_methods/tests/particle_tracking_test.cpp b/src/core/reaction_methods/tests/particle_tracking_test.cpp index df14f1e5f32..b792b9e1761 100644 --- a/src/core/reaction_methods/tests/particle_tracking_test.cpp +++ b/src/core/reaction_methods/tests/particle_tracking_test.cpp @@ -57,6 +57,9 @@ BOOST_FIXTURE_TEST_CASE(particle_type_map_test, ParticleFactory) { // exception for random index that exceeds the number of particles BOOST_CHECK_THROW(get_random_p_id(type, 10), std::runtime_error); + // exception for untracked particle types + BOOST_CHECK_THROW(get_random_p_id(type + 1, 0), std::runtime_error); + // check particle selection BOOST_CHECK_EQUAL(get_random_p_id(type, 0), pid); } diff --git a/src/core/rotation.cpp b/src/core/rotation.cpp index d402130f5a1..f9adc15c32c 100644 --- a/src/core/rotation.cpp +++ b/src/core/rotation.cpp @@ -117,7 +117,8 @@ static void define_Qdd(Particle const &p, Utils::Quaternion &Qd, * * For very high angular velocities (e.g. if the product of @p time_step * with the largest component of @ref ParticleMomentum::omega "p.omega()" - * is superior to ~2.0), the calculation might fail. + * is superior to ~2.0) and for @p time_step superior or equal to unity, + * the calculation might fail. * * \todo implement for fixed_coord_flag */ diff --git a/src/core/unit_tests/CMakeLists.txt b/src/core/unit_tests/CMakeLists.txt index 95560b0931a..fdc28c5c5c7 100644 --- a/src/core/unit_tests/CMakeLists.txt +++ b/src/core/unit_tests/CMakeLists.txt @@ -37,6 +37,8 @@ unit_test(NAME p3m_test SRC p3m_test.cpp DEPENDS EspressoUtils) unit_test(NAME link_cell_test SRC link_cell_test.cpp DEPENDS EspressoUtils) unit_test(NAME Particle_test SRC Particle_test.cpp DEPENDS EspressoUtils Boost::serialization) +unit_test(NAME rotation_test SRC rotation_test.cpp DEPENDS EspressoUtils + EspressoCore) unit_test(NAME field_coupling_couplings SRC field_coupling_couplings_test.cpp DEPENDS EspressoUtils) unit_test(NAME field_coupling_fields SRC field_coupling_fields_test.cpp DEPENDS diff --git a/src/core/unit_tests/Particle_test.cpp b/src/core/unit_tests/Particle_test.cpp index af08ba21579..86a0af10410 100644 --- a/src/core/unit_tests/Particle_test.cpp +++ b/src/core/unit_tests/Particle_test.cpp @@ -243,8 +243,6 @@ BOOST_AUTO_TEST_CASE(rattle_constructors) { } #endif // BOND_CONSTRAINT -#ifdef EXTERNAL_FORCES -#ifdef ROTATION BOOST_AUTO_TEST_CASE(particle_bitfields) { auto p = Particle(); @@ -255,24 +253,34 @@ BOOST_AUTO_TEST_CASE(particle_bitfields) { BOOST_CHECK(not p.can_rotate_around(1)); // check setting of one axis +#ifdef EXTERNAL_FORCES p.set_fixed_along(1, true); - p.set_can_rotate_around(1, true); BOOST_CHECK(p.is_fixed_along(1)); - BOOST_CHECK(p.can_rotate_around(1)); BOOST_CHECK(p.has_fixed_coordinates()); +#endif +#ifdef ROTATION + p.set_can_rotate_around(1, true); + BOOST_CHECK(p.can_rotate_around(1)); BOOST_CHECK(p.can_rotate()); +#endif // check that unsetting is properly registered +#ifdef EXTERNAL_FORCES p.set_fixed_along(1, false); - p.set_can_rotate_around(1, false); BOOST_CHECK(not p.has_fixed_coordinates()); +#endif +#ifdef ROTATION + p.set_can_rotate_around(1, false); BOOST_CHECK(not p.can_rotate()); +#endif // check setting of all flags at once +#ifdef ROTATION p.set_can_rotate_all_axes(); BOOST_CHECK(p.can_rotate_around(0)); BOOST_CHECK(p.can_rotate_around(1)); BOOST_CHECK(p.can_rotate_around(2)); + p.set_cannot_rotate_all_axes(); + BOOST_CHECK(not p.can_rotate()); +#endif } -#endif // ROTATION -#endif // EXTERNAL_FORCES \ No newline at end of file diff --git a/src/core/unit_tests/rotation_test.cpp b/src/core/unit_tests/rotation_test.cpp new file mode 100644 index 00000000000..336f16cd75f --- /dev/null +++ b/src/core/unit_tests/rotation_test.cpp @@ -0,0 +1,223 @@ +/* + * Copyright (C) 2022 The ESPResSo project + * + * 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 . + */ + +#define BOOST_TEST_MODULE rotation test +#define BOOST_TEST_DYN_LINK + +#include "config.hpp" + +#ifdef ROTATION + +#include + +#include "Particle.hpp" +#include "rotation.hpp" + +#include +#include +#include + +#include +#include +#include + +auto constexpr tol = 5. * 100. * std::numeric_limits::epsilon(); + +namespace Testing { +std::tuple, Utils::Vector3d> +setup_trivial_quat(int i, Utils::Vector3d const &v_in) { + auto quat = Utils::Quaternion{{0., 0., 0., 0.}}; + quat[i] = 1.; + auto v_ref = v_in; + if (i) { + v_ref *= -1.; + v_ref[i - 1] *= -1.; + } + return std::make_tuple(quat, v_ref); +} +} // namespace Testing + +BOOST_AUTO_TEST_CASE(convert_vector_space_to_body_test) { + auto const t_in = Utils::Vector3d{{1., 2., 3.}}; + for (int i : {0, 1, 2, 3}) { + auto p = Particle(); + Utils::Vector3d t_ref; + std::tie(p.quat(), t_ref) = Testing::setup_trivial_quat(i, t_in); + auto const t_out = convert_vector_space_to_body(p, t_in); + for (int j : {0, 1, 2}) { + BOOST_CHECK_CLOSE(t_out[j], t_ref[j], tol); + } + } +} + +BOOST_AUTO_TEST_CASE(convert_torque_to_body_frame_apply_fix_test) { + auto const t_in = Utils::Vector3d{{1., 2., 3.}}; + { + // test particle torque conversion + for (int i : {0, 1, 2, 3}) { + auto p = Particle(); + p.set_can_rotate_all_axes(); + Utils::Vector3d t_ref; + std::tie(p.quat(), t_ref) = Testing::setup_trivial_quat(i, t_in); + p.torque() = t_in; + convert_torque_to_body_frame_apply_fix(p); + auto const t_out = p.torque(); + for (int j : {0, 1, 2}) { + BOOST_CHECK_CLOSE(t_out[j], t_ref[j], tol); + } + } + } + { + // torque is set to zero for axes without rotation + for (int j : {0, 1, 2}) { + auto p = Particle(); + p.set_can_rotate_all_axes(); + p.set_can_rotate_around(j, false); + p.quat() = Utils::Quaternion::identity(); + p.torque() = t_in; + auto t_ref = t_in; + t_ref[j] = 0.; + convert_torque_to_body_frame_apply_fix(p); + BOOST_TEST(p.torque() == t_ref, boost::test_tools::per_element()); + } + } + { + // torque is always zero for non-rotatable particles + auto p = Particle(); + p.set_cannot_rotate_all_axes(); + p.quat() = Utils::Quaternion::identity(); + p.torque() = t_in; + convert_torque_to_body_frame_apply_fix(p); + auto const t_ref = Utils::Vector3d{}; + BOOST_TEST(p.torque() == t_ref, boost::test_tools::per_element()); + } +} + +BOOST_AUTO_TEST_CASE(rotate_particle_body_test) { + auto p = Particle(); + p.quat() = {1., 2., 3., 4.}; + { + // fixed particles are unaffected, quaternion is identical to original + p.set_cannot_rotate_all_axes(); + auto const phi = 2.; + auto const quat = local_rotate_particle_body(p, {0., 0., 1.}, phi); + BOOST_TEST((quat == p.quat())); + } + { + // edge case: null rotation throws an exception + p.set_can_rotate_around(2, true); + auto const phi = 2.; + BOOST_CHECK_THROW(local_rotate_particle_body(p, {1., 1., 0.}, phi), + std::exception); + } + { + // an angle of zero has no effect, quaternion is identical to original + p.set_can_rotate_all_axes(); + auto const phi = 0.; + auto const quat = local_rotate_particle_body(p, {1., 2., 3.}, phi); + BOOST_TEST((quat == p.quat())); + } + { + // an angle of pi around the z-axis flips the quaternion sequence + p.set_can_rotate_all_axes(); + auto const phi = Utils::pi(); + auto const quat = local_rotate_particle_body(p, {0., 0., 1.}, phi); + auto const quat_ref = Utils::Vector4d{{-4., 3., -2., 1.}}; + for (int i : {0, 1, 2, 3}) { + BOOST_CHECK_CLOSE(quat[i], quat_ref[i], tol); + } + } + { + // an angle of 2 pi around the z-axis flips the quaternion sign + p.set_can_rotate_all_axes(); + auto const phi = 2. * Utils::pi(); + auto const quat = local_rotate_particle_body(p, {0., 0., 1.}, phi); + auto const quat_ref = Utils::Vector4d{{-1., -2., -3., -4.}}; + for (int i : {0, 1, 2, 3}) { + BOOST_CHECK_CLOSE(quat[i], quat_ref[i], tol); + } + } +} + +BOOST_AUTO_TEST_CASE(propagate_omega_quat_particle_test) { + auto p = Particle(); + p.set_can_rotate_all_axes(); + { + // test edge case: null quaternion and no rotation + p.quat() = {0., 0., 0., 0.}; + p.omega() = {0., 0., 0.}; + propagate_omega_quat_particle(p, 0.01); + auto const quat = p.quat(); + auto const quat_ref = Utils::Quaternion::identity(); + for (int i : {0, 1, 2, 3}) { + BOOST_CHECK_CLOSE(quat[i], quat_ref[i], tol); + } + } + { + // test trivial cases with parameters extremely close to the limit: + // time step almost 1.0 and product of time step with omega almost 2.0 + auto const time_step = 0.99; + for (int j : {0, 1, 2}) { + p.quat() = Utils::Quaternion::identity(); + p.omega() = {0., 0., 0.}; + p.omega()[j] = 2.; + propagate_omega_quat_particle(p, time_step); + auto const quat = p.quat(); + auto quat_ref = Utils::Quaternion::identity(); + quat_ref[1 + j] = time_step; + quat_ref[0] = std::sqrt(1. - time_step * time_step); + for (int i : {0, 1, 2, 3}) { + BOOST_CHECK_CLOSE(quat[i], quat_ref[i], tol); + } + } + } +} + +#ifdef DIPOLES +BOOST_AUTO_TEST_CASE(convert_dip_to_quat_test) { + auto const quat_to_vector4d = [](Utils::Quaternion const &quat) { + return Utils::Vector4d{quat.data(), quat.data() + 4}; + }; + auto p = Particle(); + p.quat() = {1., 2., 3., 4.}; + { + auto const dipm = 0.8; + auto const pair = convert_dip_to_quat({0., 0., dipm}); + auto const quat = quat_to_vector4d(pair.first); + auto const quat_ref = Utils::Vector4d{{1., 0., 0., 0.}}; + for (int i : {0, 1, 2, 3}) { + BOOST_CHECK_CLOSE(quat[i], quat_ref[i], tol); + } + BOOST_CHECK_CLOSE(pair.second, dipm, tol); + } + { + auto const dipm = 1.6; + auto const pair = convert_dip_to_quat({dipm, 0., 0.}); + auto const quat = quat_to_vector4d(pair.first); + auto const quat_ref = Utils::Vector4d{{0.5, -0.5, 0.5, -0.5}}; + for (int i : {0, 1, 2, 3}) { + BOOST_CHECK_CLOSE(quat[i], quat_ref[i], tol); + } + BOOST_CHECK_CLOSE(pair.second, dipm, tol); + } +} +#endif // DIPOLES +#else // ROTATION +int main(int argc, char **argv) {} +#endif // ROTATION diff --git a/src/core/virtual_sites.cpp b/src/core/virtual_sites.cpp index e591277f0a2..629468a9b00 100644 --- a/src/core/virtual_sites.cpp +++ b/src/core/virtual_sites.cpp @@ -38,6 +38,7 @@ #include #include +#include #include namespace { @@ -66,12 +67,10 @@ calculate_vs_relate_to_params(Particle const &p_current, if (dist > min_global_cut && n_nodes > 1) { runtimeErrorMsg() << "Warning: The distance between virtual and non-virtual particle (" - << dist << ") is\nlarger than the minimum global cutoff (" - << min_global_cut - << "). This may lead to incorrect simulations\nunder certain " - "conditions. Set the \"System()\" " - "class property \"min_global_cut\" to\nincrease the minimum " - "cutoff.\n"; + << dist << ") is larger than the minimum global cutoff (" + << min_global_cut << "). This may lead to incorrect simulations under " + << "certain conditions. Adjust the property system.min_global_cut to " + << "increase the minimum cutoff."; } // Now, calculate the quaternions which specify the angle between @@ -136,6 +135,9 @@ void local_vs_relate_to(Particle &p_current, Particle const &p_relate_to) { } void vs_relate_to(int part_num, int relate_to) { + if (part_num == relate_to) { + throw std::invalid_argument("A virtual site cannot relate to itself"); + } // Get the data for the particle we act on and the one we want to relate // it to. auto const &p_current = get_particle_data(part_num); diff --git a/src/core/virtual_sites/VirtualSitesRelative.cpp b/src/core/virtual_sites/VirtualSitesRelative.cpp index c88fb2a33fd..943c1c8f25a 100644 --- a/src/core/virtual_sites/VirtualSitesRelative.cpp +++ b/src/core/virtual_sites/VirtualSitesRelative.cpp @@ -23,6 +23,7 @@ #include "Particle.hpp" #include "cells.hpp" +#include "errorhandling.hpp" #include "forces.hpp" #include "grid.hpp" #include "integrate.hpp" @@ -33,80 +34,72 @@ #include #include -#include - -namespace { /** * @brief Vector pointing from the real particle to the virtual site. * * @return Relative distance. */ -auto connection_vector( - Particle const &p_ref, - ParticleProperties::VirtualSitesRelativeParameters const &vs_rel) { +static auto connection_vector(Particle const &p_ref, Particle const &p_vs) { // Calculate the quaternion defining the orientation of the vector connecting // the virtual site and the real particle // This is obtained, by multiplying the quaternion representing the director // of the real particle with the quaternion of the virtual particle, which // specifies the relative orientation. auto const director = Utils::convert_quaternion_to_director( - p_ref.quat() * vs_rel.rel_orientation) + p_ref.quat() * p_vs.vs_relative().rel_orientation) .normalize(); - return vs_rel.distance * director; + return p_vs.vs_relative().distance * director; } /** * @brief Velocity of the virtual site * @param p_ref Reference particle for the virtual site. - * @param vs_rel Parameters for the virtual site. + * @param p_vs Virtual site. * @return Velocity of the virtual site. */ -Utils::Vector3d -velocity(Particle const &p_ref, - ParticleProperties::VirtualSitesRelativeParameters const &vs_rel) { - auto const d = connection_vector(p_ref, vs_rel); +static Utils::Vector3d velocity(Particle const &p_ref, Particle const &p_vs) { + auto const d = connection_vector(p_ref, p_vs); // Get omega of real particle in space-fixed frame auto const omega_space_frame = convert_vector_body_to_space(p_ref, p_ref.omega()); - // Obtain velocity from v=v_real particle + omega_real_particle \times - // director + // Obtain velocity from v = v_real particle + omega_real_particle * director return vector_product(omega_space_frame, d) + p_ref.v(); } /** - * @brief Get reference particle. + * @brief Get real particle tracked by a virtual site. * - * @param vs_rel Parameters to get the reference particle for. - * @return Pointer to reference particle. + * @param p Virtual site. + * @return Pointer to real particle. */ -Particle &get_reference_particle( - ParticleProperties::VirtualSitesRelativeParameters const &vs_rel) { +static Particle *get_reference_particle(Particle const &p) { + if (!p.is_virtual()) { + return nullptr; + } + auto const &vs_rel = p.vs_relative(); auto p_ref_ptr = cell_structure.get_local_particle(vs_rel.to_particle_id); if (!p_ref_ptr) { - throw std::runtime_error("No real particle associated with virtual site."); + runtimeErrorMsg() << "No real particle with id " << vs_rel.to_particle_id + << " for virtual site with id " << p.identity(); } - return *p_ref_ptr; + return p_ref_ptr; } /** * @brief Constraint force to hold the particles at its prescribed position. * - * @param f Force on the virtual site. * @param p_ref Reference particle. - * @param vs_rel Parameters. + * @param p_vs Virtual site. * @return Constraint force. */ -auto constraint_stress( - const Utils::Vector3d &f, const Particle &p_ref, - const ParticleProperties::VirtualSitesRelativeParameters &vs_rel) { +static auto constraint_stress(Particle const &p_ref, Particle const &p_vs) { /* The constraint force is minus the force on the particle, make it force * free. The counter force is translated by the connection vector to the * real particle, hence the virial stress is */ - return tensor_product(-f, connection_vector(p_ref, vs_rel)); + return tensor_product(-p_vs.force(), connection_vector(p_ref, p_vs)); } -} // namespace void VirtualSitesRelative::update() const { cell_structure.ghosts_update(Cells::DATA_PART_POSITION | @@ -114,12 +107,12 @@ void VirtualSitesRelative::update() const { auto const particles = cell_structure.local_particles(); for (auto &p : particles) { - if (!p.is_virtual()) + auto const *p_ref_ptr = get_reference_particle(p); + if (!p_ref_ptr) continue; - auto const &p_ref = get_reference_particle(p.vs_relative()); - - auto new_pos = p_ref.pos() + connection_vector(p_ref, p.vs_relative()); + auto const &p_ref = *p_ref_ptr; + auto new_pos = p_ref.pos() + connection_vector(p_ref, p); /* The shift has to respect periodic boundaries: if the reference * particles is not in the same image box, we potentially avoid shifting * to the other side of the box. */ @@ -129,7 +122,7 @@ void VirtualSitesRelative::update() const { fold_position(shift, image_shift, box_geo); p.image_box() = p_ref.image_box() - image_shift; - p.v() = velocity(p_ref, p.vs_relative()); + p.v() = velocity(p_ref, p); if (box_geo.type() == BoxType::LEES_EDWARDS) { auto const &shear_dir = box_geo.clees_edwards_bc().shear_direction; @@ -158,16 +151,15 @@ void VirtualSitesRelative::back_transfer_forces_and_torques() const { // Iterate over all the particles in the local cells for (auto &p : cell_structure.local_particles()) { - // We only care about virtual particles - if (!p.is_virtual()) + auto *p_ref_ptr = get_reference_particle(p); + if (!p_ref_ptr) continue; - auto &p_ref = get_reference_particle(p.vs_relative()); // Add forces and torques + auto &p_ref = *p_ref_ptr; p_ref.force() += p.force(); p_ref.torque() += - vector_product(connection_vector(p_ref, p.vs_relative()), p.force()) + - p.torque(); + vector_product(connection_vector(p_ref, p), p.force()) + p.torque(); } } @@ -175,13 +167,11 @@ void VirtualSitesRelative::back_transfer_forces_and_torques() const { Utils::Matrix VirtualSitesRelative::pressure_tensor() const { Utils::Matrix pressure_tensor = {}; - for (auto &p : cell_structure.local_particles()) { - if (!p.is_virtual()) - continue; - - auto const &p_ref = get_reference_particle(p.vs_relative()); - - pressure_tensor += constraint_stress(p.force(), p_ref, p.vs_relative()); + for (auto const &p : cell_structure.local_particles()) { + auto const *p_ref_ptr = get_reference_particle(p); + if (p_ref_ptr) { + pressure_tensor += constraint_stress(*p_ref_ptr, p); + } } return pressure_tensor; diff --git a/src/python/espressomd/cellsystem.pyx b/src/python/espressomd/cellsystem.pyx index 3c023ea39f7..8a76ea2f759 100644 --- a/src/python/espressomd/cellsystem.pyx +++ b/src/python/espressomd/cellsystem.pyx @@ -126,7 +126,7 @@ cdef class CellSystem: pairs = mpi_get_pairs(distance) else: pairs = self._get_pairs_of_types(distance, types) - handle_errors("") + handle_errors("Error in get_pairs()") return pairs def _get_pairs_of_types(self, distance, types): diff --git a/src/python/espressomd/integrate.pyx b/src/python/espressomd/integrate.pyx index 9851626ab2f..7cad1ef1861 100644 --- a/src/python/espressomd/integrate.pyx +++ b/src/python/espressomd/integrate.pyx @@ -210,11 +210,12 @@ cdef class Integrator: """ check_type_or_throw_except(steps, 1, int, "steps must be an int") - assert steps >= 0, "steps has to be positive" check_type_or_throw_except( recalc_forces, 1, bool, "recalc_forces has to be a bool") check_type_or_throw_except( reuse_forces, 1, bool, "reuse_forces has to be a bool") + if steps < 0: + raise ValueError("steps must be positive") _integrate(steps, recalc_forces, reuse_forces) diff --git a/src/python/espressomd/interactions.pyx b/src/python/espressomd/interactions.pyx index 9b82e08cd61..2a411679c22 100644 --- a/src/python/espressomd/interactions.pyx +++ b/src/python/espressomd/interactions.pyx @@ -24,7 +24,7 @@ import collections include "myconfig.pxi" from . import utils from .utils import is_valid_type -from .utils cimport check_type_or_throw_except +from .utils cimport check_type_or_throw_except, handle_errors from .script_interface import ScriptObjectRegistry, ScriptInterfaceHelper, script_interface_register @@ -73,9 +73,7 @@ cdef class NonBondedInteraction: """ temp_params = self._get_params_from_es_core() - if self._params != temp_params: - return False - return True + return self._params == temp_params def get_params(self): """Get interaction parameters. @@ -116,14 +114,15 @@ cdef class NonBondedInteraction: # If this instance refers to an interaction defined in the ESPResSo core, # load the parameters from there + is_valid_ia = self._part_types[0] >= 0 and self._part_types[1] >= 0 - if self._part_types[0] >= 0 and self._part_types[1] >= 0: + if is_valid_ia: self._params = self._get_params_from_es_core() # Put in values given by the user self._params.update(p) - if self._part_types[0] >= 0 and self._part_types[1] >= 0: + if is_valid_ia: self._set_params_in_es_core() # update interaction dict when user sets interaction @@ -137,6 +136,10 @@ cdef class NonBondedInteraction: self.user_interactions[self._part_types[0]][ self._part_types[1]]['type_name'] = self.type_name() + # defer exception (core and interface must always agree on parameters) + if is_valid_ia: + handle_errors(f'setting {self.type_name()} raised an error') + def validate_params(self): """Check that parameters are valid. diff --git a/src/python/espressomd/particle_data.pxd b/src/python/espressomd/particle_data.pxd index 99b076af16f..695047b4090 100644 --- a/src/python/espressomd/particle_data.pxd +++ b/src/python/espressomd/particle_data.pxd @@ -74,7 +74,7 @@ cdef extern from "particle_data.hpp": # Setter/getter/modifier functions functions void prefetch_particle_data(vector[int] ids) - int place_particle(int part, const Vector3d & p) + int place_particle(int part, const Vector3d & p) except + void set_particle_v(int part, const Vector3d & v) @@ -175,7 +175,7 @@ cdef extern from "particle_data.hpp": cdef extern from "virtual_sites.hpp": IF VIRTUAL_SITES_RELATIVE == 1: - void vs_relate_to(int part_num, int relate_to) + void vs_relate_to(int part_num, int relate_to) except + cdef extern from "rotation.hpp": Vector3d convert_vector_body_to_space(const particle & p, const Vector3d & v) diff --git a/src/python/espressomd/particle_data.pyx b/src/python/espressomd/particle_data.pyx index 623ef1e439f..28b8afc9b56 100644 --- a/src/python/espressomd/particle_data.pyx +++ b/src/python/espressomd/particle_data.pyx @@ -29,7 +29,7 @@ from .analyze cimport max_seen_particle_type from copy import copy import collections import functools -from .utils import nesting_level, array_locked, is_valid_type +from .utils import nesting_level, array_locked, is_valid_type, handle_errors from .utils cimport make_array_locked, make_const_span, check_type_or_throw_except from .utils cimport Vector3i, Vector3d, Vector4d from .utils cimport make_Vector3d @@ -373,8 +373,8 @@ cdef class ParticleHandle: _mass, 1, float, "Mass has to be 1 float") set_particle_mass(self._id, _mass) ELSE: - raise AttributeError("You are trying to set the particle mass \ - but the mass feature is not compiled in.") + raise AttributeError("You are trying to set the particle mass " + "but the MASS feature is not compiled in.") def __get__(self): self.update_particle_data() @@ -427,6 +427,8 @@ cdef class ParticleHandle: cdef Quaternion[double] q check_type_or_throw_except( _q, 4, float, "Quaternions has to be 4 floats.") + if np.linalg.norm(_q) == 0.: + raise ValueError("quaternion is zero") for i in range(4): q[i] = _q[i] set_particle_quat(self._id, q) @@ -642,6 +644,8 @@ cdef class ParticleHandle: def __set__(self, q): check_type_or_throw_except( q, 4, float, "vs_quat has to be an array-like of length 4") + if np.linalg.norm(q) == 0.: + raise ValueError("quaternion is zero") cdef Quaternion[double] _q for i in range(4): _q[i] = q[i] @@ -680,6 +684,8 @@ cdef class ParticleHandle: dist, 1, float, "The distance has to be given as a float.") check_type_or_throw_except( quat, 4, float, "The quaternion has to be given as a tuple of 4 floats.") + if np.linalg.norm(quat) == 0.: + raise ValueError("quaternion is zero") cdef Quaternion[double] q for i in range(4): q[i] = quat[i] @@ -698,7 +704,7 @@ cdef class ParticleHandle: def vs_auto_relate_to(self, rel_to): """ Setup this particle as virtual site relative to the particle - in argument ``rel_to``. + in argument ``rel_to``. A particle cannot relate to itself. Parameters ----------- @@ -713,6 +719,7 @@ cdef class ParticleHandle: check_type_or_throw_except( rel_to, 1, int, "Argument of vs_auto_relate_to has to be of type ParticleHandle or int.") vs_relate_to(self._id, rel_to) + handle_errors('vs_auto_relate_to') IF DIPOLES: property dip: @@ -1740,84 +1747,83 @@ cdef class ParticleList: # Did we get a dictionary if len(args) == 1: if hasattr(args[0], "__getitem__"): - P = args[0] + particles_dict = args[0] else: - if len(args) == 0 and len(kwargs.keys()) != 0: - P = kwargs + if len(args) == 0 and len(kwargs) != 0: + particles_dict = kwargs else: raise ValueError( "add() takes either a dictionary or a bunch of keyword args.") # Check for presence of pos attribute - if "pos" not in P: + if "pos" not in particles_dict: raise ValueError( "pos attribute must be specified for new particle") - if len(np.array(P["pos"]).shape) == 2: - return self._place_new_particles(P) + if len(np.array(particles_dict["pos"]).shape) == 2: + return self._place_new_particles(particles_dict) else: - return self._place_new_particle(P) + return self._place_new_particle(particles_dict) - def _place_new_particle(self, P): + def _place_new_particle(self, p_dict): # Handling of particle id - if "id" not in P: + if "id" not in p_dict: # Generate particle id - P["id"] = get_maximal_particle_id() + 1 + p_dict["id"] = get_maximal_particle_id() + 1 else: - if particle_exists(P["id"]): - raise Exception(f"Particle {P['id']} already exists.") + if particle_exists(p_dict["id"]): + raise Exception(f"Particle {p_dict['id']} already exists.") # Prevent setting of contradicting attributes IF DIPOLES: - if 'dip' in P and 'dipm' in P: + if 'dip' in p_dict and 'dipm' in p_dict: raise ValueError("Contradicting attributes: dip and dipm. Setting \ dip is sufficient as the length of the vector defines the scalar dipole moment.") IF ROTATION: - if 'dip' in P and 'quat' in P: + if 'dip' in p_dict and 'quat' in p_dict: raise ValueError("Contradicting attributes: dip and quat. \ Setting dip overwrites the rotation of the particle around the dipole axis. \ Set quat and scalar dipole moment (dipm) instead.") # The ParticleList can not be used yet, as the particle # doesn't yet exist. Hence, the setting of position has to be - # done here. the code is from the pos:property of ParticleHandle + # done here. check_type_or_throw_except( - P["pos"], 3, float, "Position must be 3 floats.") - if place_particle(P["id"], make_Vector3d(P["pos"])) == -1: + p_dict["pos"], 3, float, "Position must be 3 floats.") + error_code = place_particle(p_dict["id"], make_Vector3d(p_dict["pos"])) + if error_code == -1: raise Exception("particle could not be set.") - # Pos is taken care of - del P["pos"] - pid = P["id"] - del P["id"] + # position is taken care of + del p_dict["pos"] + pid = p_dict.pop("id") - if P != {}: - self.by_id(pid).update(P) + if p_dict != {}: + self.by_id(pid).update(p_dict) return self.by_id(pid) - def _place_new_particles(self, Ps): + def _place_new_particles(self, p_list_dict): # Check if all entries have the same length - n_parts = len(Ps["pos"]) - if not all(np.shape(Ps[k]) and len(Ps[k]) == n_parts for k in Ps): + n_parts = len(p_list_dict["pos"]) + if not all(np.shape(v) and len(v) == + n_parts for v in p_list_dict.values()): raise ValueError( "When adding several particles at once, all lists of attributes have to have the same size") # If particle ids haven't been provided, use free ones # beyond the highest existing one - if not "id" in Ps: + if "id" not in p_list_dict: first_id = get_maximal_particle_id() + 1 - Ps["id"] = range(first_id, first_id + n_parts) + p_list_dict["id"] = range(first_id, first_id + n_parts) # Place the particles for i in range(n_parts): - P = {} - for k in Ps: - P[k] = Ps[k][i] - self._place_new_particle(P) + p_dict = {k: v[i] for k, v in p_list_dict.items()} + self._place_new_particle(p_dict) # Return slice of added particles - return self.by_ids(Ps["id"]) + return self.by_ids(p_list_dict["id"]) # Iteration over all existing particles def __iter__(self): diff --git a/src/python/espressomd/script_interface.pyx b/src/python/espressomd/script_interface.pyx index 531639c8f1c..42746780e47 100644 --- a/src/python/espressomd/script_interface.pyx +++ b/src/python/espressomd/script_interface.pyx @@ -150,7 +150,7 @@ cdef class PScriptInterface: res = variant_to_python_object( self.sip.get().call_method(to_char_pointer(method), parameters)) - handle_errors("") + handle_errors(f'while calling method {method}()') return res def name(self): diff --git a/src/python/espressomd/system.pyx b/src/python/espressomd/system.pyx index c25c8bd279c..e4a46691ea5 100644 --- a/src/python/espressomd/system.pyx +++ b/src/python/espressomd/system.pyx @@ -511,6 +511,4 @@ cdef class System: """ check_type_or_throw_except(type, 1, int, "type must be 1 int") - number = number_of_particles_with_type(type) - handle_errors("") - return int(number) + return number_of_particles_with_type(type) diff --git a/src/python/espressomd/visualization_opengl.py b/src/python/espressomd/visualization_opengl.py index 3984349bc66..9b4b7588fe4 100644 --- a/src/python/espressomd/visualization_opengl.py +++ b/src/python/espressomd/visualization_opengl.py @@ -1587,22 +1587,17 @@ def _init_espresso_visualization(self): self.depth = 0 # LOOK FOR LB ACTOR - if self.specs['LB_draw_velocity_plane'] or \ - self.specs['LB_draw_nodes'] or \ - self.specs['LB_draw_node_boundaries']: - lb_types = [espressomd.lb.LBFluid] - if espressomd.has_features('CUDA'): - lb_types.append(espressomd.lb.LBFluidGPU) - for a in self.system.actors: - if isinstance(a, tuple(lb_types)): - # if 'agrid' in pa: - self.lb_params = a.get_params() - self.lb = a - self.lb_is_cpu = isinstance(a, espressomd.lb.LBFluid) - break + lb_types = [espressomd.lb.LBFluid] + if espressomd.has_features('CUDA'): + lb_types.append(espressomd.lb.LBFluidGPU) + for actor in self.system.actors: + if isinstance(actor, tuple(lb_types)): + self.lb_params = actor.get_params() + self.lb = actor + self.lb_is_cpu = isinstance(actor, espressomd.lb.LBFluid) + break if self.specs['LB_draw_velocity_plane']: - if self.specs['LB_plane_axis'] == 0: pn = [1.0, 0.0, 0.0] self.lb_plane_b1 = [0.0, 1.0, 0.0] diff --git a/src/script_interface/ObjectMap.hpp b/src/script_interface/ObjectMap.hpp index d6af7da67df..a41d7ddc902 100644 --- a/src/script_interface/ObjectMap.hpp +++ b/src/script_interface/ObjectMap.hpp @@ -130,12 +130,7 @@ class ObjectMap : public BaseType { } if (method == "get_map") { - std::unordered_map ret; - - for (auto const &kv : m_elements) - ret[kv.first] = kv.second; - - return ret; + return make_unordered_map_of_variants(m_elements); } if (method == "keys") { diff --git a/src/script_interface/constraints/couplings.hpp b/src/script_interface/constraints/couplings.hpp index c1ceab0de49..b96831bd883 100644 --- a/src/script_interface/constraints/couplings.hpp +++ b/src/script_interface/constraints/couplings.hpp @@ -74,8 +74,9 @@ template <> struct coupling_parameters_impl { AutoParameter::read_only, [this_]() { return this_().default_scale(); }, }, - {"particle_scales", AutoParameter::read_only, - [this_]() { return make_map(this_().particle_scales()); }}}; + {"particle_scales", AutoParameter::read_only, [this_]() { + return make_unordered_map_of_variants(this_().particle_scales()); + }}}; } }; @@ -90,9 +91,8 @@ template <> inline Viscous make_coupling(const VariantMap ¶ms) { } template <> inline Scaled make_coupling(const VariantMap ¶ms) { - auto const particle_scales = get_value_or>( - params, "particle_scales", {}); - return Scaled{get_map(particle_scales), + return Scaled{get_value_or>( + params, "particle_scales", {}), get_value(params, "default_scale")}; } } // namespace detail diff --git a/src/script_interface/get_value.hpp b/src/script_interface/get_value.hpp index acff18edf23..904230b8079 100644 --- a/src/script_interface/get_value.hpp +++ b/src/script_interface/get_value.hpp @@ -38,16 +38,64 @@ namespace ScriptInterface { namespace detail { + +/** + * @brief Demangle the symbol of a container @c value_type or @c mapped_type. + * Only for recursive variant container types used in @ref Variant. + * Other container types and non-container objects return an empty string. + */ +template struct demangle_container_value_type { +private: + template struct is_std_vector : std::false_type {}; + template + struct is_std_vector> : std::true_type {}; + + template struct is_std_unordered_map : std::false_type {}; + template + struct is_std_unordered_map> : std::true_type {}; + +public: + template ::value and + !is_std_unordered_map::value, + bool> = true> + auto operator()() const { + return std::string(""); + } + + template ::value, bool> = true> + auto operator()() const { + return Utils::demangle(); + } + + template ::value, bool> = true> + auto operator()() const { + return Utils::demangle(); + } +}; + +/** + * @brief Demangle the data type of an object wrapped inside a @ref Variant. + * When the data type involves a recursive variant (e.g. containers), the + * demangled symbol name is processed to replace all occurences of the + * variant symbol name by a human-readable string "ScriptInterface::Variant". + */ struct type_label_visitor : boost::static_visitor { template std::string operator()(const T &) const { - return Utils::demangle(); + auto const symbol_for_variant = Utils::demangle(); + auto const name_for_variant = std::string("ScriptInterface::Variant"); + auto symbol = Utils::demangle(); + for (std::string::size_type pos{}; + (pos = symbol.find(symbol_for_variant, pos)) != symbol.npos; + pos += name_for_variant.length()) { + symbol.replace(pos, symbol_for_variant.length(), name_for_variant); + } + return symbol; } }; -inline std::string type_label(const Variant &v) { - return boost::apply_visitor(type_label_visitor{}, v); -} - /* * Allows * T -> T, @@ -137,6 +185,9 @@ struct GetVectorOrEmpty : boost::static_visitor> { /* Standard case, correct type */ std::vector operator()(std::vector const &v) const { return v; } + + template ::value, bool> = true> std::vector operator()(std::vector const &vv) const { std::vector ret(vv.size()); @@ -147,17 +198,10 @@ struct GetVectorOrEmpty : boost::static_visitor> { } }; -/* std::vector cases - * We implicitly transform an empty vector into an empty vector. */ -template <> struct get_value_helper, void> { - std::vector operator()(Variant const &v) const { - return boost::apply_visitor(GetVectorOrEmpty{}, v); - } -}; - -template <> struct get_value_helper, void> { - std::vector operator()(Variant const &v) const { - return boost::apply_visitor(GetVectorOrEmpty{}, v); +/* std::vector cases */ +template struct get_value_helper, void> { + std::vector operator()(Variant const &v) const { + return boost::apply_visitor(GetVectorOrEmpty{}, v); } }; @@ -172,12 +216,24 @@ struct GetMapOrEmpty : boost::static_visitor> { std::unordered_map operator()(std::unordered_map const &v) const { return v; } + + template ::value, bool> = true> + std::unordered_map + operator()(std::unordered_map const &v) const { + std::unordered_map ret; + for (auto it = v.begin(); it != v.end(); ++it) { + ret.insert({it->first, get_value_helper{}(it->second)}); + } + return ret; + } }; /* std::unordered_map cases */ -template <> struct get_value_helper, void> { - std::unordered_map operator()(Variant const &v) const { - return boost::apply_visitor(GetMapOrEmpty{}, v); +template +struct get_value_helper, void> { + std::unordered_map operator()(Variant const &v) const { + return boost::apply_visitor(GetMapOrEmpty{}, v); } }; @@ -213,24 +269,27 @@ struct get_value_helper< * @brief Re-throw a @c boost::bad_get exception wrapped in an @ref Exception. * Write a custom error message for invalid conversions due to type mismatch * and due to nullptr values, possibly with context information if the variant - * is in a container. - * @tparam T Type which the variant was supposed to convert to - * @tparam U Type of the container the variant is contained in + * is a container. + * @tparam T Which type the variant was supposed to convert to */ -template -inline void handle_bad_get(Variant const &v) { - using Utils::demangle; - auto const what = "Provided argument of type " + type_label(v); - auto const context = - (std::is_void::value) - ? "" - : " (raised during the creation of a " + demangle() + ")"; +template inline void handle_bad_get(Variant const &v) { + auto const object_symbol_name = boost::apply_visitor(type_label_visitor{}, v); + auto const element_symbol_name = demangle_container_value_type{}(); + auto const is_container = !element_symbol_name.empty(); + auto const what = "Provided argument of type " + object_symbol_name; try { throw; } catch (bad_get_nullptr const &) { - throw Exception(what + " is a null pointer" + context); + auto const item_error = (is_container) ? " contains a value that" : ""; + throw Exception(what + item_error + " is a null pointer"); } catch (boost::bad_get const &) { - throw Exception(what + " is not convertible to " + demangle() + context); + auto const non_convertible = " is not convertible to "; + auto item_error = std::string(""); + if (is_container) { + item_error += " because it contains a value that"; + item_error += non_convertible + element_symbol_name; + } + throw Exception(what + non_convertible + Utils::demangle() + item_error); } } @@ -241,9 +300,9 @@ inline void handle_bad_get(Variant const &v) { * * This is a wrapper around boost::get that allows us to * customize the behavior for different types. This is - * needed e.g. to deal with Vector types that are not - * explicitly contained in Variant, but can easily - * be converted. + * needed e.g. to deal with containers whose elements + * have mixed types that are implicitly convertible + * to a requested type. */ template T get_value(Variant const &v) { try { @@ -255,22 +314,7 @@ template T get_value(Variant const &v) { } template -std::unordered_map get_map(std::unordered_map const &v) { - std::unordered_map ret; - auto it = v.begin(); - try { - for (; it != v.end(); ++it) { - ret.insert({it->first, detail::get_value_helper{}(it->second)}); - } - } catch (...) { - detail::handle_bad_get>(v); - throw; - } - return ret; -} - -template -std::unordered_map make_map(std::unordered_map const &v) { +auto make_unordered_map_of_variants(std::unordered_map const &v) { std::unordered_map ret; for (auto const &it : v) { ret.insert({it.first, Variant(it.second)}); diff --git a/src/script_interface/reaction_methods/ConstantpHEnsemble.hpp b/src/script_interface/reaction_methods/ConstantpHEnsemble.hpp index 834bf651062..47c0a672e3f 100644 --- a/src/script_interface/reaction_methods/ConstantpHEnsemble.hpp +++ b/src/script_interface/reaction_methods/ConstantpHEnsemble.hpp @@ -28,6 +28,7 @@ #include "core/reaction_methods/ReactionAlgorithm.hpp" #include +#include namespace ScriptInterface { namespace ReactionMethods { @@ -53,8 +54,8 @@ class ConstantpHEnsemble : public ReactionAlgorithm { get_value(params, "seed"), get_value(params, "kT"), get_value(params, "exclusion_range"), get_value(params, "constant_pH"), - get_map(get_value_or>( - params, "exclusion_radius_per_type", {}))); + get_value_or>( + params, "exclusion_radius_per_type", {})); } private: diff --git a/src/script_interface/reaction_methods/ReactionAlgorithm.hpp b/src/script_interface/reaction_methods/ReactionAlgorithm.hpp index 1b5868fe461..13f990207ea 100644 --- a/src/script_interface/reaction_methods/ReactionAlgorithm.hpp +++ b/src/script_interface/reaction_methods/ReactionAlgorithm.hpp @@ -29,6 +29,7 @@ #include #include #include +#include #include namespace ScriptInterface { @@ -70,10 +71,13 @@ class ReactionAlgorithm : public AutoParameters { [this]() { return RE()->get_exclusion_range(); }}, {"exclusion_radius_per_type", [this](Variant const &v) { - RE()->exclusion_radius_per_type = get_map( - get_value>(v)); + RE()->set_exclusion_radius_per_type( + get_value>(v)); }, - [this]() { return make_map(RE()->exclusion_radius_per_type); }}}); + [this]() { + return make_unordered_map_of_variants( + RE()->exclusion_radius_per_type); + }}}); } Variant do_call_method(std::string const &name, diff --git a/src/script_interface/reaction_methods/ReactionEnsemble.hpp b/src/script_interface/reaction_methods/ReactionEnsemble.hpp index 03b66128420..20478e1e8b6 100644 --- a/src/script_interface/reaction_methods/ReactionEnsemble.hpp +++ b/src/script_interface/reaction_methods/ReactionEnsemble.hpp @@ -28,6 +28,7 @@ #include "core/reaction_methods/ReactionEnsemble.hpp" #include +#include namespace ScriptInterface { namespace ReactionMethods { @@ -39,12 +40,11 @@ class ReactionEnsemble : public ReactionAlgorithm { } void do_construct(VariantMap const ¶ms) override { - m_re = std::make_shared<::ReactionMethods::ReactionEnsemble>( get_value(params, "seed"), get_value(params, "kT"), get_value(params, "exclusion_range"), - get_map(get_value_or>( - params, "exclusion_radius_per_type", {}))); + get_value_or>( + params, "exclusion_radius_per_type", {})); } private: diff --git a/src/script_interface/reaction_methods/WidomInsertion.hpp b/src/script_interface/reaction_methods/WidomInsertion.hpp index f563047bd9e..343679a0184 100644 --- a/src/script_interface/reaction_methods/WidomInsertion.hpp +++ b/src/script_interface/reaction_methods/WidomInsertion.hpp @@ -41,10 +41,9 @@ class WidomInsertion : public ReactionAlgorithm { } void do_construct(VariantMap const ¶ms) override { - std::unordered_map exclusion_radius_per_type; m_re = std::make_shared<::ReactionMethods::WidomInsertion>( get_value(params, "seed"), get_value(params, "kT"), 0., - exclusion_radius_per_type); + std::unordered_map{}); } Variant do_call_method(std::string const &name, diff --git a/src/script_interface/tests/get_value_test.cpp b/src/script_interface/tests/get_value_test.cpp index d6d346ace08..3410e3fd672 100644 --- a/src/script_interface/tests/get_value_test.cpp +++ b/src/script_interface/tests/get_value_test.cpp @@ -24,6 +24,7 @@ #include "script_interface/ObjectHandle.hpp" #include "script_interface/get_value.hpp" +#include #include #include #include @@ -127,67 +128,112 @@ BOOST_AUTO_TEST_CASE(get_value_from_map) { BOOST_CHECK_THROW((get_value(map, "unknown")), std::exception); } -BOOST_AUTO_TEST_CASE(get_map_value) { - using ScriptInterface::get_map; - using ScriptInterface::Variant; - - std::unordered_map const map_variant{{1, 1.5}, {2, 2.5}}; - std::unordered_map const map = get_map(map_variant); - BOOST_CHECK_EQUAL(map.at(1), 1.5); - BOOST_CHECK_EQUAL(map.at(2), 2.5); -} - -BOOST_AUTO_TEST_CASE(get_unordered_map) { +BOOST_AUTO_TEST_CASE(unordered_map) { using ScriptInterface::get_value; + using ScriptInterface::make_unordered_map_of_variants; using ScriptInterface::Variant; auto const var = Variant{std::unordered_map{{1, 1}, {2, 2.5}}}; - auto const map = get_value>(var); - BOOST_CHECK_EQUAL(get_value(map.at(1)), 1); - BOOST_CHECK_EQUAL(get_value(map.at(2)), 2.5); + { + auto const map_var = get_value>(var); + BOOST_CHECK_EQUAL(get_value(map_var.at(1)), 1); + BOOST_CHECK_EQUAL(get_value(map_var.at(2)), 2.5); + } + { + auto const map_dbl = get_value>(var); + BOOST_CHECK_EQUAL(map_dbl.at(1), 1.0); + BOOST_CHECK_EQUAL(map_dbl.at(2), 2.5); + } + { + auto const map_dbl_input = get_value>(var); + auto const map_var = make_unordered_map_of_variants(map_dbl_input); + auto const map_dbl = + get_value>(Variant{map_var}); + BOOST_CHECK_EQUAL(map_dbl.at(1), 1.0); + BOOST_CHECK_EQUAL(map_dbl.at(2), 2.5); + } +} + +auto exception_message_predicate(std::string const &pattern) { + return [=](std::exception const &ex) { + boost::test_tools::predicate_result result = true; + std::string const what = ex.what(); + std::smatch match; + if (!std::regex_search(what, match, std::regex(pattern))) { + result = false; + result.message() << "Error message \"" << what << "\" " + << "doesn't match pattern \"" << pattern << "\""; + } + return result; + }; } -BOOST_AUTO_TEST_CASE(exceptions) { - using ScriptInterface::get_map; +BOOST_AUTO_TEST_CASE(check_exceptions) { using ScriptInterface::get_value; using ScriptInterface::Variant; + assert(!!exception_message_predicate("A")(std::runtime_error("A"))); + assert(!exception_message_predicate("A")(std::runtime_error("B"))); + using so_ptr_t = std::shared_ptr; auto const so_obj = so_ptr_t(); - auto const so_ptr_tn = Utils::demangle(); + auto const msg_prefix = std::string("Provided argument of type "); + auto const variant_name = std::string("ScriptInterface::Variant"); { - auto const predicate = [](std::string const &type, std::string const &why) { - auto const message = "Provided argument of type " + type + " is " + why; - return [=](std::exception const &ex) { return ex.what() == message; }; - }; - auto const so_variant = Variant(so_obj); - BOOST_CHECK_EXCEPTION((get_value(so_variant)), std::exception, - predicate(so_ptr_tn, "a null pointer")); - BOOST_CHECK_EXCEPTION((get_value(so_variant)), std::exception, - predicate(so_ptr_tn, "not convertible to int")); + // basic types + auto const obj_variant = Variant{so_obj}; + auto const obj_variant_pattern = Utils::demangle(); + auto const what = msg_prefix + obj_variant_pattern; + auto const predicate_nullptr = + exception_message_predicate(what + " is a null pointer"); + auto const predicate_conversion = + exception_message_predicate(what + " is not convertible to int"); + BOOST_CHECK_EXCEPTION(get_value(obj_variant), std::exception, + predicate_nullptr); + BOOST_CHECK_EXCEPTION(get_value(obj_variant), std::exception, + predicate_conversion); + } + { + // vectors + auto const vec_variant = Variant{std::vector{{so_obj}}}; + auto const vec_variant_pattern = + "std::(__1::)?vector<" + variant_name + ", .*?>"; + auto const what = msg_prefix + vec_variant_pattern; + auto const predicate_nullptr = exception_message_predicate( + what + " contains a value that is a null pointer"); + auto const predicate_conversion = exception_message_predicate( + what + " is not convertible to std::(__1::)?vector because " + "it contains a value that is not convertible to int"); + BOOST_CHECK_EXCEPTION(get_value>(vec_variant), + std::exception, predicate_nullptr); + BOOST_CHECK_EXCEPTION(get_value>(vec_variant), + std::exception, predicate_conversion); } { - auto const predicate = [](std::string const &why, std::string const &type) { - auto const msg = "during the creation of a std::(__1::)?unordered_map"; - auto const pattern = why + " \\(raised " + msg + "{{1, so_obj}}; - BOOST_CHECK_EXCEPTION((get_map(so_map)), std::exception, - predicate("is a null pointer", so_ptr_tn)); - BOOST_CHECK_EXCEPTION((get_map(so_map)), std::exception, - predicate("is not convertible to int", "int")); + // unordered maps + auto const map_variant = + Variant{std::unordered_map{{1, so_obj}}}; + auto const map_variant_pattern = + "std::(__1::)?unordered_map"; + auto const what = msg_prefix + map_variant_pattern; + auto const predicate_nullptr = exception_message_predicate( + what + " contains a value that is a null pointer"); + auto const predicate_conversion = exception_message_predicate( + what + + " is not convertible to std::(__1::)?unordered_map " + "because it contains a value that is not convertible to int"); + BOOST_CHECK_EXCEPTION( + (get_value>(map_variant)), + std::exception, predicate_nullptr); + BOOST_CHECK_EXCEPTION( + (get_value>(map_variant)), std::exception, + predicate_conversion); } { using Utils::Vector3d; std::unordered_map const mixed{{1, 1}, {2, std::string("2")}}; - BOOST_CHECK_THROW((get_map(mixed)), std::exception); std::vector const v_var = {1., 2.}; std::vector const v_dbl = {1., 2.}; BOOST_CHECK_THROW((get_value(Variant{v_var})), std::exception); @@ -199,5 +245,7 @@ BOOST_AUTO_TEST_CASE(exceptions) { BOOST_CHECK_THROW((get_value(Variant{v_dbl})), std::exception); BOOST_CHECK_THROW((get_value>(Variant{})), std::exception); + BOOST_CHECK_THROW((get_value>(Variant{mixed})), + std::exception); } } diff --git a/testsuite/python/CMakeLists.txt b/testsuite/python/CMakeLists.txt index 77cd2112bec..6b91021da26 100644 --- a/testsuite/python/CMakeLists.txt +++ b/testsuite/python/CMakeLists.txt @@ -14,7 +14,7 @@ function(PYTHON_TEST) set(TEST_FILE ${TEST_FILE_CONFIGURED}) if(NOT DEFINED TEST_MAX_NUM_PROC) - set(TEST_MAX_NUM_PROC 1) + set(TEST_MAX_NUM_PROC 4) endif() if(${TEST_MAX_NUM_PROC} GREATER ${TEST_NP}) @@ -61,20 +61,20 @@ function(CHECKPOINT_TEST) endif() if(TEST_SUFFIX) set(TEST_ARGUMENTS "Test_suffix_${TEST_SUFFIX}__${TEST_MODES}") - set(TEST_SUFFIX "${TEST_MODES}_${TEST_SUFFIX}") + set(TEST_SUFFIX "_${TEST_MODES}_${TEST_SUFFIX}") else() set(TEST_ARGUMENTS "Test__${TEST_MODES}") - set(TEST_SUFFIX "${TEST_MODES}") + set(TEST_SUFFIX "_${TEST_MODES}") endif() python_test( - FILE save_checkpoint.py MAX_NUM_PROC ${TEST_NPROCS} LABELS ${TEST_LABELS} - SUFFIX ${TEST_SUFFIX} ARGUMENTS ${TEST_ARGUMENTS} DEPENDENCIES - unittest_generator.py) + FILE save_checkpoint.py MAX_NUM_PROC ${TEST_MAX_NUM_PROC} LABELS + ${TEST_LABELS} SUFFIX ${TEST_SUFFIX} ARGUMENTS ${TEST_ARGUMENTS} + DEPENDENCIES unittest_generator.py) python_test( FILE test_checkpoint.py MAX_NUM_PROC - ${TEST_NPROCS} + ${TEST_MAX_NUM_PROC} LABELS ${TEST_LABELS} SUFFIX @@ -127,6 +127,7 @@ python_test(FILE mass-and-rinertia_per_particle.py MAX_NUM_PROC 2 LABELS long) python_test(FILE integrate.py MAX_NUM_PROC 4) python_test(FILE interactions_bond_angle.py MAX_NUM_PROC 4) python_test(FILE interactions_bonded_interface.py MAX_NUM_PROC 4) +python_test(FILE interactions_bonded_interface.py MAX_NUM_PROC 1 SUFFIX 1_core) python_test(FILE interactions_bonded.py MAX_NUM_PROC 2) python_test(FILE interactions_dihedral.py MAX_NUM_PROC 4) python_test(FILE interactions_non-bonded_interface.py MAX_NUM_PROC 4) @@ -264,6 +265,7 @@ python_test(FILE elc_vs_analytic.py MAX_NUM_PROC 2) python_test(FILE rotation.py MAX_NUM_PROC 1) python_test(FILE shapes.py MAX_NUM_PROC 1) python_test(FILE h5md.py MAX_NUM_PROC 2) +python_test(FILE h5md.py MAX_NUM_PROC 1 SUFFIX 1_core) python_test(FILE mdanalysis.py MAX_NUM_PROC 2) python_test(FILE p3m_fft.py MAX_NUM_PROC 6) if(${TEST_NP} GREATER_EQUAL 8) diff --git a/testsuite/python/field_test.py b/testsuite/python/field_test.py index 327b8ebaaf9..7f80073756c 100644 --- a/testsuite/python/field_test.py +++ b/testsuite/python/field_test.py @@ -149,11 +149,10 @@ def test_homogeneous_flow_field(self): def test_potential_field(self): h = np.array([.2, .2, .2]) - box = np.array([10., 10., 10.]) scaling = 2.6 field_data = espressomd.constraints.PotentialField.field_from_fn( - box, h, self.potential) + self.system.box_l, h, self.potential) F = espressomd.constraints.PotentialField( field=field_data, @@ -186,10 +185,9 @@ def test_potential_field(self): @utx.skipIfMissingFeatures("ELECTROSTATICS") def test_electric_potential_field(self): h = np.array([.2, .2, .2]) - box = np.array([10., 10., 10.]) field_data = espressomd.constraints.ElectricPotential.field_from_fn( - box, h, self.potential) + self.system.box_l, h, self.potential) F = espressomd.constraints.ElectricPotential( field=field_data, grid_spacing=h) @@ -213,11 +211,10 @@ def test_electric_potential_field(self): def test_force_field(self): h = np.array([.8, .8, .8]) - box = np.array([10., 10., 10.]) scaling = 2.6 field_data = espressomd.constraints.ForceField.field_from_fn( - box, h, self.force) + self.system.box_l, h, self.force) F = espressomd.constraints.ForceField( field=field_data, @@ -245,11 +242,10 @@ def test_force_field(self): def test_flow_field(self): h = np.array([.8, .8, .8]) - box = np.array([10., 10., 10.]) gamma = 2.6 field_data = espressomd.constraints.FlowField.field_from_fn( - box, h, self.force) + self.system.box_l, h, self.force) F = espressomd.constraints.FlowField( field=field_data, @@ -271,6 +267,23 @@ def test_flow_field(self): np.testing.assert_allclose( -gamma * (p.v - f_val), np.copy(p.f), atol=1e-12) + def test_invalid_interpolated_field(self): + h = np.array([1., 1., 1.]) + Field = espressomd.constraints.FlowField + + grid = espressomd.constraints.FlowField.field_from_fn( + self.system.box_l, h, self.force) + + err_msg = "Constraint not compatible with box size" + with self.assertRaisesRegex(RuntimeError, err_msg): + field = Field(field=grid[:, :, :-2], grid_spacing=h, gamma=1.) + self.system.constraints.add(field) + with self.assertRaisesRegex(RuntimeError, err_msg): + field = Field(field=grid[:, 2:, :], grid_spacing=h, gamma=1.) + self.system.constraints.add(field) + + self.assertEqual(len(self.system.constraints), 0) + if __name__ == "__main__": ut.main() diff --git a/testsuite/python/h5md.py b/testsuite/python/h5md.py index de8a7427e89..8bf74639585 100644 --- a/testsuite/python/h5md.py +++ b/testsuite/python/h5md.py @@ -28,6 +28,7 @@ import espressomd import espressomd.interactions import espressomd.io.writer +import tempfile try: import h5py # h5py has to be imported *after* espressomd (MPI) skipIfMissingPythonPackage = utx.no_skip @@ -46,18 +47,15 @@ class H5mdTests(ut.TestCase): Test the core implementation of writing hdf5 files. """ - system = espressomd.System(box_l=[1.0, 1.0, 1.0]) - # avoid particles to be set outside of the main box, otherwise particle - # positions are folded in the core when writing out and we cannot directly - # compare positions in the dataset and where particles were set. One would - # need to unfold the positions of the hdf5 file. - box_l = N_PART / 2.0 - system.box_l = [box_l, box_l, box_l] + box_l = N_PART // 2 + box_l = [box_l, box_l + 1, box_l + 2] + system = espressomd.System(box_l=box_l) system.cell_system.skin = 0.4 system.time_step = 0.01 + # set particles outside the main box to verify position folding for i in range(N_PART): - p = system.part.add(id=i, pos=np.array(3 * [i], dtype=float), + p = system.part.add(id=i, pos=np.array(3 * [i - 4], dtype=float), v=np.array([1.0, 2.0, 3.0]), type=23) if espressomd.has_features(['MASS']): p.mass = 2.3 @@ -74,20 +72,22 @@ class H5mdTests(ut.TestCase): system.integrator.run(steps=0) system.time = 12.3 + n_nodes = system.cell_system.get_state()["n_nodes"] @classmethod def setUpClass(cls): - if os.path.isfile('test.h5'): - os.remove('test.h5') + cls.temp_dir = tempfile.TemporaryDirectory() + cls.temp_file = os.path.join(cls.temp_dir.name, 'test.h5') h5_units = espressomd.io.writer.h5md.UnitSystem( time='ps', mass='u', length='m', charge='e') h5 = espressomd.io.writer.h5md.H5md( - file_path="test.h5", unit_system=h5_units) + file_path=cls.temp_file, unit_system=h5_units) h5.write() h5.write() h5.flush() h5.close() - cls.py_file = h5py.File("test.h5", 'r') + cls.h5_params = h5.get_params() + cls.py_file = h5py.File(cls.temp_file, 'r') cls.py_pos = cls.py_file['particles/atoms/position/value'][1] cls.py_img = cls.py_file['particles/atoms/image/value'][1] cls.py_mass = cls.py_file['particles/atoms/mass/value'][1] @@ -102,12 +102,29 @@ def setUpClass(cls): @classmethod def tearDownClass(cls): - os.remove("test.h5") + cls.temp_dir.cleanup() def test_opening(self): - h5 = espressomd.io.writer.h5md.H5md(file_path="test.h5") + h5 = espressomd.io.writer.h5md.H5md(file_path=self.temp_file) h5.close() + @ut.skipIf(n_nodes > 1, "only runs for 1 MPI rank") + def test_exceptions(self): + h5md = espressomd.io.writer.h5md + h5_units = h5md.UnitSystem(time='ps', mass='u', length='m', charge='e') + temp_file = os.path.join(self.temp_dir.name, 'exceptions.h5') + # write a non-compliant file + with open(temp_file, 'w') as _: + pass + with self.assertRaisesRegex(RuntimeError, 'not a valid HDF5 file'): + h5md.H5md(file_path=temp_file, unit_system=h5_units) + # write a leftover backup file + os.remove(temp_file) + with open(temp_file + '.bak', 'w') as _: + pass + with self.assertRaisesRegex(RuntimeError, 'A backup of the .h5 file exists'): + h5md.H5md(file_path=temp_file, unit_system=h5_units) + def test_box(self): np.testing.assert_allclose(self.py_box, self.box_l) @@ -126,9 +143,10 @@ def test_metadata(self): def test_pos(self): """Test if positions have been written properly.""" - np.testing.assert_allclose( - np.array([3 * [float(i) % self.box_l] for i in range(N_PART)]), - np.array([x for (_, x) in sorted(zip(self.py_id, self.py_pos))])) + pos_ref = np.outer(np.arange(N_PART) - 4, [1, 1, 1]) + pos_ref = np.mod(pos_ref, self.box_l) + pos_read = [x for (_, x) in sorted(zip(self.py_id, self.py_pos))] + np.testing.assert_allclose(pos_read, pos_ref) def test_time(self): """Test for time dataset.""" @@ -136,11 +154,10 @@ def test_time(self): def test_img(self): """Test if images have been written properly.""" - images = np.append(np.zeros((int(N_PART / 2), 3)), - np.ones((int(N_PART / 2), 3))) - images = images.reshape(N_PART, 3) - np.testing.assert_allclose( - [x for (_, x) in sorted(zip(self.py_id, self.py_img))], images) + pos_ref = np.outer(np.arange(N_PART) - 4, [1, 1, 1]) + images_ref = np.floor_divide(pos_ref, self.box_l) + images_read = [x for (_, x) in sorted(zip(self.py_id, self.py_img))] + np.testing.assert_allclose(images_read, images_ref) @utx.skipIfMissingFeatures("MASS") def test_mass(self): @@ -185,22 +202,28 @@ def test_script(self): self.assertEqual(data, ref) def test_units(self): - self.assertEqual( - self.py_file['particles/atoms/id/time'].attrs['unit'], b'ps') - self.assertEqual( - self.py_file['particles/atoms/position/value'].attrs['unit'], b'm') + def get_unit(path): + return self.py_file[path].attrs['unit'] + self.assertEqual(get_unit('particles/atoms/id/time'), b'ps') + self.assertEqual(get_unit('particles/atoms/position/value'), b'm') if espressomd.has_features(['ELECTROSTATICS']): - self.assertEqual( - self.py_file['particles/atoms/charge/value'].attrs['unit'], b'e') + self.assertEqual(get_unit('particles/atoms/charge/value'), b'e') if espressomd.has_features(['MASS']): - self.assertEqual( - self.py_file['particles/atoms/mass/value'].attrs['unit'], b'u') - self.assertEqual( - self.py_file['particles/atoms/force/value'].attrs['unit'], - b'm u ps-2') - self.assertEqual( - self.py_file['particles/atoms/velocity/value'].attrs['unit'], - b'm ps-1') + self.assertEqual(get_unit('particles/atoms/mass/value'), b'u') + self.assertEqual(get_unit('particles/atoms/force/value'), b'm u ps-2') + self.assertEqual(get_unit('particles/atoms/velocity/value'), b'm ps-1') + + def test_getters(self): + self.assertEqual(self.h5_params['file_path'], self.temp_file) + self.assertEqual(os.path.abspath(self.h5_params['script_path']), + os.path.abspath(__file__)) + self.assertEqual(self.h5_params['time_unit'], 'ps') + if espressomd.has_features(['ELECTROSTATICS']): + self.assertEqual(self.h5_params['charge_unit'], 'e') + if espressomd.has_features(['MASS']): + self.assertEqual(self.h5_params['mass_unit'], 'u') + self.assertEqual(self.h5_params['force_unit'], 'm u ps-2') + self.assertEqual(self.h5_params['velocity_unit'], 'm ps-1') def test_links(self): time_ref = self.py_id_time diff --git a/testsuite/python/integrator_exceptions.py b/testsuite/python/integrator_exceptions.py index 08360150939..1f697854bfe 100644 --- a/testsuite/python/integrator_exceptions.py +++ b/testsuite/python/integrator_exceptions.py @@ -17,6 +17,9 @@ # along with this program. If not, see . # import espressomd +import espressomd.interactions +import espressomd.lees_edwards +import espressomd.shapes import unittest as ut import unittest_decorators as utx @@ -25,7 +28,6 @@ class Integrator_test(ut.TestCase): system = espressomd.System(box_l=[1., 1., 1.]) system.time_step = 0.01 - system.cell_system.skin = 0.4 msg = 'Encountered errors during integrate: ERROR: ' def setUp(self): @@ -36,27 +38,67 @@ def setUp(self): def tearDown(self): self.system.thermostat.turn_off() self.system.part.clear() + self.system.constraints.clear() + self.system.lees_edwards.protocol = None + + def test_00_common_interface(self): + self.system.integrator.set_vv() + with self.assertRaisesRegex(ValueError, 'time_step must be > 0.'): + self.system.time_step = -2. + with self.assertRaisesRegex(ValueError, 'time_step must be > 0.'): + self.system.time_step = 0. + with self.assertRaisesRegex(ValueError, 'steps must be positive'): + self.system.integrator.run(steps=-1) + with self.assertRaisesRegex(Exception, self.msg + 'cannot automatically determine skin, please set it manually'): + self.system.integrator.run() + self.system.cell_system.skin = 0.4 + with self.assertRaisesRegex(Exception, self.msg + 'cannot reuse old forces and recalculate forces'): + self.system.integrator.run(recalc_forces=True, reuse_forces=True) + if espressomd.has_features("WCA"): + wca = self.system.non_bonded_inter[0, 0].wca + wca.set_params(epsilon=1., sigma=0.01) + wall = espressomd.shapes.Wall(normal=[0., 0., 1.], dist=100.) + self.system.constraints.add( + shape=wall, particle_type=0, penetrable=False) + with self.assertRaisesRegex(Exception, self.msg + 'Constraint violated by particle 0 dist -100'): + self.system.integrator.run(0) + with self.assertRaisesRegex(Exception, 'Constraint violated by particle 0'): + self.system.analysis.energy() + wca.set_params(epsilon=0., sigma=0.) def test_vv_integrator(self): + self.system.cell_system.skin = 0.4 self.system.thermostat.set_brownian(kT=1.0, gamma=1.0, seed=42) self.system.integrator.set_vv() with self.assertRaisesRegex(Exception, self.msg + 'The VV integrator is incompatible with the currently active combination of thermostats'): self.system.integrator.run(0) def test_brownian_integrator(self): + self.system.cell_system.skin = 0.4 self.system.integrator.set_brownian_dynamics() with self.assertRaisesRegex(Exception, self.msg + 'The BD integrator requires the BD thermostat'): self.system.integrator.run(0) @utx.skipIfMissingFeatures("NPT") def test_npt_integrator(self): + self.system.cell_system.skin = 0.4 self.system.thermostat.set_brownian(kT=1.0, gamma=1.0, seed=42) self.system.integrator.set_isotropic_npt(ext_pressure=1.0, piston=1.0) with self.assertRaisesRegex(Exception, self.msg + 'The NpT integrator requires the NpT thermostat'): self.system.integrator.run(0) + self.system.thermostat.turn_off() + self.system.thermostat.set_npt(kT=1.0, gamma0=2., gammav=0.04, seed=42) + with self.assertRaisesRegex(Exception, self.msg + 'The NpT integrator cannot use Lees-Edwards'): + self.system.lees_edwards.protocol = espressomd.lees_edwards.LinearShear( + shear_velocity=1., initial_pos_offset=0., time_0=0.) + self.system.integrator.run(0) + with self.assertRaisesRegex(Exception, self.msg + 'The NpT integrator cannot use Lees-Edwards'): + self.system.lees_edwards.protocol = espressomd.lees_edwards.Off() + self.system.integrator.run(0) @utx.skipIfMissingFeatures("STOKESIAN_DYNAMICS") def test_stokesian_integrator(self): + self.system.cell_system.skin = 0.4 self.system.periodicity = 3 * [False] self.system.thermostat.set_langevin(kT=1.0, gamma=1.0, seed=42) self.system.integrator.set_stokesian_dynamics( @@ -65,6 +107,7 @@ def test_stokesian_integrator(self): self.system.integrator.run(0) def test_steepest_descent_integrator(self): + self.system.cell_system.skin = 0.4 with self.assertRaisesRegex(ValueError, r"The following keys have to be given as keyword arguments: " r"\['f_max', 'gamma', 'max_displacement'\], got " r"\['f_max', 'gamma', 'max_d'\] \(missing \['max_displacement'\]\)"): diff --git a/testsuite/python/interactions_bonded_interface.py b/testsuite/python/interactions_bonded_interface.py index dda1e79f3f4..d01069639e3 100644 --- a/testsuite/python/interactions_bonded_interface.py +++ b/testsuite/python/interactions_bonded_interface.py @@ -279,6 +279,52 @@ def test_exceptions(self): with self.assertRaisesRegex(ValueError, error_msg_not_yet_defined): self.system.bonded_inter[0] + # check cutoff exceptions + skin = self.system.cell_system.skin + box_l = self.system.box_l + node_grid = self.system.cell_system.node_grid + n_nodes = self.system.cell_system.get_state()['n_nodes'] + max_ia_cutoff = min(box_l / node_grid) - skin * (n_nodes > 1) + err_msg = r"while calling method insert\(\): ERROR: " + if n_nodes == 1: + safe_cut = 0.5 * max_ia_cutoff + err_msg += r"number of cells 1 is smaller than minimum 8" + else: + safe_cut = 1.0 * max_ia_cutoff + err_msg += r"interaction range .+ in direction [0-2] is larger than the local box size" + + # a bond with exactly the right cutoff can be added + h0 = espressomd.interactions.HarmonicBond(k=1., r_0=0., r_cut=safe_cut) + self.system.bonded_inter.add(h0) + self.assertEqual(len(self.system.bonded_inter), 1) + self.system.bonded_inter.clear() + + # a bond with a cutoff larger than the box can be added, but throws + big_cut = 1.001 * safe_cut + h1 = espressomd.interactions.HarmonicBond(k=1., r_0=0., r_cut=big_cut) + with self.assertRaisesRegex(Exception, err_msg): + self.system.bonded_inter.add(h1) + self.assertEqual(len(self.system.bonded_inter), 1) + self.system.bonded_inter.clear() + + # a dihedral halves the cutoff + safe_cut /= 2. + h2 = espressomd.interactions.HarmonicBond(k=1., r_0=0., r_cut=safe_cut) + self.system.bonded_inter.add(h2) + dihe = espressomd.interactions.Dihedral(bend=1., mult=1, phase=0.) + self.system.bonded_inter.add(dihe) + self.assertEqual(len(self.system.bonded_inter), 2) + self.system.bonded_inter.clear() + + # a dihedral halves the cutoff, safe cutoffs become unsafe + half_cut = big_cut / 2. + h3 = espressomd.interactions.HarmonicBond(k=1., r_0=0., r_cut=half_cut) + self.system.bonded_inter.add(h3) + with self.assertRaisesRegex(Exception, err_msg): + self.system.bonded_inter.add(dihe) + self.assertEqual(len(self.system.bonded_inter), 2) + self.system.bonded_inter.clear() + if __name__ == "__main__": ut.main() diff --git a/testsuite/python/interactions_non-bonded_interface.py b/testsuite/python/interactions_non-bonded_interface.py index d893f28ef79..59e9ebdb14d 100644 --- a/testsuite/python/interactions_non-bonded_interface.py +++ b/testsuite/python/interactions_non-bonded_interface.py @@ -25,7 +25,12 @@ class Non_bonded_interactionsTests(ut.TestCase): - system = espressomd.System(box_l=[20.0, 20.0, 20.0]) + system = espressomd.System(box_l=[30.0, 30.0, 30.0]) + + def tearDown(self): + if espressomd.has_features(["LENNARD_JONES"]): + self.system.non_bonded_inter[0, 0].lennard_jones.set_params( + epsilon=0., sigma=0., cutoff=0., shift=0.) def intersMatch(self, inType, outInter, inParams, outParams, msg_long): """Check, if the interaction type set and gotten back as well as the @@ -178,6 +183,19 @@ def test_exceptions(self): self.system.non_bonded_inter[0, 0].lennard_jones.set_params( epsilon=1., sigma=2., cutoff=3., shift=4., unknown=5.) + skin = self.system.cell_system.skin + box_l = self.system.box_l + node_grid = self.system.cell_system.node_grid + n_nodes = self.system.cell_system.get_state()['n_nodes'] + max_ia_cutoff = min(box_l / node_grid) - skin * (n_nodes > 1) + wrong_cutoff = 1.01 * max_ia_cutoff + lennard_jones = self.system.non_bonded_inter[0, 0].lennard_jones + with self.assertRaisesRegex(Exception, "setting LennardJones raised an error: ERROR: interaction range .+ in direction [0-2] is larger than the local box size"): + lennard_jones.set_params( + epsilon=1., sigma=1., cutoff=wrong_cutoff, shift="auto") + self.assertAlmostEqual( + lennard_jones.get_params()['cutoff'], wrong_cutoff, delta=1e-10) + if __name__ == "__main__": ut.main() diff --git a/testsuite/python/lb_vtk.py b/testsuite/python/lb_vtk.py index 7ed32ebef82..2e4e28f8088 100644 --- a/testsuite/python/lb_vtk.py +++ b/testsuite/python/lb_vtk.py @@ -56,7 +56,7 @@ def set_lbf(self): self.system.lbboundaries.add(espressomd.lbboundaries.LBBoundary( shape=espressomd.shapes.Wall(normal=[1, 0, 0], dist=1.5))) self.system.lbboundaries.add(espressomd.lbboundaries.LBBoundary( - shape=espressomd.shapes.Wall(normal=[-1, 0, 0], dist=-10.5))) + shape=espressomd.shapes.Wall(normal=[-1, 0, 0], dist=-8.5))) return lbf def parse_vtk(self, filepath, name, shape): @@ -142,6 +142,8 @@ def test_vtk(self): vtk_boundary = self.parse_vtk( 'vtk_out/boundary.vtk', 'boundary', shape) np.testing.assert_equal(vtk_boundary, node_boundary.astype(int)) + if self.system.lbboundaries is None: + np.testing.assert_equal(np.sum(node_boundary), 0.) def test_print(self): ''' diff --git a/testsuite/python/particle.py b/testsuite/python/particle.py index 812b60e0d0c..e812848bcac 100644 --- a/testsuite/python/particle.py +++ b/testsuite/python/particle.py @@ -182,29 +182,33 @@ def test_gamma_rot_single(self): if espressomd.has_features(["VIRTUAL_SITES"]): test_virtual = generateTestForScalarProperty("virtual", 1) - if espressomd.has_features(["VIRTUAL_SITES_RELATIVE"]): - def test_yy_vs_relative(self): - self.system.part.add(id=0, pos=(0, 0, 0)) - p1 = self.system.part.add(id=1, pos=(0, 0, 0)) - p1.vs_relative = (0, 5.0, (0.5, -0.5, -0.5, -0.5)) - p1.vs_quat = [1, 2, 3, 4] - np.testing.assert_array_equal( - p1.vs_quat, [1, 2, 3, 4]) - res = p1.vs_relative - self.assertEqual(res[0], 0, "vs_relative: " + res.__str__()) - self.assertEqual(res[1], 5.0, "vs_relative: " + res.__str__()) - np.testing.assert_allclose( - res[2], np.array((0.5, -0.5, -0.5, -0.5)), - err_msg="vs_relative: " + res.__str__(), atol=self.tol) - # check exceptions - with self.assertRaisesRegex(ValueError, "needs input in the form"): - p1.vs_relative = (0, 5.0) - with self.assertRaisesRegex(ValueError, "particle id has to be given as an int"): - p1.vs_relative = ('0', 5.0, (1, 0, 0, 0)) - with self.assertRaisesRegex(ValueError, "distance has to be given as a float"): - p1.vs_relative = (0, '5', (1, 0, 0, 0)) - with self.assertRaisesRegex(ValueError, "quaternion has to be given as a tuple of 4 floats"): - p1.vs_relative = (0, 5.0, (1, 0, 0)) + + @utx.skipIfMissingFeatures(["VIRTUAL_SITES_RELATIVE"]) + def test_vs_relative(self): + self.system.part.add(id=0, pos=(0, 0, 0)) + p1 = self.system.part.add(id=1, pos=(0, 0, 0)) + p1.vs_relative = (0, 5.0, (0.5, -0.5, -0.5, -0.5)) + p1.vs_quat = [1, 2, 3, 4] + np.testing.assert_array_equal(p1.vs_quat, [1, 2, 3, 4]) + res = p1.vs_relative + self.assertEqual(res[0], 0, f"vs_relative: {res}") + self.assertEqual(res[1], 5.0, f"vs_relative: {res}") + np.testing.assert_allclose( + res[2], np.array((0.5, -0.5, -0.5, -0.5)), + err_msg=f"vs_relative: {res}", atol=self.tol) + # check exceptions + with self.assertRaisesRegex(ValueError, "needs input in the form"): + p1.vs_relative = (0, 5.0) + with self.assertRaisesRegex(ValueError, "particle id has to be given as an int"): + p1.vs_relative = ('0', 5.0, (1, 0, 0, 0)) + with self.assertRaisesRegex(ValueError, "distance has to be given as a float"): + p1.vs_relative = (0, '5', (1, 0, 0, 0)) + with self.assertRaisesRegex(ValueError, "quaternion has to be given as a tuple of 4 floats"): + p1.vs_relative = (0, 5.0, (1, 0, 0)) + with self.assertRaisesRegex(ValueError, "quaternion is zero"): + p1.vs_relative = (0, 5.0, (0, 0, 0, 0)) + with self.assertRaisesRegex(ValueError, "quaternion is zero"): + p1.vs_quat = [0, 0, 0, 0] @utx.skipIfMissingFeatures("DIPOLES") def test_contradicting_properties_dip_dipm(self): @@ -217,6 +221,12 @@ def test_contradicting_properties_dip_quat(self): self.system.part.add(pos=[0, 0, 0], dip=[1, 1, 1], quat=[1.0, 1.0, 1.0, 1.0]) + @utx.skipIfMissingFeatures(["ROTATION"]) + def test_invalid_quat(self): + system = self.system + with self.assertRaisesRegex(ValueError, "quaternion is zero"): + system.part.add(pos=[0., 0., 0.], quat=[0., 0., 0., 0.]) + @utx.skipIfMissingFeatures("ELECTROSTATICS") def test_particle_selection(self): self.system.part.clear() @@ -264,11 +274,21 @@ def test_image_box(self): np.testing.assert_equal(np.copy(p.image_box), [1, 1, 1]) - def test_accessing_invalid_id_raises(self): + def test_invalid_particle_ids_exceptions(self): self.system.part.clear() handle_to_non_existing_particle = self.system.part.by_id(42) - with self.assertRaises(RuntimeError): + with self.assertRaisesRegex(RuntimeError, "Particle node for id 42 not found"): handle_to_non_existing_particle.id + p = self.system.part.add(pos=[0., 0., 0.], id=0) + with self.assertRaisesRegex(RuntimeError, "Particle node for id 42 not found"): + p._id = 42 + p.node + for i in range(1, 10): + with self.assertRaisesRegex(ValueError, f"Invalid particle id: {-i}"): + p._id = -i + p.node + with self.assertRaisesRegex(ValueError, f"Invalid particle id: {-i}"): + self.system.part.add(pos=[0., 0., 0.], id=-i) def test_parallel_property_setters(self): s = self.system diff --git a/testsuite/python/random_pairs.py b/testsuite/python/random_pairs.py index 5c4b19d2e7c..2d9a31354ef 100644 --- a/testsuite/python/random_pairs.py +++ b/testsuite/python/random_pairs.py @@ -91,7 +91,7 @@ def check_n_squared(self, n2_pairs): def test(self): periods = [0, 1] - self.system.periodicity = True, True, True + self.system.periodicity = [True, True, True] check_non_bonded_loop_trace(self.system) for periodicity in itertools.product(periods, periods, periods): diff --git a/testsuite/python/reaction_methods.py b/testsuite/python/reaction_methods.py index 4a568aea974..f9076579d53 100644 --- a/testsuite/python/reaction_methods.py +++ b/testsuite/python/reaction_methods.py @@ -72,28 +72,20 @@ def check_reaction_parameters(reactions, parameters): method.exclusion_range, exclusion_range, delta=1e-10) - if exclusion_radius_per_type: # Avoid this test for widom method + if not isinstance(method, espressomd.reaction_ensemble.WidomInsertion): self.assertEqual( - list( - method.exclusion_radius_per_type.keys()), - [1]) + list(method.exclusion_radius_per_type.keys()), [1]) self.assertAlmostEqual( method.exclusion_radius_per_type[1], exclusion_radius_per_type[1], delta=1e-10) method.exclusion_radius_per_type = {2: 0.2} self.assertEqual( - list( - method.exclusion_radius_per_type.keys()), - [2]) + list(method.exclusion_radius_per_type.keys()), [2]) self.assertAlmostEqual( - method.exclusion_radius_per_type[2], - 0.2, - delta=1e-10) + method.exclusion_radius_per_type[2], 0.2, delta=1e-10) self.assertAlmostEqual( - method.get_volume(), - self.system.volume(), - delta=1e-10) + method.get_volume(), self.system.volume(), delta=1e-10) method.set_volume(volume=1.) self.assertAlmostEqual(method.get_volume(), 1., delta=1e-10) self.assertEqual(method.get_non_interacting_type(), 100) @@ -165,36 +157,23 @@ def check_reaction_parameters(reactions, parameters): self.assertEqual(len(method.reactions), 0) def test_interface(self): + params = {'exclusion_range': 0.8, + 'exclusion_radius_per_type': {1: 0.1}} + # reaction ensemble method = espressomd.reaction_ensemble.ReactionEnsemble( - kT=1.5, exclusion_range=0.8, seed=12, exclusion_radius_per_type={1: 0.1}) - self.check_interface( - method, - kT=1.5, - exclusion_range=0.8, - gamma=1.2, - exclusion_radius_per_type={ - 1: 0.1}) + kT=1.4, seed=12, **params) + self.check_interface(method, kT=1.4, gamma=1.2, **params) # constant pH ensemble method = espressomd.reaction_ensemble.ConstantpHEnsemble( - kT=1.5, exclusion_range=0.8, seed=12, constant_pH=10, exclusion_radius_per_type={1: 0.1}) - self.check_interface( - method, - kT=1.5, - exclusion_range=0.8, - gamma=1.2, - exclusion_radius_per_type={ - 1: 0.1}) + kT=1.5, seed=14, constant_pH=10, **params) + self.check_interface(method, kT=1.5, gamma=1.2, **params) # Widom insertion - method = espressomd.reaction_ensemble.WidomInsertion(kT=1.6, seed=12) - self.check_interface( - method, - kT=1.6, - exclusion_range=0., - gamma=1., - exclusion_radius_per_type={}) + method = espressomd.reaction_ensemble.WidomInsertion(kT=1.6, seed=16) + self.check_interface(method, kT=1.6, gamma=1., exclusion_range=0., + exclusion_radius_per_type={}) def test_exceptions(self): single_reaction_params = { @@ -290,19 +269,29 @@ def test_exceptions(self): x=1, **single_reaction_params) with self.assertRaisesRegex(ValueError, err_msg): espressomd.reaction_ensemble.ReactionEnsemble( - kT=1., exclusion_range=1., seed=12, x=1, exclusion_radius_per_type={1: 0.1}) + kT=1., exclusion_range=1., seed=12, x=1) with self.assertRaisesRegex(ValueError, err_msg): espressomd.reaction_ensemble.ConstantpHEnsemble( - kT=1., exclusion_range=1., seed=12, x=1, constant_pH=2, exclusion_radius_per_type={1: 0.1}) + kT=1., exclusion_range=1., seed=12, x=1, constant_pH=2) with self.assertRaisesRegex(ValueError, err_msg): espressomd.reaction_ensemble.WidomInsertion( kT=1., seed=12, x=1) with self.assertRaisesRegex(ValueError, "Invalid value for 'kT'"): espressomd.reaction_ensemble.ReactionEnsemble( - kT=-1., exclusion_range=1., seed=12, exclusion_radius_per_type={1: 0.1}) + kT=-1., exclusion_range=1., seed=12) + + # check invalid exclusion ranges and radii with self.assertRaisesRegex(ValueError, "Invalid value for 'exclusion_range'"): espressomd.reaction_ensemble.ReactionEnsemble( - kT=1., exclusion_range=-1., seed=12, exclusion_radius_per_type={1: 0.1}) + kT=1., seed=12, exclusion_range=-1.) + with self.assertRaisesRegex(ValueError, "Invalid excluded_radius value for type 1: radius -0.10"): + espressomd.reaction_ensemble.ReactionEnsemble( + kT=1., seed=12, exclusion_range=1., exclusion_radius_per_type={1: -0.1}) + method = espressomd.reaction_ensemble.ReactionEnsemble( + kT=1., exclusion_range=1., seed=12, exclusion_radius_per_type={1: 0.1}) + with self.assertRaisesRegex(ValueError, "Invalid excluded_radius value for type 2: radius -0.10"): + method.exclusion_radius_per_type = {2: -0.1} + self.assertEqual(list(method.exclusion_radius_per_type.keys()), [1]) if __name__ == "__main__": diff --git a/testsuite/python/virtual_sites_relative.py b/testsuite/python/virtual_sites_relative.py index cefc2d2b86c..1d2dd0c64d8 100644 --- a/testsuite/python/virtual_sites_relative.py +++ b/testsuite/python/virtual_sites_relative.py @@ -24,14 +24,22 @@ import tests_common -@utx.skipIfMissingFeatures("VIRTUAL_SITES_RELATIVE") +@utx.skipIfMissingFeatures(["VIRTUAL_SITES_RELATIVE", "LENNARD_JONES"]) class VirtualSites(ut.TestCase): system = espressomd.System(box_l=[1.0, 1.0, 1.0]) np.random.seed(42) + def setUp(self): + self.system.box_l = [10.0, 10.0, 10.0] + def tearDown(self): self.system.part.clear() + self.system.thermostat.turn_off() + self.system.integrator.set_vv() + self.system.non_bonded_inter[0, 0].lennard_jones.set_params( + epsilon=0., sigma=0., cutoff=0., shift=0.) + self.system.virtual_sites = espressomd.virtual_sites.VirtualSitesOff() def multiply_quaternions(self, a, b): return np.array( @@ -85,8 +93,10 @@ def test_aa_method_switching(self): espressomd.virtual_sites.VirtualSitesRelative) def test_vs_quat(self): + self.system.time_step = 0.01 + self.system.min_global_cut = 0.23 # First check that quaternion of virtual particle is unchanged if - # have_quaterion is false. + # have_quaternion is false. self.system.virtual_sites = espressomd.virtual_sites.VirtualSitesRelative( have_quaternion=False) self.assertFalse(self.system.virtual_sites.have_quaternion) @@ -126,23 +136,35 @@ def test_vs_quat(self): self.system.integrator.run(1) self.assertAlmostEqual(np.dot(p1.director, p2.director), 0.0) - # Check exceptions. + def test_vs_exceptions(self): + system = self.system + system.virtual_sites = espressomd.virtual_sites.VirtualSitesRelative() + system.time_step = 0.01 + system.cell_system.skin = 0.1 + system.min_global_cut = 0.1 + p1 = system.part.add(pos=[0.0, 0.0, 0.0], rotation=[1, 1, 1], id=1) + p2 = system.part.add(pos=[1.0, 1.0, 1.0], rotation=[1, 1, 1], id=2) + p3 = system.part.add(pos=[1.0, 1.0, 1.0], rotation=[1, 1, 1], id=3) + # relating to anything else other than a particle or id is not allowed with self.assertRaisesRegex(ValueError, "Argument of vs_auto_relate_to has to be of type ParticleHandle or int"): p2.vs_auto_relate_to('0') - try: - p2.vs_auto_relate_to(p1) - except BaseException: - self.fail('Failed to set a vs from a particle handle') + # relating to itself is not allowed + with self.assertRaisesRegex(ValueError, "A virtual site cannot relate to itself"): + p2.vs_auto_relate_to(p2) + # relating to a deleted particle is not allowed + with self.assertRaisesRegex(Exception, "No real particle with id 3 for virtual site with id 2"): + p2.vs_auto_relate_to(p3) + p3.remove() + system.integrator.run(0, recalc_forces=True) + if system.cell_system.get_state()["n_nodes"] > 1: + with self.assertRaisesRegex(Exception, r"The distance between virtual and non-virtual particle \([0-9\.]+\) is larger than the minimum global cutoff"): + p2.vs_auto_relate_to(p1) def test_pos_vel_forces(self): system = self.system system.cell_system.skin = 0.3 system.virtual_sites = espressomd.virtual_sites.VirtualSitesRelative() - system.box_l = [10, 10, 10] system.time_step = 0.004 - system.thermostat.turn_off() - system.non_bonded_inter[0, 0].lennard_jones.set_params( - epsilon=0, sigma=0, cutoff=0, shift=0) # Check setting of min_global_cut system.min_global_cut = 0.23 @@ -215,7 +237,7 @@ def run_test_lj(self): system = self.system system.virtual_sites = espressomd.virtual_sites.VirtualSitesRelative() # Parameters - n = 90 + n = 40 phi = 0.6 sigma = 1. eps = .025 @@ -296,10 +318,6 @@ def run_test_lj(self): self.assertNotAlmostEqual(enegry_pre_change, enegry_post_change) self.assertNotAlmostEqual(pressure_pre_change, pressure_post_change) - # Turn off lj interaction - system.non_bonded_inter[0, 0].lennard_jones.set_params( - epsilon=0, sigma=0, cutoff=0, shift=0) - def test_lj(self): """Run LJ fluid test for different cell systems.""" system = self.system diff --git a/testsuite/scripts/test_importlib_wrapper.py b/testsuite/scripts/test_importlib_wrapper.py index 9d959edae26..e21904ecb6f 100644 --- a/testsuite/scripts/test_importlib_wrapper.py +++ b/testsuite/scripts/test_importlib_wrapper.py @@ -380,6 +380,21 @@ def test_delimit_statements(self): linenos_out = iw.delimit_statements(source_code) self.assertEqual(linenos_out, linenos_exp) + def test_ipython_magics(self): + lines = [ + '%matplotlib inline', + '%%matplotlib notebook', + 'import matplotlib.pyplot as plt', + '__doc__="%matplotlib inline"', + ] + code = '\n'.join(lines) + code_protected = iw.protect_ipython_magics(code) + code_protected_ref = f'\n{code}'.replace( + '\n%', '\n#_IPYTHON_MAGIC_%')[1:] + self.assertEqual(code_protected, code_protected_ref) + code_deprotected = iw.deprotect_ipython_magics(code_protected) + self.assertEqual(code_deprotected, code) + if __name__ == "__main__": ut.main() diff --git a/testsuite/scripts/tutorials/test_convert.py b/testsuite/scripts/tutorials/test_convert.py index 1386ab93826..0f24519cdc2 100644 --- a/testsuite/scripts/tutorials/test_convert.py +++ b/testsuite/scripts/tutorials/test_convert.py @@ -85,11 +85,18 @@ class HtmlRunner(ut.TestCase): "version": ".".join(map(str, sys.version_info[:3]))} } - def failed_to_run(self, cmd): - traceback.print_exc() - self.fail('Could not run @CMAKE_BINARY_DIR@/pypresso ' - '@CMAKE_BINARY_DIR@/doc/tutorials/convert.py ' + - ' '.join(cmd)) + def run_command(self, cmd, output=None): + error_msg = ("Could not run @CMAKE_BINARY_DIR@/pypresso " + "@CMAKE_BINARY_DIR@/doc/tutorials/convert.py " + f"{' '.join(cmd)}") + try: + args = convert.parser.parse_args(cmd) + args.callback(args) + except BaseException: + traceback.print_exc() + self.fail(error_msg) + if output is not None: + self.assertTrue(os.path.isfile(output), f"{output} not created") def test_html_wrapper(self): f_input = '@CMAKE_CURRENT_BINARY_DIR@/test_convert_notebook.ipynb' @@ -114,12 +121,7 @@ def test_html_wrapper(self): '--scripts', f_script, '--substitutions', 'global_var=20', '--execute'] - try: - args = convert.parser.parse_args(cmd) - args.callback(args) - except BaseException: - self.failed_to_run(cmd) - self.assertTrue(os.path.isfile(f_output), f_output + ' not created') + self.run_command(cmd, f_output) # read processed notebook with open(f_output, encoding='utf-8') as f: nb_output = nbformat.read(f, as_version=4) @@ -193,12 +195,7 @@ def test_exercise2_plugin(self): '--output', f_output, '--substitutions', 'global_var=20', '--exercise2', '--remove-empty-cells'] - try: - args = convert.parser.parse_args(cmd) - args.callback(args) - except BaseException: - self.failed_to_run(cmd) - self.assertTrue(os.path.isfile(f_output), f_output + ' not created') + self.run_command(cmd, f_output) # read processed notebook with open(f_output, encoding='utf-8') as f: nb_output = nbformat.read(f, as_version=4) @@ -246,11 +243,7 @@ def test_exercise2_conversion(self): nbformat.write(nb, f) # run command and check for errors cmd = ['exercise2', '--to-py', f_input] - try: - args = convert.parser.parse_args(cmd) - args.callback(args) - except BaseException: - self.failed_to_run(cmd) + self.run_command(cmd, f_input) # read processed notebook with open(f_input, encoding='utf-8') as f: nb_output = nbformat.read(f, as_version=4) @@ -267,11 +260,7 @@ def test_exercise2_conversion(self): self.assertEqual(next(cells, 'EOF'), 'EOF') # run command and check for errors cmd = ['exercise2', '--to-md', f_input] - try: - args = convert.parser.parse_args(cmd) - args.callback(args) - except BaseException: - self.failed_to_run(cmd) + self.run_command(cmd, f_input) # read processed notebook with open(f_input, encoding='utf-8') as f: nb_output = nbformat.read(f, as_version=4) @@ -305,11 +294,7 @@ def test_exercise2_autopep8(self): nbformat.write(nb, f) # run command and check for errors cmd = ['exercise2', '--pep8', f_input] - try: - args = convert.parser.parse_args(cmd) - args.callback(args) - except BaseException: - self.failed_to_run(cmd) + self.run_command(cmd) # read processed notebook with open(f_input, encoding='utf-8') as f: nb_output = nbformat.read(f, as_version=4)