Merge branch 'main' into new-ckpt-restart-tests

Author: mabruzzo

.circleci/config.yml

@@ -112,6 +112,10 @@ commands:
       usegrackle:
         type: boolean
         default: true
+      setupCTests:
+        type: boolean
+        default: true
+        description: whether to build CTest machinery (has no effect on pytest framework)
     steps:
       - run:
           name: "Checkout target tag from enzo-e repository."
@@ -128,9 +132,6 @@ commands:
             source $BASH_ENV
             source $HOME/venv/bin/activate
 
-            # convert boolean parameter to an env var storing 0 or 1
-            USE_DOUBLE=$(( 0 <<# parameters.usedouble >> + 1 <</ parameters.usedouble >> ))
-            USE_GRACKLE=$(( 0 <<# parameters.usegrackle >> + 1 <</ parameters.usegrackle >> ))
             # remove build directory in case we've already compiled before
             rm -rf build
 
@@ -139,11 +140,11 @@ commands:
                     -GNinja \
                     -DUSE_DOUBLE_PREC=<< parameters.usedouble >> \
                     -DUSE_GRACKLE=<< parameters.usegrackle >> \
+                    -DBUILD_TESTING=<< parameters.setupCTests >> \
                     -Bbuild \
                     -DPARALLEL_LAUNCHER_NPROC_ARG="++local;+p" \
                     -DPython3_FIND_VIRTUALENV=ONLY
               cmake --build build -j 4
-              source $HOME/venv/bin/activate
             fi
 
   run-ctests:
@@ -277,6 +278,7 @@ jobs:
           tag: tip
           skipfile: notafile
           usegrackle: << parameters.usegrackle >>
+          setupCTests: true
 
       - run-ctests:
           skipfile: notafile
@@ -321,6 +323,7 @@ jobs:
           tag: gold-standard-004
           skipfile: notafile
           usegrackle: << parameters.usegrackle >>
+          setupCTests: false
 
       - run-answer-tests:
           usedouble: << parameters.usedouble >>
@@ -332,6 +335,7 @@ jobs:
           tag: tip
           skipfile: notafile
           usegrackle: << parameters.usegrackle >>
+          setupCTests: false
 
       - run-answer-tests:
           usedouble: << parameters.usedouble >>

CMakeLists.txt

@@ -309,6 +309,13 @@ if (__processedUserDefaults)
   include(${Enzo-E_CONFIG_PATH})
 endif()
 
+# we need to pull CTest sooner or later anyways. We do it before adding the
+# source directories because it introduces the BUILD_TESTING option which is
+# set to ON if it was not previously initialized (e.g. from the command line)
+# -> this option is used within source directories to determine whether the
+#    binaries for unit-tests should be declared
+include(CTest)
+
 # In principle a more fine grained control (i.e., target specific include directories
 # rather than this global one would be preferred, but, e.g., `pngwriter.h` is curently
 # included in many targets/libraries (without being linked) so we'll use the global for
@@ -321,10 +328,7 @@ add_subdirectory(src/Enzo)
 add_subdirectory(src/External)
 
 
-include(CTest)
 
-# include(CTest) implicitly initializes BUILD_TESTING to ON if it was not
-# previously initialized (e.g. from the command line)
 if (BUILD_TESTING)
   add_subdirectory(test)
 endif()

doc/source/tests/pytest_framework.rst

@@ -9,8 +9,8 @@ simulation is run with two versions of ``enzo-e`` and their results are compared
 This is useful for testing problems with no analytical solution or generally
 verifying that results from commonly run simulations don't drift.
 
-It is also useful for testing properties of simulation runs beyond the exit-time/cycle of the simulation.
-(While such tests do exist in the ctest-framework, they often involve more boiler-plate code).
+It is also useful for testing problems that do have analytic solutions (the answer test might quantify how close a simulation result is to the analytic expected solution).
+While such tests do exist in the ctest-framework, they often involve more boiler-plate code.
 
 `pytest <https://docs.pytest.org/>`__ is a Python-based framework for detecting
 and running a series of tests within a source code repository. When running
@@ -44,7 +44,7 @@ other useful answer testing functionality are located in the source in
 `test/answer_tests/answer_testing.py`. All answer tests are located in the
 other files within the `test/answer_tests` directory.
 
-Some other functionality, that may be reused in other unrelated scripts provided in the Enzo-E repository, is provided in the ``test_utils`` subdirectory.
+Some other functionality, that may be reused in other unrelated scripts provided in the Enzo-E repository, are provided in the ``test_utils`` subdirectory.
 
 Running the Answer Test Suite
 -----------------------------
@@ -91,7 +91,7 @@ In cases where both are set, the command line argument is given precedence.
      - points to the build-directory where the target enzo-e binary was built (that binary has the path: BUILD_DIR/bin/enzo-e).
        The path to the charmrun launcher will be inferred from the `BUILD_DIR/CMakeCache.txt` file, but can be overwritten by the ``--charm`` flag or the ``CHARM_PATH`` environment variable.
        This precedence was chosen in case a user causes a change to relevant cached build-variables, but have not rebuilt Enzo-E (i.e. `CMakeCache.txt` may not be valid for the binary).
-       When this flag isn't specified, the test infrastructure searches for the enzo-e binary at ENZOE_ROOT/build/bin/enzo-e, but does'nt try to infer charmrun's location from `CMakeCache.txt`.
+       When this flag isn't specified, the test infrastructure searches for the enzo-e binary at ENZOE_ROOT/build/bin/enzo-e, but doesn't try to infer charmrun's location from `CMakeCache.txt`.
    * - ``--local-dir``
      - ``TEST_RESULTS_DIR``
      - points to a directory in which answers will be stored/loaded
@@ -121,7 +121,8 @@ First, check out the highest numbered gold standard tag and compile ``enzo-e``.
 
 .. code-block:: bash
 
-   $ git checkout gold-standard-1
+   # in the future, you will need to subsitute 004 for a higher number
+   $ git checkout gold-standard-004
    $ ...compile enzo-e
 
 Then, run the test suite by calling ``pytest`` with the answer test directory (make sure to configure behavior correctly with command-line arguments or environment variables).

doc/source/tutorial/getting_started.rst

@@ -284,7 +284,7 @@ These options control compilation choices that can be used to facillitate profil
 Testing Options
 ^^^^^^^^^^^^^^^
 
-These options configure properties of parallel automated tests.
+These options configure properties of automated tests. These options currently just affect tests in the :ref:`ctest framework <ctest>` and don't affect tests in the :ref:`pytest framework <pytest>`.
 
 .. list-table:: Testing-Related Configuration
    :widths: 10 30 5
@@ -302,6 +302,10 @@ These options configure properties of parallel automated tests.
    * - ``PARALLEL_LAUNCHER_NPROC``
      - Number of processors to run parallel unit tests
      - 4
+   * - ``BUILD_TESTING``
+     - Whether to setup the CTest infrastructure and build unit test binaries (which are primarily built to be executed by the CTest infrastructure). This has no effect on the pytest infrastructure.
+     - "ON"
+
 
 Debugging Options
 ^^^^^^^^^^^^^^^^^

src/Cello/CMakeLists.txt

@@ -338,9 +338,13 @@ function(addUnitTestBinary BINNAME SRCS LIB TESTER)
   #   TESTER:    Tester target library (usually, this is tester_default,
   #              tester_mesh, OR tester_simulation)
 
-  add_executable(${BINNAME} ${SRCS})
-  target_link_libraries(${BINNAME} PUBLIC ${LIB} ${TESTER})
-  target_link_options(${BINNAME} PRIVATE ${Cello_TARGET_LINK_OPTIONS})
+  if (BUILD_TESTING)
+    # only declare the binary for a unit-test when BUILD_TESTING is ON (the
+    # default case)
+    add_executable(${BINNAME} ${SRCS})
+    target_link_libraries(${BINNAME} PUBLIC ${LIB} ${TESTER})
+    target_link_options(${BINNAME} PRIVATE ${Cello_TARGET_LINK_OPTIONS})
+  endif()
 endfunction(addUnitTestBinary)
 
 # tests of the cello-component

src/Cello/view_ViewCollec.hpp

@@ -229,16 +229,23 @@ class ViewCollec{
   // used in destructor and to help switch the active member of the enum
   // need to explicitly call destructor because we use placement new
   void dealloc_active_member_() noexcept{
-    // note, we need to use the full qualified names of destructors to avoid
-    // errors with the nvidia compiler
+    // Note: we need to use the full qualified names of destructors to avoid
+    //       errors with the nvidia compiler
+    // Note: Subsequently, we ran into problems with how clang 10.0 requires us
+    //       to specify the destructor. I had to mutate this again to get it to
+    //       work with clang and g++. I'm not sure if I broke compatability
+    //       with nvcc (I suspect that it may still work).
+    // Going forward, the most obvious solution for getting this to work is to
+    // use C++17's std::variant
+
     switch (tag_){
       case Tag::CONTIG: {
-        single_arr_.detail::SingleAddressViewCollec_<T>::~SingleAddressViewCollec_();
-        break;
+        using ContigCollec = typename detail::SingleAddressViewCollec_<T>;
+        single_arr_.~ContigCollec(); break;
       }
       case Tag::ARR_OF_PTR: {
-        arr_of_ptr_.detail::ArrOfPtrsViewCollec_<T>::~ArrOfPtrsViewCollec_();
-        break;
+        using ArrOfPtr = typename detail::ArrOfPtrsViewCollec_<T>;
+        arr_of_ptr_.~ArrOfPtr(); break;
       }
     }
   }

src/Enzo/CMakeLists.txt

@@ -89,9 +89,14 @@ add_subdirectory(io)
 add_subdirectory(mesh)
 add_subdirectory(obsolete)
 add_subdirectory(particle)
-add_subdirectory(tests)
 add_subdirectory(utils)
 
+# note that the tests subdirectory introduces problem initializers that can
+# be used to setup test-problems. The BUILD_TESTING option has no impact on
+# what is built from this directory (it only affects whether binaries,
+# expressly used for unit-testing are built)
+add_subdirectory(tests)
+
 # Step 4: specify additional information about the Enzo library
 # (dependencies are only PUBLIC because of the way that all headers are
 #  transitively included in all targets. If they weren't public, the include
@@ -116,12 +121,14 @@ add_executable(enzo-e enzo-e.cpp)
 target_link_libraries(enzo-e PRIVATE enzo main_enzo ${External_LIBS})
 target_link_options(enzo-e PRIVATE ${Cello_TARGET_LINK_OPTIONS})
 
-# define a unit test:
-#
-# consider removing the enzo-specific stuff from this test so that we can
-# define it entirely in the Cello layer
-add_executable(
-  test_class_size "${CMAKE_CURRENT_SOURCE_DIR}/../Cello/test_class_size.cpp"
-)
-target_link_libraries(test_class_size PRIVATE enzo mesh tester_mesh)
-target_link_options(test_class_size PRIVATE ${Cello_TARGET_LINK_OPTIONS})
+if (BUILD_TESTING)
+  # define a unit test:
+  #
+  # consider removing the enzo-specific stuff from this test so that we can
+  # define it entirely in the Cello layer
+  add_executable(
+    test_class_size "${CMAKE_CURRENT_SOURCE_DIR}/../Cello/test_class_size.cpp"
+    )
+  target_link_libraries(test_class_size PRIVATE enzo mesh tester_mesh)
+  target_link_options(test_class_size PRIVATE ${Cello_TARGET_LINK_OPTIONS})
+endif()

src/Enzo/enzo-core/CMakeLists.txt

@@ -26,7 +26,9 @@ list(FILTER LOCAL_SRC_FILES EXCLUDE REGEX "test_EnzoUnits")
 
 target_sources(enzo PRIVATE ${LOCAL_SRC_FILES})
 
-# Add a unit test
-add_executable(test_enzo_units "test_EnzoUnits.cpp")
-target_link_libraries(test_enzo_units PRIVATE enzo main_enzo)
-target_link_options(test_enzo_units PRIVATE ${Cello_TARGET_LINK_OPTIONS})
+if (BUILD_TESTING)
+  # Add a unit test
+  add_executable(test_enzo_units "test_EnzoUnits.cpp")
+  target_link_libraries(test_enzo_units PRIVATE enzo main_enzo)
+  target_link_options(test_enzo_units PRIVATE ${Cello_TARGET_LINK_OPTIONS})
+endif()

src/Enzo/hydro-mhd/SolveHydroEquations.cpp

@@ -144,7 +144,20 @@ int EnzoBlock::SolveHydroEquations
 
   if (density_floor > 0.0) {
     for (int i=0; i<mx*my*mz; i++) {
-      density[i] = std::max(density[i], density_floor);
+
+      if (density[i] < density_floor) {
+          double d_pre = density[i];
+          density[i] = density_floor;
+          double density_ratio = density[i] / d_pre;
+
+          // rescale color fields to account for change in
+          // baryon mass from enforcing density floor
+          for (int ic = 0; ic < ncolor; ic++){
+            enzo_float * cfield = (enzo_float *)
+            field.values(field.groups()->item("color",ic));
+            cfield[i] *= density_ratio;
+          }
+      }
     }
   }