Miscellaneous Post MHD Tweaks/Fixes

Makefile

@@ -204,9 +204,11 @@ tidy:
 # Flags we might want
 # - --warnings-as-errors=<string> Upgrade all warnings to error, good for CI
 	clang-tidy --verify-config
+	@echo -e
 	(time clang-tidy $(CLANG_TIDY_ARGS) $(CPPFILES_TIDY) -- $(DFLAGS) $(CXXFLAGS_CLANG_TIDY) $(LIBS_CLANG_TIDY)) > tidy_results_cpp.log 2>&1 & \
 	(time clang-tidy $(CLANG_TIDY_ARGS) $(GPUFILES_TIDY) -- $(DFLAGS) $(GPUFLAGS_CLANG_TIDY) $(LIBS_CLANG_TIDY)) > tidy_results_gpu.log 2>&1 & \
 	for i in 1 2; do wait -n; done
+	@echo -e "\nResults from clang-tidy are available in the 'tidy_results_cpp.log' and 'tidy_results_gpu.log' files."
 
 clean:
 	rm -f $(CLEAN_OBJS)

src/chemistry_gpu/chemistry_functions.cpp

@@ -254,7 +254,10 @@ void Grid3D::Compute_Gas_Temperature(Real *temperature, bool convert_cosmo_units
   #ifdef DE
         GE = C.GasEnergy[id];
   #else
-        GE = (E - 0.5 * d * (vx * vx + vy * vy + vz * vz));  // TODO: this probably needs to be fixed for MHD
+        GE = E - hydro_utilities::Calc_Kinetic_Energy_From_Velocity(d, vx, vy, vz);
+    #ifdef MHD
+        GE -= mhd::utils::computeMagneticEnergy(C.magnetic_x[id], C.magnetic_y[id], C.magnetic_z[id]);
+    #endif  // MHD
   #endif
 
         dens_HI    = C.HI_density[id];

src/io/io_tests.cpp

@@ -34,7 +34,11 @@ TEST(tHYDROtMHDReadGridHdf5, RestartSlowWaveExpectCorrectOutput)
   loadRun.numMpiRanks = num_ranks;
   loadRun.chollaLaunchParams.append(" init=Read_Grid nfile=0 indir=" + read_directory);
 
+#ifdef MHD
   loadRun.setFiducialNumTimeSteps(854);
+#else   // not MHD
+  loadRun.setFiducialNumTimeSteps(427);
+#endif  // MHD
   loadRun.runL1ErrorTest(4.2E-7, 5.4E-7);
 }
 // =============================================================================

src/riemann_solvers/hlld_cuda.cu

