[newchem-cpp] mv grtest machinery into its own subdirectory

.clang-format-ignore

@@ -64,17 +64,17 @@ src/include/grackle_float.h.in
 src/include/grackle_misc.h
 src/include/grackle_rate_functions.h
 src/include/grackle_types.h
-tests/unit/grtest_cmd.cpp
-tests/unit/grtest_cmd.hpp
-tests/unit/grtest_os.cpp
-tests/unit/grtest_os.hpp
-tests/unit/grtest_utils.cpp
-tests/unit/grtest_utils.hpp
+tests/grtestutils/cmd.cpp
+tests/grtestutils/cmd.hpp
+tests/grtestutils/os.cpp
+tests/grtestutils/os.hpp
+tests/grtestutils/utils.cpp
+tests/grtestutils/utils.hpp
+tests/grtestutils/googletest/check_allclose.hpp
 tests/unit/test_chemistry_struct_synced.cpp
 tests/unit/test_ghost_zone.cpp
 tests/unit/test_interpolators_comparisons.cpp
 tests/unit/test_linalg.cpp
 tests/unit/test_status_reporting.cpp
 tests/unit/test_unit_interpolators_g.cpp
-tests/unit/utest_helpers.hpp
 

src/clib/chemistry_solver_funcs.hpp

@@ -1,6 +1,11 @@
-// See LICENSE file for license and copyright information
-
-/// @file chemistry_solver_funcs.hpp
+//===----------------------------------------------------------------------===//
+//
+// See the LICENSE file for license and copyright information
+// SPDX-License-Identifier: NCSA AND BSD-3-Clause
+//
+//===----------------------------------------------------------------------===//
+///
+/// @file
 /// @brief Defines chemistry reaction related functions invoked by the
 ///     grackle solver in order to integrate the species densities over time.
 ///
@@ -14,6 +19,8 @@
 ///     be decoupled from the derivative calculation for primoridial species
 ///   - it may also make sense to further divide logic by the kinds of species
 ///     that are affected (e.g. primordial vs grains)
+///
+//===----------------------------------------------------------------------===//
 
 #ifndef CHEMISTRY_SOLVER_FUNCS_HPP
 #define CHEMISTRY_SOLVER_FUNCS_HPP
