Code cleanup (#3699)

Description of changes:
- simplify handling of UBSAN and ASAN environment variables in CI and CMake
- better explain GPU-related CMake options and variables
- cleanup header includes
- cleanup documentation
- delete unused code, specfile and old UBSAN suppression file

.gitlab-ci.yml

@@ -1,4 +1,4 @@
-image: docker.pkg.github.com/espressomd/docker/ubuntu-20.04:06b6216c7aa3555bcf28c90734dbb84e7285c96f
+image: docker.pkg.github.com/espressomd/docker/ubuntu-20.04:700579881dceba20fa27ec476a4b4e0746b72683
 
 stages:
   - prepare
@@ -188,9 +188,7 @@ clang-sanitizer:
      CXX: 'clang++-9'
   script:
     - export myconfig=maxset with_cuda=true with_cuda_compiler=clang with_coverage=false
-    - export with_static_analysis=true test_timeout=900
-    - export with_asan=true ASAN_OPTIONS="allocator_may_return_null=1"
-    - export with_ubsan=true UBSAN_OPTIONS=suppressions=${CI_PROJECT_DIR}/maintainer/CI/ubsan.supp
+    - export with_static_analysis=true test_timeout=900 with_asan=true with_ubsan=true
     - bash maintainer/CI/build_cmake.sh
   timeout: 2h
   tags:

.travis.yml

@@ -11,7 +11,6 @@ matrix:
     - os: linux
       sudo: required
       services: docker
-      env: myconfig=maxset image=espressomd/docker-ubuntu-20.04:06b6216c7aa3555bcf28c90734dbb84e7285c96f
 
 script:
   - maintainer/CI/build_docker.sh

cmake/FindSphinx.cmake

@@ -7,8 +7,8 @@ execute_process(
   ERROR_VARIABLE QUERY_VERSION_ERR RESULT_VARIABLE QUERY_VERSION_RESULT)
 
 if(NOT QUERY_VERSION_RESULT)
-  # Sphinx switched at some point from returning ther version on stdout to
-  # printing it at stderr. Since we do not know ther version yet, we use stdout
+  # Sphinx switched at some point from returning their version on stdout to
+  # printing it at stderr. Since we do not know their version yet, we use stdout
   # if it matches a version regex, or stderr otherwise.
   if(QUERY_VERSION_OUT MATCHES "[0-9]+\.[0-9.]+")
     set(QUERY_VERSION "${QUERY_VERSION_OUT}")
@@ -26,7 +26,7 @@ if(NOT QUERY_VERSION_RESULT)
 endif()
 
 set(SPHINX_VERSION_COMPATIBLE TRUE)
-# Blacklist broken version
+# Blacklist broken versions
 if("${SPHINX_VERSION}" VERSION_EQUAL "2.1.0" OR "${SPHINX_VERSION}" VERSION_EQUAL "3.0.0")
   message(WARNING "Sphinx version ${SPHINX_VERSION} is not compatible.")
   set(SPHINX_VERSION_COMPATIBLE FALSE)

cmake/unit_test.cmake

@@ -48,13 +48,16 @@ function(UNIT_TEST)
   endif()
 
   if(WARNINGS_ARE_ERRORS)
-    set_tests_properties(
-      ${TEST_NAME}
-      PROPERTIES
-        ENVIRONMENT
-        "UBSAN_OPTIONS=suppressions=${CMAKE_SOURCE_DIR}/tools/ubsan-suppressions.txt:halt_on_error=1:print_stacktrace=1 ASAN_OPTIONS=halt_on_error=1:detect_leaks=0 MSAN_OPTIONS=halt_on_error=1"
-    )
+    set(SANITIZERS_HALT_ON_ERROR "halt_on_error=1")
+  else()
+    set(SANITIZERS_HALT_ON_ERROR "halt_on_error=0")
   endif()
+  set(UBSAN_OPTIONS "UBSAN_OPTIONS=suppressions=${CMAKE_SOURCE_DIR}/maintainer/CI/ubsan.supp:${SANITIZERS_HALT_ON_ERROR}:print_stacktrace=1")
+  set(ASAN_OPTIONS "ASAN_OPTIONS=${SANITIZERS_HALT_ON_ERROR}:detect_leaks=0:allocator_may_return_null=1")
+  set(MSAN_OPTIONS "MSAN_OPTIONS=${SANITIZERS_HALT_ON_ERROR}")
+  set_tests_properties(
+    ${TEST_NAME} PROPERTIES ENVIRONMENT
+                            "${UBSAN_OPTIONS} ${ASAN_OPTIONS} ${MSAN_OPTIONS}")
 
   add_dependencies(check_unit_tests ${TEST_NAME})
 endfunction(UNIT_TEST)

doc/sphinx/installation.rst

@@ -91,20 +91,29 @@ are required:
     sudo apt install python3-matplotlib python3-scipy ipython3 jupyter-notebook
     sudo pip3 install 'pint>=0.9'
 
+Nvidia GPU acceleration
+"""""""""""""""""""""""
+
 If your computer has an Nvidia graphics card, you should also download and install the
 CUDA SDK to make use of GPU computation:
 
 .. code-block:: bash
 
     sudo apt install nvidia-cuda-toolkit
 
-On Ubuntu 18.04, you need to modify a file to make CUDA work with the default compiler:
+On Ubuntu, the default GCC compiler is too recent for nvcc, which will generate
+compiler errors. You can either install an older version of GCC and select it
+with environment variables ``CC`` and ``CXX`` when building |es|, or edit the
+system header files as shown in the following example for Ubuntu 18.04:
 
 .. code-block:: bash
 
     sudo sed -i 's/__GNUC__ > 6/__GNUC__ > 7/g' /usr/include/crt/host_config.h
     sudo sed -i 's/than 6/than 7/g' /usr/include/crt/host_config.h
 
+AMD GPU acceleration
+""""""""""""""""""""
+
 If your computer has an AMD graphics card, you should also download and install the
 ROCm SDK to make use of GPU computation:
 
@@ -683,6 +692,13 @@ For example with ``-D WITH_CUDA=ON``, one can choose the CUDA compiler with
 ``-D ROCM_HOME=<path_to_rocm>`` variable becomes available, with default value
 ``ROCM_HOME=/opt/rocm``.
 
+Environment variables can be passed to CMake. For example, to select Clang, use
+``CC=clang CXX=clang++ cmake .. -DWITH_CUDA=ON -DWITH_CUDA_COMPILER=clang``.
+If you have multiple versions of the CUDA library installed, you can select the
+correct one with ``CUDA_BIN_PATH=/usr/local/cuda-10.0 cmake .. -DWITH_CUDA=ON``
+(with Clang as the CUDA compiler, you also need to override its default CUDA
+path with ``-DCMAKE_CXX_FLAGS=--cuda-path=/usr/local/cuda-10.0``).
+
 
 Compiling, testing and installing
 ---------------------------------

maintainer/CI/build_docker.sh

@@ -22,16 +22,14 @@ cat > "${ENV_FILE}" <<EOF
 cmake_params=${cmake_params}
 with_fftw=${with_fftw}
 with_coverage=false
-with_cuda=false
-myconfig=${myconfig}
+with_cuda=true
+CC=gcc-8
+CXX=g++-8
 check_procs=${check_procs}
 make_check=${make_check}
 build_type=RelWithAssert
+myconfig=maxset
 EOF
-
-if [ -z "${image}" ]; then
-    echo "ERROR: environment variable 'image' is missing" >&2
-    exit 1
-fi
+image="espressomd/docker-ubuntu-20.04:06b6216c7aa3555bcf28c90734dbb84e7285c96f"
 
 docker run -u espresso --env-file "${ENV_FILE}" -v "${PWD}:/travis" -it "${image}" /bin/bash -c "cp -r /travis .; cd travis && maintainer/CI/build_cmake.sh" || exit 1

samples/gibbs_ensemble_client.py

@@ -53,7 +53,7 @@
 MSG_EXCHANGE_PART_ADD_REVERT = 41
 MSG_EXCHANGE_PART_REMOVE = 5
 MSG_EXCHANGE_PART_REMOVE_REVERT = 51
-MSG_ENEGRY = 6
+MSG_ENERGY = 6
 
 parser = argparse.ArgumentParser()
 parser.add_argument('-n', '--number-particles', type=int, nargs=1)
@@ -166,10 +166,10 @@ def send_data(data, socket):
 
 # send the initial energy
 energy = system.analysis.energy()['total']
-send_data(pickle.dumps([MSG_ENEGRY, energy]), socket)
+send_data(pickle.dumps([MSG_ENERGY, energy]), socket)
 
 while msg[0] != MSG_END:
-    # receive comand to execute next step
+    # receive command to execute next step
     msg = recv_data(socket)
     if msg[0] == MSG_END:
         break
@@ -191,7 +191,7 @@ def send_data(data, socket):
 
     # calculation energy and send it to the host
     energy = system.analysis.energy()['total']
-    send_data(pickle.dumps([MSG_ENEGRY, energy]), socket)
+    send_data(pickle.dumps([MSG_ENERGY, energy]), socket)
 
 # closing the socket
 socket.close()

samples/gibbs_ensemble_socket.py

@@ -81,7 +81,7 @@
 EXCHANGE_CHANCE = 0.8
 VOLUME_CHANCE = 1.0 - INIT_MOVE_CHANCE - EXCHANGE_CHANCE
 
-# socket paramters
+# socket parameters
 HOST = 'localhost'
 PORT = 31415
 NUMBER_OF_CLIENTS = 2
@@ -96,7 +96,7 @@
 MSG_EXCHANGE_PART_ADD_REVERT = 41
 MSG_EXCHANGE_PART_REMOVE = 5
 MSG_EXCHANGE_PART_REMOVE_REVERT = 51
-MSG_ENEGRY = 6
+MSG_ENERGY = 6
 
 # script locations
 espresso_executable = "../pypresso"
@@ -139,7 +139,7 @@ class Box:
     def recv_energy(self):
         '''Received the energy data from the client.'''
         msg = self.recv_data()
-        if msg[0] == MSG_ENEGRY:
+        if msg[0] == MSG_ENERGY:
             self.energy = msg[1]
             return 0
         else:

src/core/CellStructure.hpp

@@ -154,7 +154,7 @@ struct CellStructure {
    */
   Particle &append_indexed_particle(ParticleList &pl, Particle &&p) {
     /* Check if cell may reallocate, in which case the index
-     * entries for all particles in this celle have to be
+     * entries for all particles in this cell have to be
      * updated. */
     auto const may_reallocate = pl.size() >= pl.capacity();
     auto &new_part = pl.insert(std::move(p));

src/core/RuntimeErrorCollector.cpp

@@ -23,7 +23,7 @@
 
 #include <boost/mpi/collectives.hpp>
 
-#include <sstream>
+#include <iostream>
 #include <utility>
 
 using namespace std;

src/core/accumulators/Correlator.cpp

@@ -162,8 +162,6 @@ std::vector<double> fcs_acf(std::vector<double> const &A,
   return C;
 }
 
-/* global variables */
-
 /* Error codes */
 constexpr const char init_errors[][64] = {
     "",                                                              // 0

src/core/accumulators/Correlator.hpp

@@ -116,9 +116,6 @@
 #include <boost/multi_array.hpp>
 #include <boost/serialization/access.hpp>
 
-#include <cmath>
-#include <cstdlib>
-#include <map>
 #include <memory>
 #include <utility>
 

src/core/actor/DipolarBarnesHut.hpp

@@ -44,16 +44,13 @@ class DipolarBarnesHut : public Actor {
     m_itolsq = itolsq;
     setBHPrecision(&m_epssq, &m_itolsq);
     if (!s.requestFGpu())
-      std::cerr << "DipolarBarnesHut needs access to forces on GPU!"
-                << std::endl;
+      runtimeErrorMsg() << "DipolarBarnesHut needs access to forces on GPU!";
 
     if (!s.requestRGpu())
-      std::cerr << "DipolarBarnesHut needs access to positions on GPU!"
-                << std::endl;
+      runtimeErrorMsg() << "DipolarBarnesHut needs access to positions on GPU!";
 
     if (!s.requestDipGpu())
-      std::cerr << "DipolarBarnesHut needs access to dipoles on GPU!"
-                << std::endl;
+      runtimeErrorMsg() << "DipolarBarnesHut needs access to dipoles on GPU!";
   };
 
   void computeForces(SystemInterface &s) override {
@@ -66,10 +63,7 @@ class DipolarBarnesHut : public Actor {
     summarizeBH(m_bh_data.blocks);
     sortBH(m_bh_data.blocks);
     if (forceBH(&m_bh_data, k, s.fGpuBegin(), s.torqueGpuBegin())) {
-      fprintf(
-          stderr,
-          "forceBH: some of kernels encounter the algorithm functional error");
-      errexit();
+      runtimeErrorMsg() << "kernels encountered a functional error";
     }
   };
   void computeEnergy(SystemInterface &s) override {
@@ -82,10 +76,7 @@ class DipolarBarnesHut : public Actor {
     summarizeBH(m_bh_data.blocks);
     sortBH(m_bh_data.blocks);
     if (energyBH(&m_bh_data, k, (&(((CUDA_energy *)s.eGpu())->dipolar)))) {
-      fprintf(
-          stderr,
-          "energyBH: some of kernels encounter the algorithm functional error");
-      errexit();
+      runtimeErrorMsg() << "kernels encountered a functional error";
     }
   };
 

src/core/actor/DipolarBarnesHut_cuda.cu

@@ -476,7 +476,7 @@ __global__ __launch_bounds__(THREADS3, FACTOR3) void summarizationKernel() {
   // position of equivalent total dipole and its magnitude:
   // (like a mass and the center of mass)
   float p[3], u[3];
-  // Per-block BH tree cashing:
+  // Per-block BH tree caching:
   __shared__ int child[THREADS3 * 8];
 
   // no children by default:
@@ -522,14 +522,14 @@ __global__ __launch_bounds__(THREADS3, FACTOR3) void summarizationKernel() {
           ch = bhpara->child[k * 8 + i];
           if (ch >= 0) {
             if (i != j) {
-              // Move children to front (needed later for a speed only).
+              // Move child to front (needed later for a speed only).
               // The child's octant change is incorrect from
               // a tree organization perspective. However, the sum
               // will be the same.
               bhpara->child[k * 8 + i] = -1;
               bhpara->child[k * 8 + j] = ch;
             }
-            // Cache a missing children in the block shared memory:
+            // Cache a missing child in the block shared memory:
             child[missing * THREADS3 + threadIdx.x] = ch;
             m = bhpara->mass[ch];
             // Is a child the particle? Only particles have non-negative mass
@@ -612,8 +612,7 @@ __global__ __launch_bounds__(THREADS3, FACTOR3) void summarizationKernel() {
           bhpara->r[3 * k + l] = p[l] * m;
           bhpara->u[3 * k + l] = u[l];
         }
-        __threadfence(); // make sure data are visible before setting
-        //                    // mass
+        __threadfence(); // make sure data are visible before setting mass
         bhpara->mass[k] = cm;
         __threadfence();
         k += inc;
@@ -731,7 +730,7 @@ __global__ __launch_bounds__(THREADS5, FACTOR5) void forceCalculationKernel(
     // "epssqd" which define a fraction of the octant cell and
     // an additive distance respectively.
     // Their joint contribution for the given tree depth are
-    // calculated withing the array dq[i], which will
+    // calculated within the array dq[i], which will
     // be compared later with squared distance between the particle
     // and the cell depending on a cell level.
     // Original tree box edge (2*radiusd) should be divided *0.5