@@ -189,9 +189,7 @@ __device__ __host__ mhd::_internal::State loadState(Real const *interfaceArr, Re
     #endif  // SCALAR
     #ifdef DE
   state.thermalEnergySpecific = interfaceArr[threadId + n_cells * grid_enum::GasEnergy] / state.density;
-    #endif  // DE}
 
-    #ifdef DE  // PRESSURE_DE
   Real energyNonThermal = hydro_utilities::Calc_Kinetic_Energy_From_Velocity(state.density, state.velocityX,
                                                                              state.velocityY, state.velocityZ) +
                           mhd::utils::computeMagneticEnergy(magneticX, state.magneticY, state.magneticZ);
@@ -203,7 +201,7 @@ __device__ __host__ mhd::_internal::State loadState(Real const *interfaceArr, Re
   // Note that this function does the positive pressure check
   // internally
   state.gasPressure = mhd::utils::computeGasPressure(state, magneticX, gamma);
-    #endif  // PRESSURE_DE
+    #endif  // DE
 
   state.totalPressure =
       mhd::utils::computeTotalPressure(state.gasPressure, magneticX, state.magneticY, state.magneticZ);

src/system_tests/system_tester.cpp

@@ -11,6 +11,7 @@
 #include <algorithm>
 #include <cmath>
 #include <cstdio>
+#include <filesystem>
 #include <fstream>
 #include <limits>
 #include <numeric>
@@ -55,17 +56,15 @@ void systemTest::SystemTestRunner::runTest()
   _testParticlesFileVec.resize(numMpiRanks);
   for (size_t fileIndex = 0; fileIndex < numMpiRanks; fileIndex++) {
     // Load the hydro data
-    if (_hydroDataExists) {
-      std::string fileName = "/1.h5." + std::to_string(fileIndex);
-      _checkFileExists(_outputDirectory + fileName);
-      _testHydroFieldsFileVec[fileIndex].openFile(_outputDirectory + fileName, H5F_ACC_RDONLY);
+    std::string filePath = _outputDirectory + "/1.h5." + std::to_string(fileIndex);
+    if (_hydroDataExists and std::filesystem::exists(filePath)) {
+      _testHydroFieldsFileVec[fileIndex].openFile(filePath, H5F_ACC_RDONLY);
     }
 
     // Load the particles data
-    if (_particleDataExists) {
-      std::string fileName = "/1_particles.h5." + std::to_string(fileIndex);
-      _checkFileExists(_outputDirectory + fileName);
-      _testParticlesFileVec[fileIndex].openFile(_outputDirectory + fileName, H5F_ACC_RDONLY);
+    filePath = _outputDirectory + "/1_particles.h5." + std::to_string(fileIndex);
+    if (_particleDataExists and std::filesystem::exists(filePath)) {
+      _testParticlesFileVec[fileIndex].openFile(filePath, H5F_ACC_RDONLY);
     }
   }
 
@@ -207,14 +206,16 @@ void systemTest::SystemTestRunner::runL1ErrorTest(double const &maxAllowedL1Erro
   std::vector<H5::H5File> initialHydroFieldsFileVec(numMpiRanks);
   for (size_t fileIndex = 0; fileIndex < numMpiRanks; fileIndex++) {
     // Initial time data
-    std::string fileName = "/0.h5." + std::to_string(fileIndex);
-    _checkFileExists(_outputDirectory + fileName);
-    initialHydroFieldsFileVec[fileIndex].openFile(_outputDirectory + fileName, H5F_ACC_RDONLY);
+    std::string filePath = _outputDirectory + "/0.h5." + std::to_string(fileIndex);
+    if (std::filesystem::exists(filePath)) {
+      initialHydroFieldsFileVec[fileIndex].openFile(filePath, H5F_ACC_RDONLY);
+    }
 
     // Final time data
-    fileName = "/1.h5." + std::to_string(fileIndex);
-    _checkFileExists(_outputDirectory + fileName);
-    _testHydroFieldsFileVec[fileIndex].openFile(_outputDirectory + fileName, H5F_ACC_RDONLY);
+    filePath = _outputDirectory + "/1.h5." + std::to_string(fileIndex);
+    if (std::filesystem::exists(filePath)) {
+      _testHydroFieldsFileVec[fileIndex].openFile(filePath, H5F_ACC_RDONLY);
+    }
   }
 
   // Get the list of test dataset names
@@ -264,18 +265,25 @@ void systemTest::SystemTestRunner::runL1ErrorTest(double const &maxAllowedL1Erro
         << "The initial and final '" << dataSetName << "' datasets are not the same length";
 
     // Compute the L1 Error.
-    double L1Error = 0;
+    double L1_error     = 0.0;
+    double fp_sum_error = 0.0;
     for (size_t i = 0; i < initialData.size(); i++) {
       double const diff = std::abs(initialData.at(i) - finalData.at(i));
-      L1Error += diff;
-      maxError = (diff > maxError) ? diff : maxError;
+
+      maxError = std::max(maxError, diff);
+
+      // Perform a Kahan sum to maintain precision in the result
+      double const y = diff - fp_sum_error;
+      double const t = L1_error + y;
+      fp_sum_error   = (t - L1_error) - y;
+      L1_error       = t;
     }
 
-    L1Error *= (1. / static_cast<double>(initialDims[0] * initialDims[1] * initialDims[2]));
-    L2Norm += L1Error * L1Error;
+    L1_error /= static_cast<double>(initialDims[0] * initialDims[1] * initialDims[2]);
+    L2Norm += L1_error * L1_error;
 
     // Perform the correctness check
-    EXPECT_LT(L1Error, maxAllowedL1Error)
+    EXPECT_LT(L1_error, maxAllowedL1Error)
         << "the L1 error for the " << dataSetName << " data has exceeded the allowed value";
   }
 
@@ -304,9 +312,13 @@ void systemTest::SystemTestRunner::launchCholla()
   EXPECT_EQ(returnLaunch, 0) << "Warning: Launching Cholla returned a non-zero exit status. Likely "
                              << "failed to launch. Please see the log files" << std::endl;
 
-  _safeMove("run_output.log", _outputDirectory);
-  // TODO: instead of commenting out, change to check if exist
-  //_safeMove("run_timing.log", _outputDirectory);
+  // Move the output files to the correct spots
+  std::filesystem::rename(::globalChollaRoot.getString() + "/run_output.log", _outputDirectory + "/run_output.log");
+  try {
+    std::filesystem::rename(::globalChollaRoot.getString() + "/run_timing.log", _outputDirectory + "/run_timing.log");
+  } catch (const std::filesystem::filesystem_error &error) {
+    // This file might not exist and isn't required so don't worry if it doesn't exist
+  }
 }
 // =============================================================================
 
@@ -315,9 +327,10 @@ void systemTest::SystemTestRunner::openHydroTestData()
 {
   _testHydroFieldsFileVec.resize(numMpiRanks);
   for (size_t fileIndex = 0; fileIndex < numMpiRanks; fileIndex++) {
-    std::string fileName = "/1.h5." + std::to_string(fileIndex);
-    _checkFileExists(_outputDirectory + fileName);
-    _testHydroFieldsFileVec[fileIndex].openFile(_outputDirectory + fileName, H5F_ACC_RDONLY);
+    std::string filePath = _outputDirectory + "/1.h5." + std::to_string(fileIndex);
+    if (std::filesystem::exists(filePath)) {
+      _testHydroFieldsFileVec[fileIndex].openFile(filePath, H5F_ACC_RDONLY);
+    }
   }
 }
 // =============================================================================
@@ -389,22 +402,32 @@ systemTest::SystemTestRunner::SystemTestRunner(bool const &particleData, bool co
   _fullTestFileName        = fullTestName.substr(0, fullTestName.find("/"));
 
   // Generate the input paths. Strip out everything after a "/" since that
-  // probably indicates a parameterized test. Also, check that the files exist
-  // and load fiducial HDF5 file if required
+  // probably indicates a parameterized test.
   _chollaPath = ::globalChollaRoot.getString() + "/bin/cholla." + ::globalChollaBuild.getString() + "." +
                 ::globalChollaMachine.getString();
-  _checkFileExists(_chollaPath);
+
+  // Check that Cholla exists and abort if it doesn't
+  if (not std::filesystem::exists(_chollaPath)) {
+    throw std::invalid_argument("Error: Cholla executable not found.");
+  }
+
+  // Check that settings file exist
   if (useSettingsFile) {
     _chollaSettingsPath =
         ::globalChollaRoot.getString() + "/src/system_tests/input_files/" + _fullTestFileName + ".txt";
-    _checkFileExists(_chollaSettingsPath);
   } else {
     _chollaSettingsPath = ::globalChollaRoot.getString() + "/src/system_tests/input_files/" + "blank_settings_file.txt";
-    _checkFileExists(_chollaSettingsPath);
   }
+  if (not std::filesystem::exists(_chollaSettingsPath)) {
+    throw std::invalid_argument("Error: Cholla settings file not found at :" + _chollaSettingsPath);
+  }
+
+  // Check that the fiducial file exists and load it if it does
   if (useFiducialFile) {
     _fiducialFilePath = ::globalChollaRoot.getString() + "/cholla-tests-data/system_tests/" + _fullTestFileName + ".h5";
-    _checkFileExists(_fiducialFilePath);
+    if (not std::filesystem::exists(_fiducialFilePath)) {
+      throw std::invalid_argument("Error: Cholla settings file not found at :" + _fiducialFilePath);
+    }
     _fiducialFile.openFile(_fiducialFilePath, H5F_ACC_RDONLY);
     _fiducialDataSetNames = _findDataSetNames(_fiducialFile);
     _fiducialFileExists   = true;
@@ -447,31 +470,6 @@ systemTest::SystemTestRunner::~SystemTestRunner()
 // Private Members
 // =============================================================================
 
-// =============================================================================
-void systemTest::SystemTestRunner::_checkFileExists(std::string const &filePath)
-{
-  // TODO C++17 std::filesystem does this better
-  std::fstream file;
-  file.open(filePath);
-  if (not file) {
-    std::string errMessage = "Error: File '" + filePath + "' not found.";
-    throw std::invalid_argument(errMessage);
-  }
-}
-// =============================================================================
-
-// =============================================================================
-void systemTest::SystemTestRunner::_safeMove(std::string const &sourcePath, std::string const &destinationDirectory)
-{
-  // TODO C++17 std::filesystem does this better
-  _checkFileExists(sourcePath);
-  if (std::rename(sourcePath.c_str(), (destinationDirectory + "/" + sourcePath).c_str()) < 0) {
-    std::string errMessage = "Error: File '" + sourcePath + "' could not be moved to '" + destinationDirectory + "`";
-    throw std::invalid_argument(errMessage);
-  }
-}
-// =============================================================================
-
 // =============================================================================
 void systemTest::SystemTestRunner::_checkNumTimeSteps()
 {

src/system_tests/system_tester.h

@@ -321,24 +321,6 @@ class systemTest::SystemTestRunner
   /// no particle data
   bool _particleDataExists = false;
 
-  /*!
-   * \brief Move a file. Throws an exception if the file does not exist.
-   * or if the move was unsuccessful
-   *
-   * \param[in] sourcePath The path the the file to be moved
-   * \param[in] destinationDirectory The path to the director the file should
-   * be moved to
-   */
-  void _safeMove(std::string const &sourcePath, std::string const &destinationDirectory);
-
-  /*!
-   * \brief Checks if the given file exists. Throws an exception if the
-   * file does not exist.
-   *
-   * \param[in] filePath The path to the file to check for
-   */
-  void _checkFileExists(std::string const &filePath);
-
   /*!
    * \brief Using GTest assertions to check if the fiducial and test data have
    * the same number of time steps