Feature: Add SUNComm type (#370)

Addresses #275. Requires MPI to be linked to SUNDIALS core if it is enabled thus all-in-one MPI and non-MPI builds are no longer supported. Instead user will have to separately build and install with and without MPI. --------- Co-authored-by: Daniel R. Reynolds <[email protected]> Co-authored-by: Steven Roberts <[email protected]> Co-authored-by: David J. Gardner <[email protected]>
LLNL · Dec 18, 2023 · 947ad4b · 947ad4b
1 parent cc69603
commit 947ad4b
Show file tree

Hide file tree

Showing 621 changed files with 2,027 additions and 1,666 deletions.
diff --git a/.gitlab/spack_packages/sundials/package.py b/.gitlab/spack_packages/sundials/package.py
@@ -751,7 +751,6 @@ def initconfig_package_entries(self):
                 self.cache_option_from_variant("USE_GENERIC_MATH", "generic-math"),
                 # Logging
                 self.cache_string_from_variant("SUNDIALS_LOGGING_LEVEL", "logging-level"),
-                self.cache_option_from_variant("SUNDIALS_LOGGING_ENABLE_MPI", "logging-mpi"),
                 # Monitoring
                 self.cache_option_from_variant("SUNDIALS_BUILD_WITH_MONITORING", "monitoring"),
                 # Profiling

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,9 +2,43 @@
 
 ## Changes to SUNDIALS in release X.X.X
 
+**Breaking change** 
+We have replaced the use of a type-erased (i.e., `void*`) pointer to a
+communicator in place of `MPI_Comm` throughout the SUNDIALS API with a
+`SUNComm`, which is just a typedef to an `int` in builds without MPI
+and a typedef to a `MPI_Comm` in builds with MPI. Here is what this means:
+
+- All users will need to update their codes because the call to 
+  `SUNContext_Create` now takes a `SUNComm` instead
+  of type-erased pointer to a communicator. For non-MPI codes,
+  pass `SUN_COMM_NULL` to the `comm` argument instead of
+  `NULL`. For MPI codes, pass the `MPI_Comm` directly. 
+  The required change should be doable with a find-and-replace. 
+
+- The same change must be made for calls to 
+  `SUNLogger_Create` or `SUNProfiler_Create`. 
+
+- Some users will need to update their calls to `N_VGetCommunicator`, and 
+  update any custom `N_Vector` implementations tht provide 
+  `N_VGetCommunicator`, since it now returns a `SUNComm`. 
+
+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).
+
+**Breaking change**
+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. 
+removed.  
+
+**Breaking change**
+Users now need to link to `sundials_core` in addition to the libraries already linked to. 
+This will be picked up automatically in projects that use the SUNDIALS CMake target.
+The library `sundials_generic` has been superceded by `sundials_core` and is no longer available.
+This fixes some duplicate symbol errors on Windows when linking to multiple SUNDIALS libraries.
 
 The previously deprecated types `realtype` and `booleantype` were removed from `sundials_types.h`
 and replaced with `sunrealtype` and `sunbooleantype`. The deprecated names for these types

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -19,7 +19,7 @@
 # Initial setup.
 # ===============================================================
 
-cmake_minimum_required(VERSION 3.12)
+cmake_minimum_required(VERSION 3.18)
 
 # Project SUNDIALS (initially only C supported)
 # sets PROJECT_SOURCE_DIR and PROJECT_BINARY_DIR variables.

diff --git a/benchmarks/advection_reaction_3D/kokkos/advection_reaction_3D.cpp b/benchmarks/advection_reaction_3D/kokkos/advection_reaction_3D.cpp
@@ -73,7 +73,7 @@ int main(int argc, char *argv[])
   MPI_Init(&argc, &argv);
 
   /* Create SUNDIALS context */
-  SUNContext_Create((void*) &comm, &ctx);
+  SUNContext_Create(comm, &ctx);
 
   /* Initialize Kokkos */
   Kokkos::initialize(argc, argv);

diff --git a/benchmarks/advection_reaction_3D/raja/advection_reaction_3D.cpp b/benchmarks/advection_reaction_3D/raja/advection_reaction_3D.cpp
@@ -75,7 +75,7 @@ int main(int argc, char *argv[])
   MPI_Init(&argc, &argv);
 
   /* Create SUNDIALS context */
