Refactor cleaning test directories

This is a minor refactor of how we clean up
test directories in tests/*/.

We clean the test directory each time
we run the test. We also cleanup all test
directories by running `make testclean`.

As part of this, I renamed 'mini.dat' to 'mini.xyz' in all tests
so that we can simply delete all *.dat files.
To ensure I did not forget any output files,
I asked Git about the untracked files after
I ran the `make test && make testclean` command.

To find all untracked files, even those
ignored by .gitignore, I used `git ls-files -o`

I also cleaned up the 'diff_files' and 'makeref' functions,
hopefully they are a bit more readable now. We need to
carefully test that I didn't break anything!

Test plan:
 - clone fresh repo
 - ./configure && make && make test
 - make clean && make testclean
 - `git ls-files -o` should not list any extra files.

Author: danielhollas

.gitignore

@@ -1,43 +1,6 @@
-*mod
+# See also .gitignore files in doc/, tests/ and src/
 make.vars
-src/libabin.a
-*.o
 *.gcda
 *.gcno
 *.gcov
-*temp
-tests/*/*diff
-tests/*/*dat
-tests/*/*out
-tests/*/*bin
-tests/*/*bin.??
-tests/*/*bin.old
-tests/*/*dat.??
-tests/*/*xyz
-tests/*/*xyz.?
-tests/*/*xyz.??
-tests/*/geom.dat.00*
-tests/*/geom_mm.dat.00*
-tests/REMD/DFTB/OUT*
-tests/*/output
-tests/REMD/restart.xyz.??.*
-tests/*/restart.xyz.old
-tests/*/restart.xyz.0?
-tests/*/restart.xyz.10
-tests/*/restart.xyz.0?.old
-tests/*/debug.nacm
-tests/*/ERROR
-tests/SH_BUTCHER/geom.?.?.?
-tests/CP2K*/WATER*
-tests/PLUMED/*
-docs/*.log
-docs/*.bbl
-docs/*.aux
-docs/*.blg
-docs/*.toc
-docs/*out
-# Default binary name
-src/abin
-src/abin.*
-# Include mini.dat, needed for tests
-!mini.dat
+*.mod

docs/.gitignore

@@ -0,0 +1,6 @@
+*.log
+*.bbl
+*.aux
+*.blg
+*.toc
+*out

src/.gitignore

@@ -0,0 +1,5 @@
+*mod
+libabin.a
+*.o
+# Default binary name
+abin

src/surfacehop.F90

@@ -758,6 +758,8 @@ subroutine surfacehop(x, y, z, vx, vy, vz, vx_old, vy_old, vz_old, dt, eclas)
             write(chist,*)ist
             write(chihop,*)ihop
             write(chit,*)it
+            ! TODO: Rename this file to something more specific,
+            ! e.g. geom_hop.from.to.timestep.xyz
             formt='geom.'//trim(adjustl(chist))//'.'//trim(adjustl(chihop))//'.'//adjustl(chit)
             open(500,file=trim(formt))
             write(500,*)natom

tests/.gitignore

@@ -0,0 +1,24 @@
+*/*diff
+*/*dat
+*/*out
+*/*bin
+*/*bin.??
+*/*bin.old
+*/*dat.??
+*/*xyz
+*/*xyz.?
+*/*xyz.??
+*/geom.dat.00*
+*/geom_mm.dat.00*
+REMD/DFTB/OUT*
+*/abin.out
+REMD/restart.xyz.??.*
+*/restart.xyz.old
+*/restart.xyz.0?
+*/restart.xyz.10
+*/restart.xyz.0?.old
+*/debug.nacm
+*/ERROR
+SH_BUTCHER/geom.?.?.?
+CP2K*/WATER*
+PLUMED/*

tests/ABINITIO/mini.dat tests/ABINITIO/mini.xyz

@@ -0,0 +1,3 @@
+#!/bin/bash 
+
+rm cp2k.inp.* WATER?-RESTART.wfn* movie.xyz restart.xyz cp2k.out*

tests/CMD/mini.dat tests/CMD/mini.xyz

@@ -1,17 +1,16 @@
 #!/bin/bash
 set -euo pipefail
 
-# Parameters passed from Makefile
-ABINEXE="$PWD/src/$1 -x mini.dat"
-
-# verify that the number of parameters is correct!
+# Parameters are passed from Makefile.
 if [[ $# -ne 7 ]]; then
   echo "ERROR: Incorrect number of parameters passed to $0"
   echo "Invoked as:"
   echo "$0 $@"
   exit 1
 fi
 
+ABINEXE="$PWD/src/$1 -x mini.xyz"
+ABINOUT="abin.out"
 TESTS="$2"
 MPI="$3"
 FFTW="$4"
@@ -24,79 +23,100 @@ ACTION="$7"
 # It's not needed if you use system-wide MPI installation
 # or if mpirun is in your PATH.
 
-if [[ $ACTION = "makeref" ]];then
+if [[ $ACTION = "makeref" && $TESTS = "all" ]];then
   echo "ERROR: You should not call makeref on all tests at once."
   echo "Specify a concrete test which you want to modify, e.g."
-  echo "make test TEST=CMD"
+  echo "make makeref TEST=CMD"
   exit 1
 fi
 
 cd $(dirname $0)
 TESTDIR=$PWD
 
-function dif_files {
-local status=0
-local cont
-local files
-local f
-# Do comparison for all existing reference files
-files=$(ls *.ref)
-for f in $files   # $* 
-do
-   file=$(basename $f .ref)
-   if [[ -e $file.ref ]];then  # this should now be always true
-      error_code=0
-      diff -q $file $file.ref || error_code=$?
-      if [[ $error_code -eq 2 ]];then
-        # This means the $file does not exist.
-        # Something is seriously wrong, ABIN probably crashed prematurely.
-        # No need for further checks, exit NOW.
-        return $error_code
-
-      elif [[ $error_code -ne 0 ]];then
-         # The reference file is different, but maybe it's just numerical noise?
-         error_code=0
-         diff -y -W 500  $file $file.ref | egrep -e '|' -e '<' -e '>' > $file.diff
-         ../numdiff.py $file.diff || error_code=$?
-         if [[ $error_code -ne 0 ]];then
-            # The changes were bigger that the thresholds specified in numdiff.py
-            status=$error_code
-            echo "File $file differs from the reference."
-         fi
-      fi
-   fi
-done
-return $status
+function diff_files {
+  local return_status=0
+  local error_code
+  local ref_file
+  local test_file
+  # Compare test results with all existing reference files
+  local reference_files=$(ls *.ref)
+  if [[ -z $reference_files ]];then
+    echo "ERROR: No reference files were found"
+    return 1
+  fi
+
+  for ref_file in $reference_files
+  do
+    test_file=$(basename $ref_file .ref)
+    if [[ ! -f $test_file ]];then
+      # The output file does not exist.
+      # Something went seriously wrong, ABIN probably crashed prematurely.
+      # No need for further checks, exit NOW.
+      echo "ERROR: Could not find output file \"$test_file\""
+      return 1
+    fi
+
+    error_code=0
+    diff -q $test_file $ref_file || error_code=$?
+    if [[ $error_code -ne 0 ]];then
+       # The reference file is different, but maybe it's just numerical noise?
+       error_code=0
+       diff -y -W 500  $test_file $ref_file | egrep -e '|' -e '<' -e '>' > $test_file.diff
+
+       ../numdiff.py $test_file.diff || error_code=$?
+
+       if [[ $error_code -ne 0 ]];then
+          # The changes were bigger that the thresholds specified in numdiff.py
+          return_status=$error_code
+          echo "File $test_file differs from the reference."
+       fi
+    fi
+  done
+  return $return_status
 }
 
+# Update already existing reference files.
+# Called by `make makeref TEST=TEST_FOLDER`
+# If you're creating a completely new test,
+# you need to create the reference files manually.
 function makeref {
-local files
-local f
-echo "Making new reference files."
-files=$(ls *.ref)
-for f in $files 
-do
-   file=$(basename $f .ref)
-   if [[ -f $file.ref ]];then
-      mv $file $file.ref
-   fi
-done
+  local ref_file
+  local test_file
+  local reference_files=$(ls *.ref)
+  echo "Making new reference files."
+  if [[ -z $reference_files ]];then
+    echo "ERROR: No reference files were found."
+    exit 1
+  fi
+  for ref_file in $reference_files
+  do
+    test_file=$(basename $ref_file .ref)
+    if [[ ! -f $test_file ]];then
+      echo "ERROR: Could not find output file \"$test_file\""
+      exit 1
+    fi
+    echo "mv $test_file $ref_file"
+    mv $test_file $ref_file
+  done
 }
 
 function clean {
-rm -rf $* output
-rm -f *.diff
-if [[ -e "restart.xyz.0.ref" ]];then
-   cp restart.xyz.0.ref restart.xyz
-fi
+  if [[ -f "test.sh" ]];then
+    ./test.sh clean
+  fi
+  rm -rf $*
+  rm -f *.diff
 }
 
-files=( *-RESTART.wfn* cp2k.out bkl.dat phase.dat wfcoef.dat restart_sh.bin restart_sh.bin.old restart_sh.bin.?? nacm_all.dat minimize.dat geom.mini.xyz temper.dat temper.dat radius.dat vel.dat cv.dat cv_dcv.dat  dist.dat angles.dat dihedrals.dat geom.dat.??? geom_mm.dat.??? DYN/OUT* MM/OUT* state.dat stateall.dat stateall_grad.dat ERROR debug.nacm dotprod.dat pop.dat prob.dat PES.dat energies.dat est_energy.dat movie.xyz movie.xyz movie_mini.xyz restart.xyz.old restart.xyz.? restart.xyz.?? restart.xyz SOC.dat )
+# List of all possible ABIN output files.
+# Used by `make testclean` to cleanup test directories.
+output_files=( *.dat *.out ERROR movie.xyz forces.xyz velocities.xyz
+geom.dat.??? geom_mm.dat.??? geom.mini.xyz nacmrest.dat.?? geom.?.?.? bck.* DYN/OUT* MM/OUT*
+restart_sh.bin restart_sh.bin.old restart_sh.bin.?? restart.xyz.old restart.xyz.? restart.xyz.?? restart.xyz )
 
 # Run all tests
-if  [[ $TESTS = "all" ]];then
-   #folders=( CMD SH_EULER SH_RK4 SH_BUTCHER SH_RK4_PHASE PIMD SHAKE HARMON MINI QMMM GLE PIGLE)
-   # DH: Temporarily disable GLE and PIGLE tests
+if [[ $TESTS = "all" ]];then
+   # TODO: Re-enable GLE and PIGLE tests!
    folders=(CMD SH_EULER SH_RK4 SH_BUTCHER SH_RK4_PHASE LZ_SS LZ_ST LZ_ENE PIMD SHAKE HARMON MINI QMMM)
 
    let index=${#folders[@]}+1
@@ -114,9 +134,9 @@ if  [[ $TESTS = "all" ]];then
    fi
 
    if [[ $CP2K = "TRUE" ]];then
-      # At this point, we do not support MMWATER potential 
-      # with CP2K, which is used in majority of tests
-      # ABINITIO needs OpenMP, which is not compatible with CP2K interface
+      # At this point, we do not support MMWATER potential.
+      # with CP2K, which is used in majority of tests.
+      # ABINITIO needs OpenMP, which is not compatible with CP2K interface.
       folders=(SH_BUTCHER HARMON CP2K CP2K_MPI)
    fi
 
@@ -154,13 +174,8 @@ do
    echo "Entering directory $dir"
    cd $dir
 
-   # Always clean test directory
-   # before runnning the test
-   if [[ -f "test.sh" ]];then
-      ./test.sh clean
-   else
-      clean ${files[@]}
-   fi
+   # Always clean the test directory before runnning the test.
+   clean ${output_files[@]}
 
    # If we just want to clean the directories,
    # we skip the the actual test here
@@ -170,39 +185,42 @@ do
       continue
    fi
 
-   # Redirection to dev/null apparently needed for CP2K tests.
-   # Otherwise, STDIN is screwed up. I have no idea why.
-   # http://stackoverflow.com/questions/1304600/read-error-0-resource-temporarily-unavailable
-   # TODO: Figure out a different solution
+   # For special cases such as REMD, we need a more complicated
+   # test setup. If a file, test.sh is present in the test directory,
+   # we will use it.
    if [[ -f "test.sh" ]];then
 
+      # Redirection to dev/null apparently needed for CP2K tests.
+      # Otherwise, STDIN is screwed up. I have no idea why.
+      # http://stackoverflow.com/questions/1304600/read-error-0-resource-temporarily-unavailable
+      # TODO: Figure out a different solution
       #./test.sh $ABINEXE 2> /dev/null
       ./test.sh $ABINEXE || true
 
    else
       if [[ -f "velocities.in" ]];then
-         $ABINEXE -v "velocities.in" > output || true
+         $ABINEXE -v "velocities.in" > $ABINOUT || true
       else
-         $ABINEXE > output || true
+         $ABINEXE > $ABINOUT || true
       fi
 
       #for testing restart
       if [[ -e input.in2 ]];then
-         $ABINEXE -i input.in2 >> output || true
+         $ABINEXE -i input.in2 >> $ABINOUT || true
       fi
    fi
 
    if [[ $ACTION = "makeref" ]];then
 
-      makeref ${files[@]}
+      makeref
 
    else
 
       # Since we're running in the -e mode,
       # we need to "hide" this possibly failing command
       # https://stackoverflow.com/a/11231970/3682277
       current_error=0
-      dif_files ${files[@]} || current_error=$?
+      diff_files || current_error=$?
       if [[ $current_error -ne 0 ]];then
         global_error=1
         echo "$dir FAILED"