Fix test with gcc7

Some tests where not compiling with GCC v.7.

I have done a few quick fix, but the following tests are failing

• test27cells.sh
• test125cellsPerturbed.sh
• test-driver (dirty crash)
• testRiemannHLLC
• testPeriodicBC.sh
• testPeriodicBCPerturbed.sh

On my master and with GCC 5.4, only the two first are failing.

TestPeriodicBC[Perturbed].sh

failing due to a segmentation fault

RiemannHLLC

[05783.2] testRiemannHLLC.c:check_riemann_symmetry():76: Asymmetry in flux solution!
[05782.9] check_riemann_symmetry: Flux solver asymmetric: [7.467e+00,1.018e+02,-5.346e+01,-9.949e+01,1.567e+03] == [-7.467e+00,-1.018e+02,5.346e+01,9.949e+01,-1.567e+03]

FAIL testRiemannHLLC (exit status: 134)

Test Cells

Accuracy error (relative error of about 1e-3)

Driver

../test-driver: line 107: 17857 Aborted (core dumped) "$@" > $log_file 2>&1

I will look to theses bugs, but if you have any suggestions before I start. I will gladly listen to them

changed the description

Here is my config (I am using the enable-debug option)

 ------- Summary --------

   SWIFT v.0.6.0

   Compiler         : mpicc
    - vendor        : gnu
    - version       : 7.2.0
    - flags         : -g -O0  -gdwarf -fvar-tracking-assignments -O3 -fomit-frame-pointer -malign-double -fstrict-aliasing -ffast-math -funroll-loops -march=core-avx2 -mavx2 -pthread -Wall -Wextra -Wno-unused-parameter -Wshadow -Werror
   MPI enabled      : yes
   HDF5 enabled     : yes
    - parallel      : yes
   Metis enabled    : yes
   FFTW3 enabled    : yes
   libNUMA enabled  : yes
   Using tcmalloc   : no
   Using jemalloc   : no
   CPU profiler     : no
   Pthread barriers : yes

   Hydro scheme       : gadget2
   Dimensionality     : 3
   Kernel function    : cubic-spline
   Equation of state  : ideal-gas
   Adiabatic index    : 5/3
   Riemann solver     : none
   Cooling function   : none

   External potential  : none
   Multipole order     : 5
   No gravity below ID : no

   Individual timers     : no
   Task debugging        : no
   Threadpool debugging  : no
   Debugging checks      : no
   Interaction debugging : no
   Naive interactions    : no
   Gravity checks        : no

 ------------------------

For the periodic boundary conditions, switching O3 to O2 is enough (or printing just before line 169).

added 1 commit

• 3b2c8a1b - remove abort (autotools deal badly with it)

Compare with previous version

I removed the call to error as test-driver is printing the abort

The other bugs are related to a lack of accuracy (seems that the relative error is always below 1e-3)

Don't worry about the accuracy failures, those will need to be looked at by @jwillis or @matthieu. Seems every new compiler breaks those.

We don't have 7.2.0 on COSMA, do we? I have 7.2.0 on my laptop so I could test this when I get home.

4.8 and 5.4 on cosma.

Don't see this problem under Fedora 26, what exactly is the error?

I am getting the following error (I have been able to get the same behavior with a different failing test). It is not important, but it would be cleaner if we change this.

PASS: testLogger
../test-driver: line 107: 28213 Aborted                 (core dumped) "$@" > $log_file 2>&1
FAIL: testRiemannHLLC

But message() followed by exit() is exactly what error() is. Why the change?

it is an abort, not an exit

I see, so it is the philosophy not an actual error. Actually for tests I agree that a non-zero exit is better style, you only want to count the failures. In a production system I have occasionally wondered if we want abort(), those core files will be too large to be useful.

Peter, you mean that we should redefine error() to use exit() instead of abort() throughout the code?

Yes, an exit(1) is fine for most errors, why would you want a core file as well as the message. They rarely are of use as MPI dumps tend to truncate, and then there is the potential size to worry about.

That makes sense.

Let's do this somewhere else than it this merge request though. And when we have nothing better to worry about. :)

@jwillis you can use spack to install/compile gcc v7 ;)

added 1 commit

• 095105be - Add comment to remove a quick fix

Compare with previous version

I can't reproduce these crashes on my laptop (Haswell) with gcc 7.2. Everything runs smoothly.

For the accuracy errors, we will replace these tests with something more robust as they are a real pain.

I'll open a discussion thread for exit() vs. abort() and merge in the other two genuine fixes.

I have "cherry-picked" the relevant commits directly into master.

closed

@lhausammann for reference your suggestion for the abort() has been pushed in !473 (merged).