-  SUNContext_Create((void*) &comm, &ctx);
+  SUNContext_Create(comm, &ctx);
 
   /* Create SUNDIALS memory helper */
 #if defined(USE_CUDA)

diff --git a/benchmarks/diffusion_2D/main_arkode.cpp b/benchmarks/diffusion_2D/main_arkode.cpp
@@ -62,7 +62,7 @@ int main(int argc, char* argv[])
   SUNContext  ctx  = NULL;
   SUNProfiler prof = NULL;
 
-  flag = SUNContext_Create((void*) &comm, &ctx);
+  flag = SUNContext_Create(comm, &ctx);
   if (check_flag(&flag, "SUNContextCreate", 1)) return 1;
 
   flag = SUNContext_GetProfiler(ctx, &prof);

diff --git a/benchmarks/diffusion_2D/main_cvode.cpp b/benchmarks/diffusion_2D/main_cvode.cpp
@@ -58,7 +58,7 @@ int main(int argc, char* argv[])
   SUNContext  ctx  = NULL;
   SUNProfiler prof = NULL;
 
-  flag = SUNContext_Create((void*) &comm, &ctx);
+  flag = SUNContext_Create(comm, &ctx);
   if (check_flag(&flag, "SUNContextCreate", 1)) return 1;
 
   flag = SUNContext_GetProfiler(ctx, &prof);

diff --git a/benchmarks/diffusion_2D/main_ida.cpp b/benchmarks/diffusion_2D/main_ida.cpp
@@ -56,7 +56,7 @@ int main(int argc, char* argv[])
   SUNContext  ctx  = NULL;
   SUNProfiler prof = NULL;
 
-  flag = SUNContext_Create((void*) &comm, &ctx);
+  flag = SUNContext_Create(comm, &ctx);
   if (check_flag(&flag, "SUNContextCreate", 1)) return 1;
 
   flag = SUNContext_GetProfiler(ctx, &prof);

diff --git a/benchmarks/nvector/cuda/test_nvector_performance_cuda.cu b/benchmarks/nvector/cuda/test_nvector_performance_cuda.cu
@@ -132,7 +132,7 @@ int main(int argc, char *argv[])
   printf("  number of tests       %d  \n", ntests);
   printf("  timing on/off         %d  \n", print_timing);
 
-  flag = SUNContext_Create(NULL, &ctx);
+  flag = SUNContext_Create(SUN_COMM_NULL, &ctx);
   if (flag) return flag;
 
   /* Create vectors */

diff --git a/benchmarks/nvector/hip/test_nvector_performance_hip.cpp b/benchmarks/nvector/hip/test_nvector_performance_hip.cpp
@@ -132,7 +132,7 @@ int main(int argc, char *argv[])
   printf("  number of tests       %d  \n", ntests);
   printf("  timing on/off         %d  \n", print_timing);
 
-  flag = SUNContext_Create(NULL, &ctx);
+  flag = SUNContext_Create(SUN_COMM_NULL, &ctx);
   if (flag) return flag;
 
   /* Create vectors */

diff --git a/benchmarks/nvector/kokkos/CMakeLists.txt b/benchmarks/nvector/kokkos/CMakeLists.txt
@@ -16,7 +16,7 @@ foreach(backend ${KOKKOS_EXAMPLES_BACKENDS})
 
   sundials_add_nvector_benchmark(test_nvector_performance_kokkos.${backend}
     SOURCES test_nvector_performance_kokkos.cpp
-    SUNDIALS_TARGETS sundials_generic sundials_nveckokkos
+    SUNDIALS_TARGETS sundials_core sundials_nveckokkos
     INSTALL_SUBDIR nvector/kokkos
     )
 

diff --git a/benchmarks/nvector/mpiplusx/test_nvector_performance_mpiplusx.c b/benchmarks/nvector/mpiplusx/test_nvector_performance_mpiplusx.c
@@ -116,7 +116,7 @@ int main(int argc, char *argv[])
     printf("  number of MPI procs   %d  \n", nprocs);
   }
 
