Improve code coverage

Factor out duplicated code, remove unreachable code, add extra checks.

.gitlab-ci.yml

@@ -292,6 +292,7 @@ tutorials-samples-maxset:
      with_coverage: 'false'
      with_coverage_python: 'true'
      with_scafacos: 'true'
+     with_stokesian_dynamics: 'true'
      make_check_unit_tests: 'false'
      make_check_python: 'false'
      make_check_tutorials: 'true'

src/core/MpiCallbacks.hpp

@@ -508,7 +508,7 @@ class MpiCallbacks {
    * @brief Remove callback.
    *
    * Remove the callback id from the callback list.
-   * This is a collective call that must be run on all node.
+   * This is a collective call that must be run on all nodes.
    *
    * @param id Identifier of the callback to remove.
    */

src/core/unit_tests/MpiCallbacks_test.cpp

@@ -33,6 +33,7 @@
 
 #include <algorithm>
 #include <functional>
+#include <stdexcept>
 #include <string>
 
 static bool called = false;

src/core/unit_tests/Particle_serialization_test.cpp

@@ -213,7 +213,7 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(
   {
     static_assert(!TraitWrapper::template apply<Particle>::value);
     typename Checker::buffer_type buffer_ref = {"Particle"};
-    typename Checker::buffer_type buffer;
+    typename Checker::buffer_type buffer = {};
     Checker oa{buffer};
     Particle p;
     oa &p;
@@ -228,7 +228,7 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(
         "Utils::compact_vector<int>",
 #endif
     };
-    typename Checker::buffer_type buffer;
+    typename Checker::buffer_type buffer = {};
     Checker oa{buffer};
     Particle p;
     oa | p;

src/core/unit_tests/Verlet_list_test.cpp

@@ -280,8 +280,11 @@ int main(int argc, char **argv) {
   espresso::system = std::make_unique<EspressoSystemStandAlone>(argc, argv);
   // the test case only works for 4 MPI ranks
   boost::mpi::communicator world;
-  if (world.size() == 4)
-    return boost::unit_test::unit_test_main(init_unit_test, argc, argv);
+  int error_code = 0;
+  if (world.size() == 4) {
+    error_code = boost::unit_test::unit_test_main(init_unit_test, argc, argv);
+  }
+  return error_code;
 }
 #else // ifdef LENNARD_JONES
 int main(int argc, char **argv) {}

src/core/unit_tests/p3m_test.cpp

@@ -27,6 +27,8 @@
 
 #include <array>
 #include <cstddef>
+#include <limits>
+#include <stdexcept>
 #include <vector>
 
 BOOST_AUTO_TEST_CASE(calc_meshift_false) {
@@ -58,3 +60,27 @@ BOOST_AUTO_TEST_CASE(calc_meshift_true) {
     }
   }
 }
+
+#if defined(P3M) || defined(DP3M)
+BOOST_AUTO_TEST_CASE(analytic_cotangent_sum) {
+  auto constexpr kernel = p3m_analytic_cotangent_sum;
+  auto constexpr tol = 100. * std::numeric_limits<double>::epsilon();
+
+  // check only trivial cases
+  for (auto const cao : {1, 2, 3, 4, 5, 6, 7}) {
+    BOOST_CHECK_CLOSE(kernel(0, 0., cao), 1., tol);
+  }
+  BOOST_CHECK_CLOSE(kernel(1, 0.5, 1), 1., tol);
+  BOOST_CHECK_CLOSE(kernel(1, 0.5, 2), 1. / 3., tol);
+  BOOST_CHECK_CLOSE(kernel(1, 0.5, 3), 2. / 15., tol);
+  BOOST_CHECK_CLOSE(kernel(1, 0.5, 4), 17. / 315., tol);
+  BOOST_CHECK_CLOSE(kernel(1, 0.5, 5), 62. / 2835., tol);
+  BOOST_CHECK_CLOSE(kernel(1, 0.5, 6), 1382. / 155925., tol);
+  BOOST_CHECK_CLOSE(kernel(1, 0.5, 7), 21844. / 6081075., tol);
+
+  // check assertion
+  for (auto const invalid_cao : {-1, 0, 8}) {
+    BOOST_CHECK_THROW(kernel(1, 0., invalid_cao), std::logic_error);
+  }
+}
+#endif // defined(P3M) || defined(DP3M)

src/utils/tests/Factory_test.cpp

@@ -17,12 +17,6 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-/* Unit tests for the Utils::Factory class.
- * The factory is tested by registering different types of classes
- * with it (first test), and then checking if instances of those classes can be
- * made via the Factory (second test).
- */
-
 #define BOOST_TEST_MODULE Factory test
 #define BOOST_TEST_DYN_LINK
 #include <boost/test/unit_test.hpp>
@@ -41,43 +35,28 @@ struct DerivedTestClass : public TestClass {
   void method() override {}
 };
 
-struct OtherDerivedTestClass : public TestClass {
-  void method() override {}
-};
-
-/* Check registration of construction functions */
-BOOST_AUTO_TEST_CASE(register_class) {
-  Utils::Factory<TestClass> factory;
-
-  factory.register_new<OtherDerivedTestClass>("other_derived_class");
-
-  BOOST_CHECK(factory.has_builder("other_derived_class"));
-}
-
-/* Check object construction. */
 BOOST_AUTO_TEST_CASE(make) {
   Utils::Factory<TestClass> factory;
-  factory.register_new<DerivedTestClass>("derived_test_class");
+  auto const derived_class_name = std::string{"derived_test_class"};
 
-  /* Make a derived object */
-  auto o = factory.make("derived_test_class");
-  BOOST_CHECK(o);
-
-  /* Check for correct (derived) type */
-  BOOST_CHECK(dynamic_cast<DerivedTestClass *>(o.get()) != nullptr);
-}
-
-BOOST_AUTO_TEST_CASE(type_name) {
-  const std::string derived_class_name = "derived_test_class";
-
-  Utils::Factory<TestClass> factory;
+  // Register construction function
   factory.register_new<DerivedTestClass>(derived_class_name);
 
-  /* Make an object */
+  // Check registration of construction function
+  BOOST_REQUIRE(factory.has_builder(derived_class_name));
+
+  // Make a derived object
   auto o = factory.make(derived_class_name);
+  BOOST_REQUIRE(o);
   o->method();
+
+  // Check for correct type name
   BOOST_CHECK_EQUAL(factory.type_name(*o.get()), derived_class_name);
 
-  /* Make an unknown object */
+  // Check for correct (derived) type
+  BOOST_CHECK(dynamic_cast<DerivedTestClass *>(o.get()) != nullptr);
+
+  // Make an unknown object
+  BOOST_CHECK(not factory.has_builder("unknown"));
   BOOST_CHECK_THROW(factory.make("unknown"), std::domain_error);
 }

src/utils/tests/gatherv_test.cpp

@@ -28,56 +28,72 @@
 
 #include <algorithm>
 #include <string>
+#include <utility>
 #include <vector>
 
-/*
- * Check that implementation behaves
- * like @c MPI_Gatherv with an mpi datatype.
- * To test this, we gather the rank from
- * every rank to one rank and then check
- * that the value was written to the
- * correct position in the output array.
- */
-BOOST_AUTO_TEST_CASE(mpi_type) {
+struct identity {
+  template <class T> constexpr T &&operator()(T &&t) const noexcept {
+    return std::forward<T>(t);
+  }
+};
+
+template <typename T, class F> void check(T default_value, F conversion) {
   boost::mpi::communicator world;
   auto const rank = world.rank();
   auto const size = world.size();
   auto const root = world.size() - 1;
+  auto const in = conversion(rank);
 
   /* out-of-place */
   {
     if (rank == root) {
-      std::vector<int> out(size, -1);
+      std::vector<T> out(size, default_value);
       std::vector<int> sizes(size, 1);
 
-      Utils::Mpi::gatherv(world, &rank, 1, out.data(), sizes.data(), root);
+      Utils::Mpi::gatherv(world, &in, 1, out.data(), sizes.data(), root);
 
       for (int i = 0; i < size; i++) {
-        BOOST_CHECK_EQUAL(out.at(i), i);
+        BOOST_CHECK_EQUAL(out.at(i), conversion(i));
       }
+    } else if (rank % 2 == 0) {
+      Utils::Mpi::gatherv(world, &in, 1, static_cast<T *>(nullptr), nullptr,
+                          root);
     } else {
-      Utils::Mpi::gatherv(world, &rank, 1, root);
+      Utils::Mpi::gatherv(world, &in, 1, root);
     }
   }
 
   /* in-place */
   {
     if (rank == root) {
-      std::vector<int> out(size, -1);
-      out[rank] = rank;
+      std::vector<T> out(size, default_value);
+      out[rank] = in;
       std::vector<int> sizes(size, 1);
 
       Utils::Mpi::gatherv(world, out.data(), 1, out.data(), sizes.data(), root);
 
       for (int i = 0; i < size; i++) {
-        BOOST_CHECK_EQUAL(out.at(i), i);
+        BOOST_CHECK_EQUAL(out.at(i), conversion(i));
       }
+    } else if (rank % 2 == 0) {
+      Utils::Mpi::gatherv(world, &in, 1, static_cast<T *>(nullptr), nullptr,
+                          root);
     } else {
-      Utils::Mpi::gatherv(world, &rank, 1, root);
+      Utils::Mpi::gatherv(world, &in, 1, root);
     }
   }
 }
 
+/*
+ * Check that implementation behaves
+ * like @c MPI_Gatherv with an mpi datatype.
+ * To test this, we gather the rank from
+ * every rank to one rank and then check
+ * that the value was written to the
+ * correct position in the output array.
+ */
+BOOST_AUTO_TEST_CASE(mpi_type) { check(-1, identity{}); }
+
 /*
  * Check that implementation behaves
  * like @c MPI_Gatherv with a non-mpi datatype.
@@ -87,44 +103,7 @@ BOOST_AUTO_TEST_CASE(mpi_type) {
  * correct position in the output array.
  */
 BOOST_AUTO_TEST_CASE(non_mpi_type) {
-  boost::mpi::communicator world;
-  auto const rank = world.rank();
-  auto const size = world.size();
-  auto const root = world.size() - 1;
-  auto const in = std::to_string(rank);
-
-  /* out-of-place */
-  {
-    if (rank == root) {
-      std::vector<std::string> out(size);
-      std::vector<int> sizes(size, 1);
-
-      Utils::Mpi::gatherv(world, &in, 1, out.data(), sizes.data(), root);
-
-      for (int i = 0; i < size; i++) {
-        BOOST_CHECK_EQUAL(out.at(i), std::to_string(i));
-      }
-    } else {
-      Utils::Mpi::gatherv(world, &in, 1, root);
-    }
-  }
-
-  /* in-place */
-  {
-    if (rank == root) {
-      std::vector<std::string> out(size);
-      out[rank] = in;
-      std::vector<int> sizes(size, 1);
-
-      Utils::Mpi::gatherv(world, out.data(), 1, out.data(), sizes.data(), root);
-
-      for (int i = 0; i < size; i++) {
-        BOOST_CHECK_EQUAL(out.at(i), std::to_string(i));
-      }
-    } else {
-      Utils::Mpi::gatherv(world, &in, 1, root);
-    }
-  }
+  check(std::string{""}, static_cast<std::string (&)(int)>(std::to_string));
 }
 
 int main(int argc, char **argv) {

testsuite/python/ek_eof_one_species.py

@@ -24,7 +24,6 @@
 import tempfile
 import contextlib
 
-import sys
 import math
 import numpy as np
 
@@ -159,55 +158,25 @@ def hydrostatic_pressure(ek, tensor_entry, box_x, box_y, box_z, agrid):
 
 
 def bisection():
+    args = [params_base[k]
+            for k in ('width', 'bjerrum_length', 'sigma', 'valency')]
     # initial parameters for bisection scheme
     size = math.pi / (2.0 * params_base['width'])
     pnt0 = 0.0
     pntm = pnt0 + size
     pnt1 = pnt0 + 1.9 * size
-
     # the bisection scheme
     tol = 1.0e-08
     while size > tol:
-        val0 = solve(
-            pnt0,
-            params_base['width'],
-            params_base['bjerrum_length'],
-            params_base['sigma'],
-            params_base['valency'])
-        val1 = solve(
-            pnt1,
-            params_base['width'],
-            params_base['bjerrum_length'],
-            params_base['sigma'],
-            params_base['valency'])
-        valm = solve(
-            pntm,
-            params_base['width'],
-            params_base['bjerrum_length'],
-            params_base['sigma'],
-            params_base['valency'])
-
-        if (val0 < 0.0 and val1 > 0.0):
-            if valm < 0.0:
-                pnt0 = pntm
-                size = size / 2.0
-                pntm = pnt0 + size
-            else:
-                pnt1 = pntm
-                size = size / 2.0
-                pntm = pnt1 - size
-        elif (val0 > 0.0 and val1 < 0.0):
-            if valm < 0.0:
-                pnt1 = pntm
-                size = size / 2.0
-                pntm = pnt1 - size
-            else:
-                pnt0 = pntm
-                size = size / 2.0
-                pntm = pnt0 + size
+        size /= 2.0
+        val0, val1, valm = map(lambda x: solve(x, *args), [pnt0, pnt1, pntm])
+        assert val0 < 0.0 and val1 > 0.0, "Bisection method failed"
+        if valm < 0.0:
+            pnt0 = pntm
+            pntm += size
         else:
-            sys.exit("Bisection method fails:\n"
-                     "Tuning of regular boundaries may be required.")
+            pnt1 = pntm
+            pntm -= size
     return pntm