Merge branch 'feature/error-handling-just-the-core-formatted' into fe…

…ature/error-handling-ida
LLNL · Dec 7, 2023 · 8f7ce21 · 8f7ce21
2 parents 04b178d + ca55fc5
commit 8f7ce21
Show file tree

Hide file tree

Showing 17 changed files with 136 additions and 79 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -58,7 +58,8 @@ Changed the `SUNProfiler` so that it does not rely on `MPI_WTime` in any case.
 This fixes https://github.com/LLNL/sundials/issues/312. 
 
 **Major feature**
-SUNDIALS now has more robust and uniform error handling. See the "Error Handling"
+SUNDIALS now has more robust and uniform error handling. Non-release builds will
+be built with additional error checking by default. See the "Error Handling"
 section in the user guide for details.
 
 **Deprecation notice**
@@ -123,9 +124,13 @@ and a typedef to a `MPI_Comm` in builds with MPI. Here is what this means:
 The change away from type-erased pointers for `SUNComm` fixes problems like the 
 one described in [GitHub Issue #275](https://github.com/LLNL/sundials/issues/275).
 
+The SUNLogger is now always MPI-aware if MPI is enabled in SUNDIALS and the
+`SUNDIALS_LOGGING_ENABLE_MPI` CMake option and macro definition were removed 
+accordingly.
+
 **Breaking change**
 Functions, types and header files that were previously deprecated have been
-removed.  Additionally, the `SUNDIALS_LOGGING_ENABLE_MPI` macro was removed.
+removed.
 
 **Breaking change**
 Users now need to link to `sundials_core` in addition to the libraries already linked to. 

diff --git a/cmake/SundialsBuildOptionsPre.cmake b/cmake/SundialsBuildOptionsPre.cmake
@@ -317,10 +317,11 @@ sundials_option(SUNDIALS_TEST_DEVTESTS BOOL
 sundials_option(SUNDIALS_TEST_UNITTESTS BOOL
   "Include unit tests in make test" OFF ADVANCED)
 
-# Disable unit tests that use GTest
-sundials_option(SUNDIALS_TEST_DISABLE_GTEST BOOL
-  "Disable GTest unit tests" OFF ADVANCED)
+# Include unit tests based on Google test in regression tests
+sundials_option(SUNDIALS_TEST_ENABLE_GTEST BOOL
+  "Disable GTest unit tests" ON ADVANCED)
 
+# Enable clang-tidy to run as part of the CMake build
 sundials_option(SUNDIALS_DEV_CLANG_TIDY BOOL
   "Enable the clang-tidy linter" OFF ADVANCED)
 

diff --git a/cmake/SundialsSetupTesting.cmake b/cmake/SundialsSetupTesting.cmake
@@ -135,7 +135,7 @@ if(SUNDIALS_TEST_DEVTESTS)
 endif()
 
 # Check if unit tests are enabled
-if(SUNDIALS_TEST_UNITTESTS AND NOT SUNDIALS_TEST_DISABLE_GTEST)
+if(SUNDIALS_TEST_UNITTESTS AND SUNDIALS_TEST_ENABLE_GTEST)
   find_package(GTest)
   if(NOT (TARGET GTest::gtest_main OR TARGET GTest::Main))
     include(FetchContent)

diff --git a/doc/arkode/guide/source/Introduction.rst b/doc/arkode/guide/source/Introduction.rst
@@ -186,7 +186,9 @@ This fixes `GitHub Issue #312 <https://github.com/LLNL/sundials/issues/312>`_.
 Added Fortran support for the LAPACK  dense ``SUNLinearSolver`` implementation.
 
 **Major feature**
-SUNDIALS now has more robust and uniform error handling. See :numref:`SUNDIALS.Errors` for details.
+SUNDIALS now has more robust and uniform error handling. Non-release builds will
+be built with additional error checking by default. See :numref:`SUNDIALS.Errors`
+for details.
 
 **Deprecation notice**
 The functions in `sundials_math.h` will be deprecated in the next release.
@@ -247,7 +249,7 @@ and a typedef to a ``MPI_Comm`` in builds with MPI. Here is what this means:
   ``N_VGetCommunicator``, since it now returns a ``SUNComm``. 
 
 The change away from type-erased pointers for :c:type:`SUNComm` fixes problems like the 
-one described in `GitHub Issue #275 <https://github.com/LLNL/sundials/issues/275>_`.
+one described in `GitHub Issue #275 <https://github.com/LLNL/sundials/issues/275>`_.
 
 **Breaking change**
 The SUNLogger is now always MPI-aware if MPI is enabled in SUNDIALS and the

diff --git a/doc/cvode/guide/source/Introduction.rst b/doc/cvode/guide/source/Introduction.rst
@@ -138,7 +138,9 @@ This fixes `GitHub Issue #312 <https://github.com/LLNL/sundials/issues/312>`_.
 Added Fortran support for the LAPACK  dense ``SUNLinearSolver`` implementation.
 
 **Major feature**
-SUNDIALS now has more robust and uniform error handling. See :numref:`SUNDIALS.Errors` for details.
+SUNDIALS now has more robust and uniform error handling. Non-release builds will
+be built with additional error checking by default. See :numref:`SUNDIALS.Errors`
+for details.
 
 **Deprecation notice**
 The functions in `sundials_math.h` will be deprecated in the next release.
@@ -199,7 +201,7 @@ and a typedef to a ``MPI_Comm`` in builds with MPI. Here is what this means:
   ``N_VGetCommunicator``, since it now returns a ``SUNComm``. 
 
 The change away from type-erased pointers for :c:type:`SUNComm` fixes problems like the 
-one described in `GitHub Issue #275 <https://github.com/LLNL/sundials/issues/275>_`.
+one described in `GitHub Issue #275 <https://github.com/LLNL/sundials/issues/275>`_.
 
 **Breaking change**
 The SUNLogger is now always MPI-aware if MPI is enabled in SUNDIALS and the

diff --git a/doc/cvodes/guide/source/Introduction.rst b/doc/cvodes/guide/source/Introduction.rst
@@ -141,7 +141,9 @@ This fixes `GitHub Issue #312 <https://github.com/LLNL/sundials/issues/312>`_.
 Added Fortran support for the LAPACK  dense ``SUNLinearSolver`` implementation.
 
 **Major feature**
-SUNDIALS now has more robust and uniform error handling. See :numref:`SUNDIALS.Errors` for details.
+SUNDIALS now has more robust and uniform error handling. Non-release builds will
+be built with additional error checking by default. See :numref:`SUNDIALS.Errors`
+for details.
 
 **Deprecation notice**
 The functions in `sundials_math.h` will be deprecated in the next release.
@@ -202,7 +204,7 @@ and a typedef to a ``MPI_Comm`` in builds with MPI. Here is what this means:
   ``N_VGetCommunicator``, since it now returns a ``SUNComm``. 
 
 The change away from type-erased pointers for :c:type:`SUNComm` fixes problems like the 
-one described in `GitHub Issue #275 <https://github.com/LLNL/sundials/issues/275>_`.
+one described in `GitHub Issue #275 <https://github.com/LLNL/sundials/issues/275>`_.
 
 **Breaking change**
 The SUNLogger is now always MPI-aware if MPI is enabled in SUNDIALS and the

diff --git a/doc/ida/guide/source/Introduction.rst b/doc/ida/guide/source/Introduction.rst
@@ -99,7 +99,9 @@ This fixes `GitHub Issue #312 <https://github.com/LLNL/sundials/issues/312>`_.
 Added Fortran support for the LAPACK  dense ``SUNLinearSolver`` implementation.
 
 **Major feature**
-SUNDIALS now has more robust and uniform error handling. See :numref:`SUNDIALS.Errors` for details.
+SUNDIALS now has more robust and uniform error handling. Non-release builds will
+be built with additional error checking by default. See :numref:`SUNDIALS.Errors`
+for details.
 
 **Deprecation notice**
 The functions in `sundials_math.h` will be deprecated in the next release.
@@ -160,7 +162,7 @@ and a typedef to a ``MPI_Comm`` in builds with MPI. Here is what this means:
   ``N_VGetCommunicator``, since it now returns a ``SUNComm``. 
 
 The change away from type-erased pointers for :c:type:`SUNComm` fixes problems like the 
-one described in `GitHub Issue #275 <https://github.com/LLNL/sundials/issues/275>_`.
+one described in `GitHub Issue #275 <https://github.com/LLNL/sundials/issues/275>`_.
 
 **Breaking change**
 The SUNLogger is now always MPI-aware if MPI is enabled in SUNDIALS and the

diff --git a/doc/idas/guide/source/Introduction.rst b/doc/idas/guide/source/Introduction.rst
@@ -116,7 +116,9 @@ This fixes `GitHub Issue #312 <https://github.com/LLNL/sundials/issues/312>`_.
 Added Fortran support for the LAPACK  dense ``SUNLinearSolver`` implementation.
 
 **Major feature**
-SUNDIALS now has more robust and uniform error handling. See :numref:`SUNDIALS.Errors` for details.
+SUNDIALS now has more robust and uniform error handling. Non-release builds will
+be built with additional error checking by default. See :numref:`SUNDIALS.Errors`
+for details.
 
 **Deprecation notice**
 The functions in `sundials_math.h` will be deprecated in the next release.
@@ -177,7 +179,7 @@ and a typedef to a ``MPI_Comm`` in builds with MPI. Here is what this means:
   ``N_VGetCommunicator``, since it now returns a ``SUNComm``. 
 
 The change away from type-erased pointers for :c:type:`SUNComm` fixes problems like the 
-one described in `GitHub Issue #275 <https://github.com/LLNL/sundials/issues/275>_`.
+one described in `GitHub Issue #275 <https://github.com/LLNL/sundials/issues/275>`_.
 
 **Breaking change**
 The SUNLogger is now always MPI-aware if MPI is enabled in SUNDIALS and the

diff --git a/doc/kinsol/guide/source/Introduction.rst b/doc/kinsol/guide/source/Introduction.rst
@@ -108,7 +108,9 @@ CMake targets.
 Added Fortran support for the LAPACK  dense ``SUNLinearSolver`` implementation.
 
 **Major feature**
-SUNDIALS now has more robust and uniform error handling. See :numref:`SUNDIALS.Errors` for details.
+SUNDIALS now has more robust and uniform error handling. Non-release builds will
+be built with additional error checking by default. See :numref:`SUNDIALS.Errors`
+for details.
 
 **Deprecation notice**
 The functions in `sundials_math.h` will be deprecated in the next release.
@@ -169,7 +171,7 @@ and a typedef to a ``MPI_Comm`` in builds with MPI. Here is what this means:
   ``N_VGetCommunicator``, since it now returns a ``SUNComm``. 
 
 The change away from type-erased pointers for :c:type:`SUNComm` fixes problems like the 
-one described in `GitHub Issue #275 <https://github.com/LLNL/sundials/issues/275>_`.
+one described in `GitHub Issue #275 <https://github.com/LLNL/sundials/issues/275>`_.
 
 **Breaking change**
 The SUNLogger is now always MPI-aware if MPI is enabled in SUNDIALS and the

diff --git a/doc/shared/sundials/Fortran.rst b/doc/shared/sundials/Fortran.rst
@@ -549,7 +549,7 @@ a C file pointer, SUNDIALS provides two utility functions for creating a
 
    .. versionchanged:: 7.0.0
    
-      The function signature was updated to return a `SUNErrCode`.
+      The function signature was updated to return a `SUNErrCode` and the `fp` parameter was changed from `FILE*` to `FILE**`.  
  
 
 

diff --git a/doc/shared/sundials/Install.rst b/doc/shared/sundials/Install.rst
@@ -527,15 +527,6 @@ illustration only.
 
    Default: ``sm_30``
 
-.. cmakeoption:: ENABLE_XBRAID
-
-   Enable or disable the ARKStep + XBraid interface.
-
-   Default: ``OFF``
-
-   .. note:: See additional information on building with *XBraid*
-             enabled in  :numref:`Installation.CMake.ExternalLibraries`.
-
 .. cmakeoption:: EXAMPLES_ENABLE_C
 
    Build the SUNDIALS C examples
@@ -599,6 +590,55 @@ illustration only.
 
    Default: ``OFF``
 
+.. cmakeoption:: SUNDIALS_LOGGING_LEVEL
+
+   Set the maximum logging level for the SUNLogger runtime API. The higher this is set,
+   the more output that may be logged, and the more performance may degrade. The options are:
+
+   - ``0`` -- no logging
+   - ``1`` -- log errors
+   - ``2`` -- log errors + warnings
+   - ``3`` -- log errors + warnings + informational output
+   - ``4`` -- log errors + warnings + informational output + debug output
+   - ``5`` -- log all of the above and even more (e.g. vector valued variables may be logged)
+
+   Default: 0
+
+
+.. cmakeoption:: SUNDIALS_BUILD_WITH_MONITORING
+
+   Build SUNDIALS with capabilties for fine-grained monitoring of solver progress
+   and statistics. This is primarily useful for debugging.
+
+   Default: OFF
+
+   .. warning::
+
+      Building with monitoring may result in minor performance degradation even
+      if monitoring is not utilized.
+
+.. cmakeoption:: SUNDIALS_BUILD_WITH_PROFILING
+
+   Build SUNDIALS with capabilties for fine-grained profiling.
+   This requires POSIX timers or the Windows ``profileapi.h`` timers.
+
+   Default: OFF
+
+   .. warning::
+
+      Profiling will impact performance, and should be enabled judiciously.
+
+.. cmakeoption:: SUNDIALS_ENABLE_ERROR_CHECKS
+
+   Build SUNDIALS with more extensive checks for unrecoverable errors. 
+
+   Default: ``OFF`` when ``CMAKE_BUILD_TYPE=Release|RelWithDebInfo `` and ``ON`` otherwise.
+
+   .. warning::
+
+      Error checks will impact performance, but can be helpful for debugging.
+
+
 .. cmakeoption:: ENABLE_GINKGO
 
    Enable interfaces to the Ginkgo linear algebra library.
@@ -986,45 +1026,6 @@ illustration only.
 
    Default: OFF
 
-
-.. cmakeoption:: SUNDIALS_LOGGING_LEVEL
-
-   Set the maximum logging level for the SUNLogger runtime API. The higher this is set,
-   the more output that may be logged, and the more performance may degrade. The options are:
-
-   - ``0`` -- no logging
-   - ``1`` -- log errors
-   - ``2`` -- log errors + warnings
-   - ``3`` -- log errors + warnings + informational output
-   - ``4`` -- log errors + warnings + informational output + debug output
-   - ``5`` -- log all of the above and even more (e.g. vector valued variables may be logged)
-
-   Default: 0
-
-
-.. cmakeoption:: SUNDIALS_BUILD_WITH_MONITORING
-
-   Build SUNDIALS with capabilties for fine-grained monitoring of solver progress
-   and statistics. This is primarily useful for debugging.
-
-   Default: OFF
-
-   .. warning::
-
-      Building with monitoring may result in minor performance degradation even
-      if monitoring is not utilized.
-
-.. cmakeoption:: SUNDIALS_BUILD_WITH_PROFILING
-
-   Build SUNDIALS with capabilties for fine-grained profiling.
-   This requires POSIX timers or the Windows ``profileapi.h`` timers.
-
-   Default: OFF
-
-   .. warning::
-
-      Profiling will impact performance, and should be enabled judiciously.
-
 .. cmakeoption:: ENABLE_CALIPER
 
    Enable CALIPER support
@@ -1115,19 +1116,19 @@ illustration only.
       The advanced option, :cmakeop:`SUNDIALS_INDEX_TYPE` can be used to provide
       a type not listed here.
 
-.. cmakeoption:: SUNDIALS_MATH_LIBRARY
-
-   The standard C math library (e.g., ``libm``) to link with.
-
-   Default: ``-lm`` on Unix systems, none otherwise
-
 .. cmakeoption:: SUNDIALS_PRECISION
 
    The floating-point precision used in SUNDIALS packages and class
    implementations, options are: ``double``, ``single``, or ``extended``
 
    Default: ``double``
 
+.. cmakeoption:: SUNDIALS_MATH_LIBRARY
+
+   The standard C math library (e.g., ``libm``) to link with.
+
+   Default: ``-lm`` on Unix systems, none otherwise
+
 .. cmakeoption:: SUNDIALS_INSTALL_CMAKEDIR
 
    Installation directory for the SUNDIALS cmake files (relative to
@@ -1145,6 +1146,15 @@ illustration only.
 
       This option is deprecated. Use :cmakeop:`SUNDIALS_MATH_LIBRARY`.
 
+.. cmakeoption:: ENABLE_XBRAID
+
+   Enable or disable the ARKStep + XBraid interface.
+
+   Default: ``OFF``
+
+   .. note:: See additional information on building with *XBraid*
+             enabled in  :numref:`Installation.CMake.ExternalLibraries`.
+
 .. cmakeoption:: XBRAID_DIR
 
    The root directory of the XBraid installation.

diff --git a/doc/superbuild/source/developers/development/Checklist.rst b/doc/superbuild/source/developers/development/Checklist.rst
@@ -84,4 +84,4 @@ system, etc. developers should adhere to the following checklist.
 
 #. If answer files changed, and it is expected/desired, then update the `.out` files
    that are embedded in the `examples/` directory AND the
-   `"answers" repository <https://github.com/sundials-codes/answers>_`. 
+   `"answers" repository <https://github.com/sundials-codes/answers>`_. 
diff --git a/src/sundials/sundials_context.c b/src/sundials/sundials_context.c
@@ -93,7 +93,7 @@ SUNErrCode SUNContext_Create(SUNComm comm, SUNContext* sunctx_out)
     sunctx->own_logger   = logger != NULL;
     sunctx->profiler     = profiler;
     sunctx->own_profiler = profiler != NULL;
-    sunctx->last_err     = 0;
+    sunctx->last_err     = SUN_SUCCESS;
     sunctx->err_handler  = eh;
     sunctx->comm         = comm;
   }

diff --git a/src/sundials/sundials_mpi_errors.c b/src/sundials/sundials_mpi_errors.c
@@ -36,6 +36,5 @@ void SUNMPIAbortErrHandlerFn(int line, const char* func, const char* file,
                      "different "
                      "error handler to avoid program termination.\n");
   free(file_and_line);
-  sleep(1);
   MPI_Abort(sunctx->comm, err_code);
 }
diff --git a/test/config_cmake.py b/test/config_cmake.py
@@ -565,8 +565,8 @@ def main():
             'SUNDIALS_TEST_UNITTESTS', 'OFF', 'BOOL',
             'SUNDIALS unit tests')
 
-    add_arg(group, '--no-gtest', 'SUNDIALS_TEST_DISABLE_GTEST',
-            'SUNDIALS_TEST_DISABLE_GTEST', 'OFF', 'BOOL',
+    add_arg(group, '--no-gtest', 'SUNDIALS_TEST_ENABLE_GTEST',
+            'SUNDIALS_TEST_ENABLE_GTEST', 'ON', 'BOOL',
             'SUNDIALS GTest unit tests')
 
     # test output directory

diff --git a/test/env/setup_env.sh b/test/env/setup_env.sh
@@ -107,7 +107,7 @@ case "$SUNDIALS_TEST_TYPE" in
 esac
 
 # Disable GTest in CI until we determine SEGFAULT cause in test_sundials_errors.c
-export SUNDIALS_TEST_DISABLE_GTEST=ON
+export SUNDIALS_TEST_ENABLE_GTEST=OFF
 
 # Build Type
 export CMAKE_BUILD_TYPE="Debug"