2684 coulomb sr test

[christophlohrmann]

Fixes #2684 and also the way charges and the coulomb prefactor are used in Coulomb energy/force pair calculations

Description of changes:

PR Checklist

• Tests?
  • Interface
  • Core
• Docs?

[codecov]

Codecov Report

Merging #2718 into python will increase coverage by <1%.
The diff coverage is 85%.

@@           Coverage Diff           @@
##           python   #2718    +/-   ##
=======================================
+ Coverage      79%     79%   +<1%     
=======================================
  Files         508     508            
  Lines       27050   27052     +2     
=======================================
+ Hits        21590   21624    +34     
+ Misses       5460    5428    -32

Impacted Files	Coverage Δ
...re/bonded_interactions/bonded_interaction_data.hpp	100% <ø> (ø)	⬆️
src/core/electrostatics_magnetostatics/mmm1d.cpp	31% <0%> (ø)	⬆️
src/core/energy_inline.hpp	66% <100%> (+2%)	⬆️
src/core/pressure_inline.hpp	85% <100%> (ø)	⬆️
...e/electrostatics_magnetostatics/reaction_field.hpp	100% <100%> (ø)	⬆️
src/core/electrostatics_magnetostatics/mmm2d.cpp	90% <100%> (ø)	⬆️
src/core/bonded_interactions/bonded_coulomb_sr.hpp	100% <100%> (ø)
src/core/bonded_interactions/bonded_coulomb_sr.cpp	100% <100%> (ø)
src/core/forces_inline.hpp	81% <100%> (+1%)	⬆️
...e/electrostatics_magnetostatics/coulomb_inline.hpp	90% <100%> (-1%)	⬇️
... and 5 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2007aaa...742cc6b. Read the comment docs.

[jngrad]

@christophlohrmann: I don't run into the Scafacos issue you mentioned when I restore the const modifier for double const *d in the argument list of calc_pair_force() in fa28888.
@KaiSzuttor: in docker I can't reproduce the error log from clang:6.0 (https://gitlab.icp.uni-stuttgart.de/espressomd/espresso/-/jobs/97366). The mmm1d.hpp file doesn't raise any warning, and CMake eventually runs into a LeakSanitizer error. ptrace is not installed. Here's a MWE:

docker run --volume /scratch/jgrad:/local:rw -it --user espresso --runtime=nvidia gitlab.icp.uni-stuttgart.de:4567/espressomd/docker/clang:6.0
cd /local/es-cl/
export myconfig=maxset with_coverage=false with_static_analysis=true with_asan=true with_ubsan=true test_timeout=900
bash maintainer/cuda_build.sh

[ 27%] Building CXX object src/core/CMakeFiles/EspressoCore.dir/electrostatics_magnetostatics/mmm1d.cpp.o
[ 27%] Building CXX object src/core/CMakeFiles/EspressoCore.dir/electrostatics_magnetostatics/mmm2d.cpp.o
...
[ 58%] Linking CXX shared library EspressoScriptInterface.so
[ 58%] Built target EspressoScriptInterface
[ 58%] Generating myconfig.pxi
[ 58%] Generating myconfig.pxi
[ 58%] Generating myconfig.pxi
[ 58%] Generating myconfig.pxi
==1813==LeakSanitizer has encountered a fatal error.
==1813==HINT: For debugging, try setting environment variable LSAN_OPTIONS=verbosity=1:log_threads=1
==1813==HINT: LeakSanitizer does not work under ptrace (strace, gdb, etc)
src/python/espressomd/CMakeFiles/visualization_mayavi.dir/build.make:100: recipe for target 'src/python/espressomd/myconfig.pxi' failed
make[2]: *** [src/python/espressomd/myconfig.pxi] Error 1
make[2]: *** Deleting file 'src/python/espressomd/myconfig.pxi'

[fweik]

The clang-tidy warning is a valid issue, please change it. Also I don't think parameters passed by value should be made const, this is of no concern to the caller.

[jngrad]

@fweik I want to change it, but after 3 hours of troubleshooting in docker with the clang:6.0 container, I still can't get clang-tidy to flag the const with a warning, so I don't know how many occurrences of this warning are in the PR. I can't change this one line, push and wait for the next const warning to appear in the CI, this might take hours depending on how many there are.

[fweik]

It's not clear to me what you are tying to do? How is any of this related to sanitizers?

[fweik]

bors r+

[jngrad]

Here's the log from the failed job on clang:6.0

[  4%] Building CXX object src/core/CMakeFiles/EspressoCore.dir/electrostatics_magnetostatics/coulomb.cpp.o
/builds/espressomd/espresso/src/core/electrostatics_magnetostatics/mmm1d.hpp:71:34: error:
parameter 'q1q2' is const-qualified in the function declaration; const-qualification of parameters only
has an effect in function definitions [readability-avoid-const-params-in-decls,-warnings-as-errors]
double mmm1d_coulomb_pair_energy(double const q1q2, double const d[3],
                                 ^      ~~~~~~~~~~

So I looked for readability-avoid-const-params-in-decls and found it is was clang-tidy flag, not a compiler flag. The CI logfile stops at 48% for the core and 5% for the non-core, so I didn't how many similar errors could be in the rest of the code. To find out, I pulled the PR and tried to compile it using the CMake options declared for the relevant job in gitlab-ci.yml, but clang-tidy is not installed on our infrastructure. Then I fetched the clang:6.0 docker container to get clang-tidy and compile the PR again with the exact environment that failed in CI, but I got a completely different output. Tinkering with the CMake options for hours changed nothing, the behavior on the CI is not reproducible on my machine.

[fweik]

Ah I see. I just checked with the clang-7 from the ubuntu repository, and it finds the issues as expected (and then some). If you don't have the package installed you could just spin up a docker container and install it there, this is also useful to avoid the boost problem that clang-tidy finds and is fixed in newer boost versions. (this is why the docker container for the CI has a patch for boost). (I checked with a fresh ubuntu:bionic container, installing cmake clang++-7 clang-tidy-7 libboost-all-dev cython python-dev python-numpy.)

The issue you are seeing could be related to ptrace not being available inside a normal container,
if so this could potentially be fixed using the --privileged option of docker run or adding some capability.

[bors]

Build succeeded

• ICP GitLab CI