@@ -579,7 +586,9 @@ inline void species_density_updates_gauss_seidel(
 
         if ( (my_chemistry->metal_chemistry == 1)  && 
              (itmask_metal[i] != MASK_FALSE) )  {
-          scoef = scoef;
+          // we comment out the following line that assigns scoef to itself since
+          // it has no practical impact and produces a compiler warning
+          // scoef = scoef;
           acoef = acoef
               + kcol_buf.data[CollisionalRxnLUT::kz44][i] *   CII(i,j,k) / 12.
               + kcol_buf.data[CollisionalRxnLUT::kz45][i] *   OII(i,j,k) / 16.
@@ -1932,7 +1941,9 @@ inline void species_density_derivatives_0d(
 
     if ((my_chemistry->metal_chemistry == 1)  && 
         (itmask_metal[0] != MASK_FALSE))  {
-      scoef = scoef;
+      // we comment out the following line that assigns scoef to itself since
+      // it has no practical impact and produces a compiler warning
+      // scoef = scoef;
       acoef = acoef
           + kcr_buf.data[CollisionalRxnLUT::kz44][0]    *   CII        / 12.
           + kcr_buf.data[CollisionalRxnLUT::kz45][0]    *   OII        / 16.

src/clib/solve_rate_cool_g-cpp.cpp

@@ -791,7 +791,7 @@ int solve_rate_cool_g(
       // declare 2 variables (primarily used for subcycling, but also used in
       // error reporting)
       int iter;
-      double ttmin;
+      double ttmin = huge8;
 
       // ------------------ Loop over subcycles ----------------
 

tests/CMakeLists.txt

@@ -1,3 +1,4 @@
+add_subdirectory(grtestutils)
 add_subdirectory(unit)
 
 # down below, we add tests for various code-examples

tests/grtestutils/CMakeLists.txt

@@ -0,0 +1,138 @@
+# Purpose: define "libraries" to aid with the testing of Grackle
+# ==============================================================
+
+# first, define an interface "library" (i.e. nothing is compiled) for
+# book-keeping purposes. dependents get "linked" against this library in order
+# to be able to access grackle's internal functionality
+
+add_library(_grackle_internals INTERFACE)
+target_link_libraries(_grackle_internals INTERFACE Grackle::Grackle)
+
+# short-term macro definitions (both will become unnecessary in the near future)
+target_compile_definitions(_grackle_internals
+  # currently required for functions declared by fortran_func_wrappers.hpp
+  INTERFACE "$<$<PLATFORM_ID:Linux,Darwin>:LINUX>"
+  # suppresses warnings about C internal headers using our deprecated public
+  # headers (the files should include grackle.h instead). We're currently
+  # holding off on properly fixing this to minimize conflicts with pending PRs
+  # on the newchem-cpp branch
+  INTERFACE GRIMPL_PUBLIC_INCLUDE=1
+)
+
+# this makes it possible to write an include-directive of a header from
+# src/clib, by writing something like
+#   #include "{name}.hpp"
+# where {name} is replaced by the name of the header
+# -> frankly, I don't love this because it's not immediately clear that we are
+#    including a private header. The alternative is to require the paths
+#    in the include directives to include the name of the directory holding
+#    the implementation files (e.g. "clib/{header}.hpp")
+target_include_directories(_grackle_internals
+  INTERFACE ${PROJECT_SOURCE_DIR}/src/clib
+)
+
+# grtestutils: testing utility library
+# ====================================
+# - this is an internal library that defines reusable testing utilities
+# - Frankly, I don't love the name. Originally I called it grtest, but I'm a
+#   a little concerned that may look a little too much like gtest (i.e. the
+#   shorthand that googletest uses).
+
+# Code Organization
+# -----------------
+# This code in this directory is organized in a slightly peculiar manner
+# - code in the main directory should not depend upon googletest
+# - the googletest subdirectory is intended to define custom googletest
+#   assertions and fixtures. This subdirectory should **ONLY** include
+#   headers (so that, at most, this internal library has a transitive
+#   dependence on googletest)
+#
+# RATIONALE:
+# - over time, the idea has been floated to introducing C++ logic to
+#   accelerate certain calculations provided by the python wrapper:
+#   - PR #343 proposed creating a C++ accelerated harness for performing
+#     certain kinds of semi-analytic calculations (like freefall or evolving
+#     constant density)
+#   - PR #370 proposed introducing a function to infer the internal energy at
+#     a desired temperature (yes, the PR proposes it as a python function, but
+#     it would be faster as a C++ function).
+#   - Relatedly, it would also be useful to be able to directly compute
+#     temperature or chemical equilibrium
+# - in each case this functionality would also be extremely useful for the
+#   googletest framework or for writing simple C++-only benchmarking programs
+#   - this is extremely useful if you want to use tools like valgrind or
+#     compiler sanitizers. While it's generally possible to use these tools in
+#     a python extension module, it may require compiling CPython itself from
+#     source. Moreover it can be extremely slow (these tools introduce to
+#     basic C/C++ operations and that overhead will be apply to the Python
+#     interpretter)
+#   - this is important if we want to test grackle when compiled with various
+#     backends. Historically, the python bindings have never been able to use
+#     Grackle with the OpenMP backend. While we can and will address this,
+#     there will additional challenges as we introduce GPU acceleration
+# - this library is intended as a location where that kind of hypothetical
+#   functionality can be introduced. The organization strategy ensures that
+#   it will be straightforward to provide this hypothetical functionality to
+#   the python bindings (and any C++ testing code) without making the python
+#   bindings link against googletest.
+
+add_library(grtestutils
+  # files outside of the googletest subdirectory
+  # -> these shouldn't include headers from the googletest subdirectory
+  cmd.hpp cmd.cpp
+  utils.hpp utils.cpp
+  os.hpp os.cpp
+
+  # files in the googletest subdirectory (reminder: should just be headers!)
+  googletest/check_allclose.hpp
+)
+
+target_link_libraries(grtestutils
+  PRIVATE _grackle_internals
+  PUBLIC Grackle::Grackle
+  # to express the transitive dependency on googletest (it's transitive since
+  # its only referenced by header files), we should theoretically write
+  #   INTERFACE GTest::GTest
+  # we choose not to do this since all test code pulls in this dependency by
+  # explicitly listing `testdeps` as a dependency (defined below)
+)
+
+target_compile_features(grtestutils PUBLIC cxx_std_17)
+
+# configure the grtestutils target so that when a file outside of this
+# directory includes a header from within this directory, the include
+# directive looks something like:
+#    #include "grtestutils/<name>.hpp"
+# OR
+#    #include "grtestutils/googletest/<name>.hpp
+target_include_directories(grtestutils INTERFACE ${PROJECT_SOURCE_DIR}/tests)
+
+# these compile-definitions act as short-term hacks
+target_compile_definitions(grtestutils
+  # this hack helps us get the path to the directory holding data files (we can
+  # remove it once we introduce automatic file management in PR 235, PR 237,
+  # and PR 246)
+  PRIVATE GR_DATADIR=${CMAKE_CURRENT_SOURCE_DIR}/../../grackle_data_files/input/
+
+  # this hack lets us use Operating-system specific functionality (Once PR #237
+  # is merged, we should make use of the machinery introduced by that PR for
+  # enabling/disabling os-specific features)
+  PRIVATE "$<$<PLATFORM_ID:Linux,Darwin>:PLATFORM_GENERIC_UNIX>"
+)
+
+
+# 2. testdeps: bundles dependencies used by tests
+# ===============================================
+# - this is an interface library (i.e. nothing is compiled) that ONLY exists
+#   to aggregate dependencies, include directories and compile definitions
+# - all tests should depend on this target
+
+add_library(testdeps INTERFACE)
+target_link_libraries(testdeps
+  INTERFACE Grackle::Grackle grtestutils _grackle_internals
+            GTest::gtest_main GTest::gmock
+)
+# indicates that all consumers are written in C++17 or newer
+target_compile_features(testdeps INTERFACE cxx_std_17)
+
+

tests/grtestutils/README.md

@@ -0,0 +1,3 @@
+Defines a simple testing utility library.
+
+See the CMakeLists.txt for a discussion of code organization.

tests/unit/grtest_cmd.cpp → tests/grtestutils/cmd.cpp

@@ -1,18 +1,13 @@
-#include "grtest_cmd.hpp"
+#include "cmd.hpp"
+
+// internal Grackle routine:
+#include "status_reporting.h"  // GR_INTERNAL_ERROR, GR_INTERNAL_REQUIRE
 
 #include <stdio.h>  // FILE, fgets, fprintf, stderr, (popen/pclose on POSIX)
 
-#include <cstdio>  // std::exit
 #include <sstream>  // std::stringstream
 #include <string>
 
-// TODO: replace with Grackle's existing err machinery
-[[noreturn]] static void err_(std::string msg="") {
-  const char* ptr = (msg.empty()) ? "<unspecified>" : msg.c_str();
-  fprintf(stderr, "ERROR: %s\n", ptr);
-  std::exit(1);
-}
-
 #ifdef PLATFORM_GENERIC_UNIX
 
 #define TEMP_BUF_SIZE 128
@@ -29,14 +24,14 @@ grtest::ProcessStatusAndStdout grtest::capture_status_and_output(
 
   // fp represents a buffered pipe to the standard output of the command.
   FILE* fp = popen(command.data(), "r");
-  if (fp == nullptr) { err_("there was a problem launching the command"); }
+  GR_INTERNAL_REQUIRE(fp != nullptr, "there's a problem launching the command");
 
   // if our reads from the pipe outpace the rate at which the command writes to
   // the pipe, fgets won't return until more data becomes available. If the
   // processess ends, fgets encounters EOF (causing fgets to return)
   while (fgets(temp_buf, TEMP_BUF_SIZE, fp) != nullptr) { buf << temp_buf; }
   int status = pclose(fp);
-  if (status == -1) { err_("there was a problem closing the command"); }
+  GR_INTERNAL_REQUIRE(status != -1, "there was a problem closing the command");
   return {status, buf.str()};
 }
 
@@ -45,7 +40,7 @@ grtest::ProcessStatusAndStdout grtest::capture_status_and_output(
 grtest::ProcessStatusAndStdout grtest::capture_status_and_output(
   const std::string& command
 ) {
-  err_("not implemented on this platform");
+  GR_INTERNAL_ERROR("not implemented on this platform");
 }
 
 #endif /* PLATFORM_GENERIC_UNIX */

tests/unit/grtest_os.cpp → tests/grtestutils/os.cpp

@@ -1,4 +1,4 @@
-#include "grtest_os.hpp"
+#include "os.hpp"
 
 // the following 2 headers are the c versions of the headers (rather than the
 // C++ versions) since it seems more likely that posix-specific functions are

tests/unit/grtest_utils.cpp → tests/grtestutils/utils.cpp

@@ -1,4 +1,4 @@
-#include "grtest_utils.hpp"
+#include "utils.hpp"
 
 #include <cstring>
 

tests/unit/CMakeLists.txt

@@ -1,56 +1,7 @@
-# declare testdeps to bundle together dependencies used by all tests
-# ------------------------------------------------------------------
-add_library(testdeps INTERFACE)
-target_link_libraries(testdeps INTERFACE Grackle::Grackle GTest::gtest_main)
+# NOTE: the testdeps target is defined within ../grtestutils/CMakeLists.txt
 
-# compiler defs are short-term hacks to help tests invoke internal functions
-# from Grackle (both of these will become unnecessary in the near future)
-target_compile_definitions(testdeps
-  # needed to invoke Fortran functions from C
-  INTERFACE "$<$<PLATFORM_ID:Linux,Darwin>:LINUX>"
-  # suppresses warnings about C internal headers using our deprecated public
-  # headers (the files should include grackle.h instead). We're currently
-  # holding off on properly fixing this to minimize conflicts with pending PRs
-  # on the newchem-cpp branch
-  INTERFACE GRIMPL_PUBLIC_INCLUDE=1
-)
-
-# needed to let tests call internal functions from C
-target_include_directories(testdeps INTERFACE ${PROJECT_SOURCE_DIR}/src/clib)
-target_compile_features(testdeps INTERFACE cxx_std_17)
-
-# declare the grtest utility library
-# ----------------------------------
-# -> this is an internal library that defines reusable testing utilities
-# -> if we add more files to this library, we should consider relocating the
-#    library to a different directory
-
-add_library(grtest_utils
-  grtest_cmd.hpp grtest_cmd.cpp
-  grtest_utils.hpp grtest_utils.cpp
-  grtest_os.hpp grtest_os.cpp
-)
-# we are being a little lazy with our usage of testdeps right here
-target_link_libraries(grtest_utils PUBLIC testdeps)
-target_compile_features(grtest_utils PUBLIC cxx_std_17)
-
-# these compile-definitions act as short-term hacks
-target_compile_definitions(grtest_utils
-  # this hack helps us get path input-file directory (we can remove it once we
-  # introduce automatic file management in PR 235, PR 237, and PR 246)
-  PRIVATE GR_DATADIR=${CMAKE_CURRENT_SOURCE_DIR}/../../grackle_data_files/input/
-
-  # this hack lets us use Operating-system specific functionality (Once PR #237
-  # is merged, we should make use of the machinery introduced by that PR for
-  # enabling/disabling os-specific features)
-  PRIVATE "$<$<PLATFORM_ID:Linux,Darwin>:PLATFORM_GENERIC_UNIX>"
-)
-
-# start declaring targets for tests
-# ---------------------------------
 add_executable(runInterpolationTests test_unit_interpolators_g.cpp)
 target_link_libraries(runInterpolationTests testdeps)
-
 gtest_discover_tests(runInterpolationTests)
 
 add_executable(runLinAlgTests test_linalg.cpp)
@@ -62,7 +13,7 @@ target_link_libraries(runGrainSpeciesIntoTests testdeps)
 gtest_discover_tests(runGrainSpeciesIntoTests)
 
 add_executable(runStatusReporting test_status_reporting.cpp)
-target_link_libraries(runStatusReporting grtest_utils)
+target_link_libraries(runStatusReporting testdeps)
 gtest_discover_tests(runStatusReporting)
 
 # one might argue that the following is more of an integration or end-to-end
@@ -74,7 +25,7 @@ gtest_discover_tests(runVisitorTests)
 # one might argue that the following is more of an integration or end-to-end
 # test than a unit-test
 add_executable(runGhostZoneTests test_ghost_zone.cpp)
-target_link_libraries(runGhostZoneTests grtest_utils testdeps)
+target_link_libraries(runGhostZoneTests testdeps)
 gtest_discover_tests(runGhostZoneTests)
 
 # this target tests that the members of the chemistry_data struct can be
@@ -84,7 +35,7 @@ gtest_discover_tests(runGhostZoneTests)
 # problems for "death-tests", so these test-cases should remain separate from
 # the rest of the gtest framework
 add_executable(runSyncedChemistryData test_chemistry_struct_synced.cpp)
-target_link_libraries(runSyncedChemistryData grtest_utils testdeps)
+target_link_libraries(runSyncedChemistryData testdeps)
 target_compile_definitions(runSyncedChemistryData
   PRIVATE
     READER_PATH=${PROJECT_SOURCE_DIR}/tests/scripts/castxml_output_reader.py

tests/unit/test_chemistry_struct_synced.cpp

@@ -1,4 +1,4 @@
-#include "grtest_cmd.hpp"
+#include "grtestutils/cmd.hpp"
 
 #include <grackle.h>
 

tests/unit/test_ghost_zone.cpp

@@ -24,7 +24,7 @@
 #include <string>
 #include <vector>
 
-#include "grtest_utils.hpp"
+#include "grtestutils/utils.hpp"
 
 #include <grackle.h>
 

tests/unit/test_linalg.cpp

@@ -2,7 +2,7 @@
 #include <gtest/gtest.h>
 
 #include "fortran_func_wrappers.hpp"
-#include "utest_helpers.hpp"
+#include "grtestutils/googletest/check_allclose.hpp"
 
 
 /// Records the paramters for a linear algebra test-case