-  flag = SUNContext_Create(&comm, &ctx);
+  flag = SUNContext_Create(comm, &ctx);
   if (flag) return flag;
 
   /* Create vectors */

diff --git a/benchmarks/nvector/openmp/test_nvector_performance_openmp.c b/benchmarks/nvector/openmp/test_nvector_performance_openmp.c
@@ -108,7 +108,7 @@ int main(int argc, char *argv[])
   printf("  timing on/off         %d  \n", print_timing);
   printf("  number of threads     %d  \n", nthreads);
 
-  flag = SUNContext_Create(NULL, &ctx);
+  flag = SUNContext_Create(SUN_COMM_NULL, &ctx);
   if (flag) return flag;
 
   /* Create vectors */

diff --git a/benchmarks/nvector/parallel/test_nvector_performance_parallel.c b/benchmarks/nvector/parallel/test_nvector_performance_parallel.c
@@ -114,7 +114,7 @@ int main(int argc, char *argv[])
     printf("  number of MPI procs   %d  \n", nprocs);
   }
 
-  flag = SUNContext_Create(&comm, &ctx);
+  flag = SUNContext_Create(comm, &ctx);
   if (flag) return flag;
 
   /* Create vectors */

diff --git a/benchmarks/nvector/parhyp/test_nvector_performance_parhyp.c b/benchmarks/nvector/parhyp/test_nvector_performance_parhyp.c
@@ -115,7 +115,7 @@ int main(int argc, char *argv[])
     printf("  number of MPI procs   %d  \n", nprocs);
   }
 
-  flag = SUNContext_Create(&comm, &ctx);
+  flag = SUNContext_Create(comm, &ctx);
   if (flag) return flag;
 
   /* set partitioning */

diff --git a/benchmarks/nvector/petsc/test_nvector_performance_petsc.c b/benchmarks/nvector/petsc/test_nvector_performance_petsc.c
@@ -65,7 +65,7 @@ int main(int argc, char *argv[])
   ierr = PetscInitializeNoArguments();
   CHKERRQ(ierr);
 
-  if (SUNContext_Create(&comm, &sunctx)) {
+  if (SUNContext_Create(comm, &sunctx)) {
     printf("ERROR: SUNContext_Create returned nonzero\n");
     return(-1);
   }

diff --git a/benchmarks/nvector/pthreads/test_nvector_performance_pthreads.c b/benchmarks/nvector/pthreads/test_nvector_performance_pthreads.c
@@ -106,7 +106,7 @@ int main(int argc, char *argv[])
   printf("  timing on/off         %d  \n", print_timing);
   printf("  number of threads     %d  \n", nthreads);
 
-  flag = SUNContext_Create(NULL, &ctx);
+  flag = SUNContext_Create(SUN_COMM_NULL, &ctx);
   if (flag) return flag;
 
   /* Create vectors */

diff --git a/benchmarks/nvector/serial/test_nvector_performance_serial.c b/benchmarks/nvector/serial/test_nvector_performance_serial.c
@@ -99,7 +99,7 @@ int main(int argc, char *argv[])
   printf("  number of tests       %d  \n", ntests);
   printf("  timing on/off         %d  \n", print_timing);
 
-  flag = SUNContext_Create(NULL, &ctx);
+  flag = SUNContext_Create(SUN_COMM_NULL, &ctx);
   if (flag) return flag;
 
   /* Create vectors */

diff --git a/cmake/SUNDIALSConfig.cmake.in b/cmake/SUNDIALSConfig.cmake.in
@@ -55,6 +55,11 @@ endforeach()
 
 ### ------- Create TPL imported targets
 
+if("@ENABLE_MPI@" AND NOT TARGET MPI::MPI_C)
+  set(MPI_C_COMPILER "@MPI_C_COMPILER@")
+  find_dependency(MPI)
+endif()
+
 if("@ENABLE_OPENMP@" AND NOT TARGET OpenMP::OpenMP_C)
   find_dependency(OpenMP)
 endif()

diff --git a/cmake/SundialsBuildOptionsPre.cmake b/cmake/SundialsBuildOptionsPre.cmake
@@ -84,10 +84,6 @@ if(SUNDIALS_LOGGING_LEVEL GREATER_EQUAL 3)
   message(WARNING "SUNDIALS built with additional logging turned on, performance may be affected.")
 endif()
 
-set(DOCSTR "Build SUNDIALS logging with MPI support")
-sundials_option(SUNDIALS_LOGGING_ENABLE_MPI BOOL "${DOCSTR}" "OFF"
-                DEPENDS_ON ENABLE_MPI)
-
 # ---------------------------------------------------------------
 # Option to use the generic math libraries
 # ---------------------------------------------------------------

diff --git a/cmake/SundialsSetupCompilers.cmake b/cmake/SundialsSetupCompilers.cmake
@@ -414,7 +414,7 @@ endforeach()
 # ===============================================================
 
 foreach(lang ${_SUNDIALS_ENABLED_LANGS})
-  if((SUNDIALS_BUILD_WITH_PROFILING OR SUNDIALS_LOGGING_ENABLE_MPI) AND ENABLE_MPI)
+  if(ENABLE_MPI)
     if(DEFINED MPI_${lang}_COMPILER)
       set(_EXAMPLES_${lang}_COMPILER "${MPI_${lang}_COMPILER}" CACHE INTERNAL "${lang} compiler for installed examples")
     endif()

diff --git a/cmake/SundialsSetupCuda.cmake b/cmake/SundialsSetupCuda.cmake
@@ -99,7 +99,7 @@ message(STATUS "CUDA Separable Compilation: ${CMAKE_CUDA_SEPARABLE_COMPILATION}"
 # Configure compiler for installed examples
 # ===============================================================
 
-if((SUNDIALS_BUILD_WITH_PROFILING OR SUNDIALS_LOGGING_ENABLE_MPI) AND ENABLE_MPI)
+if(ENABLE_MPI)
   set(_EXAMPLES_CUDA_HOST_COMPILER "${MPI_CXX_COMPILER}" CACHE INTERNAL "${lang} compiler for installed examples")
 else()
   set(_EXAMPLES_CUDA_HOST_COMPILER "${CMAKE_CUDA_HOST_COMPILER}" CACHE INTERNAL "${lang} compiler for installed examples")

diff --git a/cmake/macros/SundialsAddLibrary.cmake b/cmake/macros/SundialsAddLibrary.cmake
@@ -201,7 +201,7 @@ macro(sundials_add_library target)
     if(${_libtype} MATCHES "STATIC")
       target_compile_definitions(${obj_target} PRIVATE SUNDIALS_STATIC_DEFINE)
     else()
-      target_compile_definitions(${obj_target} PRIVATE sundials_generic_EXPORTS)
+      target_compile_definitions(${obj_target} PRIVATE sundials_core_EXPORTS)
     endif()
 
     # add all other compile definitions to object library
@@ -268,16 +268,6 @@ macro(sundials_add_library target)
         target_link_libraries(${_actual_target_name} ${sundials_add_library_LINK_LIBRARIES})
       endif()
 
-      if(SUNDIALS_BUILD_WITH_PROFILING OR SUNDIALS_LOGGING_ENABLE_MPI)
-        if(ENABLE_MPI AND MPI_C_FOUND)
-          # Workaround issues with sundials_generic object library dependency on
-          # MPI not getting propagated when building examples.
-          # Workaround bug in CMake < 3.17.3 when using MPI::MPI_C and CUDA
-          target_include_directories(${_actual_target_name} PUBLIC ${MPI_C_INCLUDE_DIRS})
-          target_link_libraries(${_actual_target_name} PUBLIC ${MPI_C_LIBRARIES})
-        endif()
-      endif()
-
       if(SUNDIALS_BUILD_WITH_PROFILING)
         if(ENABLE_CALIPER)
           target_link_libraries(${_actual_target_name} PUBLIC caliper)
@@ -306,7 +296,7 @@ macro(sundials_add_library target)
       if(${_libtype} MATCHES "STATIC")
         target_compile_definitions(${_actual_target_name} PRIVATE SUNDIALS_STATIC_DEFINE)
       else()
-        target_compile_definitions(${obj_target} PRIVATE sundials_generic_EXPORTS)
+        target_compile_definitions(${obj_target} PRIVATE sundials_core_EXPORTS)
       endif()
 
       # add all other compile definitions

diff --git a/doc/arkode/guide/source/Introduction.rst b/doc/arkode/guide/source/Introduction.rst
@@ -133,10 +133,43 @@ Changes from previous versions
 Changes in vX.X.X
 -----------------
 
+**Breaking change** 
+We have replaced the use of a type-erased (i.e., ``void*``) pointer to a
+communicator in place of ``MPI_Comm`` throughout the SUNDIALS API with a
+:c:type:`SUNComm`, which is just a typedef to an ``int`` in builds without MPI
+and a typedef to a ``MPI_Comm`` in builds with MPI. Here is what this means:
+
+- All users will need to update their codes because the call to 
+  :c:func:`SUNContext_Create` now takes a :c:type:`SUNComm` instead
+  of type-erased pointer to a communicator. For non-MPI codes,
+  pass :c:type:`SUN_COMM_NULL` to the ``comm`` argument instead of
+  ``NULL``. For MPI codes, pass the ``MPI_Comm`` directly. 
+  The required change should be doable with a find-and-replace. 
+
+- The same change must be made for calls to 
+  :c:func:`SUNLogger_Create` or :c:func:`SUNProfiler_Create`. 
+
+- Some users will need to update their calls to ``N_VGetCommunicator``, and 
+  update any custom ``N_Vector`` implementations tht provide 
+  ``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>_`.
+
+**Breaking change**
+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.
 
+**Breaking change**
+Users now need to link to ``sundials_core`` in addition to the libraries already linked to. 
+This will be picked up automatically in projects that use the SUNDIALS CMake target. The library ``sundials_generic`` has been superceded by ``sundials_core`` and is no longer available.
+This fixes some duplicate symbol errors on Windows when linking to multiple SUNDIALS libraries.
+
 The previously deprecated types ``realtype`` and ``booleantype`` were removed
 from ``sundials_types.h`` and replaced with ``sunrealtype`` and
 ``sunbooleantype``. The deprecated names for these types can be used by including

diff --git a/doc/arkode/guide/source/Usage/General.rst b/doc/arkode/guide/source/Usage/General.rst
@@ -207,4 +207,3 @@ linear solver were performed using the ARKBBDPRE module, the header
 initialization routines.
 
 
-.. include:: ../../../../shared/Types.rst
diff --git a/doc/arkode/guide/source/index.rst b/doc/arkode/guide/source/index.rst
@@ -66,7 +66,7 @@ with support by the `US Department of Energy <http://www.doe.gov>`_,
    sunnonlinsol/index.rst
    sunadaptcontroller/index.rst
    sunmemory/index.rst
-   Install_link.rst
+   sundials/Install_link.rst
    Constants
    Butcher
    History_link.rst

diff --git a/doc/arkode/guide/source/sundials/Install_link.rst b/doc/arkode/guide/source/sundials/Install_link.rst
@@ -0,0 +1,13 @@
+.. ----------------------------------------------------------------
+   SUNDIALS Copyright Start
+   Copyright (c) 2002-2023, Lawrence Livermore National Security
+   and Southern Methodist University.
+   All rights reserved.
+
+   See the top-level LICENSE and NOTICE files for details.
+
+   SPDX-License-Identifier: BSD-3-Clause
+   SUNDIALS Copyright End
+   ----------------------------------------------------------------
+
+.. include:: ../../../../shared/sundials/Install.rst
diff --git a/doc/cvodes/guide/source/Install_link.rst → ...kode/guide/source/sundials/Types_link.rst b/doc/cvodes/guide/source/Install_link.rst → ...kode/guide/source/sundials/Types_link.rst
@@ -10,4 +10,4 @@
    SUNDIALS Copyright End
    ----------------------------------------------------------------
 
-.. include:: ../../../shared/Install.rst
+.. include:: ../../../../shared/sundials/Types.rst
diff --git a/doc/arkode/guide/source/sundials/index.rst b/doc/arkode/guide/source/sundials/index.rst
@@ -23,6 +23,7 @@ six packages all leverage some other common infrastructure, which we discuss
 in this section.
 
 .. toctree::
+   Types_link
    SUNContext_link
    Logging_link
    Profiling_link