From 9a82481024f25beb8ad9c92ddbe2d12500c25bd5 Mon Sep 17 00:00:00 2001 From: Johannes Holke Date: Tue, 25 Nov 2025 11:16:52 +0100 Subject: [PATCH 1/5] Let t8_shmem_init return the size of the intranode communicator --- src/t8_data/t8_shmem.c | 8 ++++++-- src/t8_data/t8_shmem.h | 5 ++++- test/t8_data/t8_gtest_shmem.cxx | 3 ++- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/t8_data/t8_shmem.c b/src/t8_data/t8_shmem.c index 64468fa175..6403ce393f 100644 --- a/src/t8_data/t8_shmem.c +++ b/src/t8_data/t8_shmem.c @@ -60,7 +60,7 @@ t8_shmem_array_is_initialized (const t8_shmem_array_t array) } #endif -void +int t8_shmem_init (sc_MPI_Comm comm) { /* Check whether intranode and internode comms are set @@ -73,8 +73,12 @@ t8_shmem_init (sc_MPI_Comm comm) if (intranode == sc_MPI_COMM_NULL || internode == sc_MPI_COMM_NULL) { /* The inter/intra comms are not set. We need to set them to * initialize shared memory usage. */ - sc_mpi_comm_get_and_attach (comm); + return sc_mpi_comm_get_and_attach (comm); } + int intranode_size; + const int mpiret = sc_MPI_Comm_size (intranode, &intranode_size); + SC_CHECK_MPI (mpiret); + return intranode_size; } void diff --git a/src/t8_data/t8_shmem.h b/src/t8_data/t8_shmem.h index 3840505d4a..07653d3ab0 100644 --- a/src/t8_data/t8_shmem.h +++ b/src/t8_data/t8_shmem.h @@ -53,8 +53,11 @@ T8_EXTERN_C_BEGIN (); * \note This function needs to be called to enable shared memory usage for a communicator. * \note Calling this function multiple times with the same communicator is safe and does * not change the behaviour. + * \return If the intranode communicator cannot be + * obtained, return 0. + * Otherwise return size of intranode communicator. */ -void +int t8_shmem_init (sc_MPI_Comm comm); #if T8_ENABLE_DEBUG diff --git a/test/t8_data/t8_gtest_shmem.cxx b/test/t8_data/t8_gtest_shmem.cxx index eadb30c389..bbed280d34 100644 --- a/test/t8_data/t8_gtest_shmem.cxx +++ b/test/t8_data/t8_gtest_shmem.cxx @@ -58,8 +58,9 @@ TEST_P (shmem, test_shmem_init_finalize) int mpiret; /* setup shared memory usage */ - t8_shmem_init (comm); + const int intrasize_from_init = t8_shmem_init (comm); + ASSERT_GT (intrasize_from_init, 0) << "Error in t8_shmem_init. No intranode communicator set."; /* Get intranode and internode comm */ sc_mpi_comm_get_node_comms (comm, &intranode, &internode); From 1233b2da955bb5afb7f970431e05596af66662b1 Mon Sep 17 00:00:00 2001 From: Johannes Holke Date: Tue, 25 Nov 2025 11:25:45 +0100 Subject: [PATCH 2/5] Use return value of t8_shmem_init when called --- src/t8_cmesh/t8_cmesh_commit.cxx | 2 +- src/t8_cmesh/t8_cmesh_partition.cxx | 2 +- src/t8_cmesh/t8_cmesh_save.cxx | 2 +- src/t8_forest/t8_forest_partition.cxx | 8 ++++---- src/t8_vtk/t8_with_vtk/t8_vtk_reader.cxx | 2 +- .../t8_gtest_cmesh_set_partition_offsets.cxx | 7 +++++-- test/t8_data/t8_gtest_shmem.cxx | 13 +++++++++---- 7 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/t8_cmesh/t8_cmesh_commit.cxx b/src/t8_cmesh/t8_cmesh_commit.cxx index 80159e061e..c7bc5c461c 100644 --- a/src/t8_cmesh/t8_cmesh_commit.cxx +++ b/src/t8_cmesh/t8_cmesh_commit.cxx @@ -241,7 +241,7 @@ t8_cmesh_commit_partitioned_new (t8_cmesh_t cmesh, sc_MPI_Comm comm) /* TODO: reset cmesh */ return; } - t8_shmem_init (comm); + SC_CHECK_ABORT (t8_shmem_init (comm) > 0, "Error in shared memory setup. Could not build Cmesh."); t8_cmesh_set_shmem_type (comm); /* TODO: do we actually need the shared array? */ t8_stash_attribute_sort (cmesh->stash); diff --git a/src/t8_cmesh/t8_cmesh_partition.cxx b/src/t8_cmesh/t8_cmesh_partition.cxx index 4f9a88a173..508f9c430b 100644 --- a/src/t8_cmesh/t8_cmesh_partition.cxx +++ b/src/t8_cmesh/t8_cmesh_partition.cxx @@ -195,7 +195,7 @@ t8_cmesh_gather_treecount_ext (const t8_cmesh_t cmesh, sc_MPI_Comm comm, const i tree_offset = cmesh->first_tree_shared ? -cmesh->first_tree - 1 : cmesh->first_tree; if (cmesh->tree_offsets == NULL) { - t8_shmem_init (comm); + SC_CHECK_ABORT (t8_shmem_init (comm) > 0, "Error in shared memory setup."); t8_shmem_set_type (comm, T8_SHMEM_BEST_TYPE); /* Only allocate the shmem array, if it is not already allocated */ cmesh->tree_offsets = t8_cmesh_alloc_offsets (cmesh->mpisize, comm); diff --git a/src/t8_cmesh/t8_cmesh_save.cxx b/src/t8_cmesh/t8_cmesh_save.cxx index f6c9b77cc1..19d0837737 100644 --- a/src/t8_cmesh/t8_cmesh_save.cxx +++ b/src/t8_cmesh/t8_cmesh_save.cxx @@ -834,7 +834,7 @@ t8_cmesh_load_and_distribute (const char *fileprefix, const int num_files, sc_MP T8_ASSERT (mpisize >= num_files); /* Try to set the comm type */ - t8_shmem_init (comm); + SC_CHECK_ABORT (t8_shmem_init (comm) > 0, "Error in shared memory setup. Could not load cmesh."); t8_shmem_set_type (comm, T8_SHMEM_BEST_TYPE); /* Use cmesh_bcast, if only one process loads the cmesh: */ diff --git a/src/t8_forest/t8_forest_partition.cxx b/src/t8_forest/t8_forest_partition.cxx index 994b1d5f83..7042fc9ed1 100644 --- a/src/t8_forest/t8_forest_partition.cxx +++ b/src/t8_forest/t8_forest_partition.cxx @@ -104,7 +104,7 @@ t8_forest_partition_create_offsets (t8_forest_t forest) t8_debugf ("Building offsets for forest %p\n", (void *) forest); comm = forest->mpicomm; /* Set the shmem array type of comm */ - t8_shmem_init (comm); + SC_CHECK_ABORT (t8_shmem_init (comm) > 0, "Error in shared memory setup. Could not partition forest."); t8_shmem_set_type (comm, T8_SHMEM_BEST_TYPE); /* Initialize the offset array as a shmem array * holding mpisize+1 many t8_gloidx_t */ @@ -280,7 +280,7 @@ t8_forest_partition_create_first_desc (t8_forest_t forest) if (forest->global_first_desc == NULL) { /* Set the shmem array type of comm */ - t8_shmem_init (comm); + SC_CHECK_ABORT (t8_shmem_init (comm) > 0, "Error in shared memory setup. Could not partition forest."); t8_shmem_set_type (comm, T8_SHMEM_BEST_TYPE); /* Initialize the offset array as a shmem array * holding mpisize+1 many t8_linearidx_t to store the elements linear ids */ @@ -383,7 +383,7 @@ t8_forest_partition_create_tree_offsets (t8_forest_t forest) if (forest->tree_offsets == NULL) { /* Set the shmem array type of comm */ - t8_shmem_init (comm); + SC_CHECK_ABORT (t8_shmem_init (comm) > 0, "Error in shared memory setup. Could not partition forest."); t8_shmem_set_type (comm, T8_SHMEM_BEST_TYPE); /* Only allocate the shmem array, if it is not already allocated */ t8_shmem_array_init (&forest->tree_offsets, sizeof (t8_gloidx_t), forest->mpisize + 1, comm); @@ -448,7 +448,7 @@ t8_forest_partition_compute_new_offset (t8_forest_t forest) T8_ASSERT (forest->element_offsets == NULL); /* Set the shmem array type to comm */ - t8_shmem_init (comm); + SC_CHECK_ABORT (t8_shmem_init (comm) > 0, "Error in shared memory setup. Could not partition forest."); t8_shmem_set_type (comm, T8_SHMEM_BEST_TYPE); /* Initialize the shmem array */ t8_shmem_array_init (&forest->element_offsets, sizeof (t8_gloidx_t), forest->mpisize + 1, comm); diff --git a/src/t8_vtk/t8_with_vtk/t8_vtk_reader.cxx b/src/t8_vtk/t8_with_vtk/t8_vtk_reader.cxx index 2b847e4fe7..31871f32c0 100644 --- a/src/t8_vtk/t8_with_vtk/t8_vtk_reader.cxx +++ b/src/t8_vtk/t8_with_vtk/t8_vtk_reader.cxx @@ -259,7 +259,7 @@ t8_vtk_partition (t8_cmesh_t cmesh, const int mpirank, const int mpisize, t8_glo t8_gloidx_t first_tree = 0; t8_gloidx_t last_tree = 1; /* Compute the global id of the first tree on each proc. */ - t8_shmem_init (comm); + SC_CHECK_ABORT (t8_shmem_init (comm) > 0, "Error in shared memory setup in vtk output."); t8_shmem_set_type (comm, T8_SHMEM_BEST_TYPE); t8_shmem_array_t offsets = NULL; t8_shmem_array_init (&offsets, sizeof (t8_gloidx_t), mpisize + 1, comm); diff --git a/test/t8_cmesh/t8_gtest_cmesh_set_partition_offsets.cxx b/test/t8_cmesh/t8_gtest_cmesh_set_partition_offsets.cxx index 812840a971..38aff21831 100644 --- a/test/t8_cmesh/t8_gtest_cmesh_set_partition_offsets.cxx +++ b/test/t8_cmesh/t8_gtest_cmesh_set_partition_offsets.cxx @@ -116,7 +116,8 @@ TEST_P (cmesh_set_partition_offsets_nocommit, test_set_offsets) * the array corresponds to any valid partition. * We use the offset_concentrate function to build an offset array for a partition * that concentrates all trees at one process. */ - t8_shmem_init (sc_MPI_COMM_WORLD); + const int intranode_size = t8_shmem_init (sc_MPI_COMM_WORLD); + ASSERT_GT (intranode_size, 0) << "Could not initialize shared memory."; t8_shmem_array_t shmem_array = t8_cmesh_offset_concentrate (main_process, sc_MPI_COMM_WORLD, inum_trees); /* Set the partition offsets */ @@ -135,7 +136,9 @@ TEST_P (cmesh_set_partition_offsets_commit, test_set_offsets) * the array corresponds to any valid partition. * We use the offset_concentrate function to build an offset array for a partition * that concentrates all trees at one process. */ - t8_shmem_init (comm); + const int intranode_size = t8_shmem_init (comm); + ASSERT_GT (intranode_size, 0) << "Could not initialize shared memory."; + t8_shmem_array_t shmem_array = t8_cmesh_offset_concentrate (main_process, comm, inum_trees); /* Set the partition offsets */ diff --git a/test/t8_data/t8_gtest_shmem.cxx b/test/t8_data/t8_gtest_shmem.cxx index bbed280d34..5586e38f9a 100644 --- a/test/t8_data/t8_gtest_shmem.cxx +++ b/test/t8_data/t8_gtest_shmem.cxx @@ -114,7 +114,9 @@ TEST_P (shmem, test_sc_shmem_alloc) t8_debugf ("Checking shared memory type %s.\n", sc_shmem_type_to_string[shmem_type]); /* setup shared memory usage */ - t8_shmem_init (comm); + const int intranode_size = t8_shmem_init (comm); + ASSERT_GT (intranode_size, 0) << "Could not initialize shared memory."; + t8_shmem_set_type (comm, shmem_type); #if T8_ENABLE_MPI @@ -197,7 +199,8 @@ TEST_P (shmem, test_shmem_array_allgatherv) const sc_shmem_type_t shmem_type = (sc_shmem_type_t) shmem_type_int; /* setup shared memory usage */ - t8_shmem_init (comm); + const int intranode_size = t8_shmem_init (comm); + ASSERT_GT (intranode_size, 0) << "Could not initialize shared memory."; t8_shmem_set_type (comm, shmem_type); #if T8_ENABLE_MPI @@ -247,7 +250,8 @@ TEST_P (shmem, test_shmem_array_prefix) const sc_shmem_type_t shmem_type = (sc_shmem_type_t) shmem_type_int; /* setup shared memory usage */ - t8_shmem_init (comm); + const int intranode_size = t8_shmem_init (comm); + ASSERT_GT (intranode_size, 0) << "Could not initialize shared memory."; t8_shmem_set_type (comm, shmem_type); #if T8_ENABLE_MPI @@ -292,7 +296,8 @@ TEST_P (shmem, test_shmem_array) const sc_shmem_type_t shmem_type = (sc_shmem_type_t) shmem_type_int; /* setup shared memory usage */ - t8_shmem_init (comm); + const int intranode_size = t8_shmem_init (comm); + ASSERT_GT (intranode_size, 0) << "Could not initialize shared memory."; t8_shmem_set_type (comm, shmem_type); #if T8_ENABLE_MPI From f2f69cd8a9ae49ed907a372a7696e8b0f493e6b9 Mon Sep 17 00:00:00 2001 From: Johannes Holke Date: Wed, 26 Nov 2025 09:55:04 +0100 Subject: [PATCH 3/5] Dont initialize shmem if MPI is not enabled --- src/t8_data/t8_shmem.c | 8 ++++++++ test/t8_data/t8_gtest_shmem.cxx | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/t8_data/t8_shmem.c b/src/t8_data/t8_shmem.c index 6403ce393f..16642b970b 100644 --- a/src/t8_data/t8_shmem.c +++ b/src/t8_data/t8_shmem.c @@ -63,6 +63,14 @@ t8_shmem_array_is_initialized (const t8_shmem_array_t array) int t8_shmem_init (sc_MPI_Comm comm) { +#ifndef T8_ENABLE_MPI + // If we do not use MPI, there is nothing to do. + // We only have a single process. + return 1; +#endif +#ifndef SC_ENABLE_MPICOMMSHARED + SC_ABORT ("Trying to use shared memory but SC_ENABLE_MPICOMMSHARED is not set."); +#endif /* Check whether intranode and internode comms are set * for the current communicator. */ sc_MPI_Comm intranode; diff --git a/test/t8_data/t8_gtest_shmem.cxx b/test/t8_data/t8_gtest_shmem.cxx index 5586e38f9a..d2594bd668 100644 --- a/test/t8_data/t8_gtest_shmem.cxx +++ b/test/t8_data/t8_gtest_shmem.cxx @@ -59,8 +59,8 @@ TEST_P (shmem, test_shmem_init_finalize) /* setup shared memory usage */ const int intrasize_from_init = t8_shmem_init (comm); - ASSERT_GT (intrasize_from_init, 0) << "Error in t8_shmem_init. No intranode communicator set."; + /* Get intranode and internode comm */ sc_mpi_comm_get_node_comms (comm, &intranode, &internode); From a7cf3ff1db7b40a57feab95bc34e6546ec789bea Mon Sep 17 00:00:00 2001 From: Johannes Holke Date: Wed, 26 Nov 2025 10:04:17 +0100 Subject: [PATCH 4/5] Ignore warnings when executing CMake tests --- CMakeLists.txt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 84bed9ad77..4759be3c1a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -20,6 +20,12 @@ cmake_minimum_required( VERSION 3.16 ) +# When executing cmesh test, i.e. check_c_source_compiles/check_cxx_source_compiles and others +# we need to ignore the -Werror option, since these tests often produce warnings (see check_mpicommshared.cmake for example). +# If theses warnings are interpreted as errors, the test fails even though we want it to pass. +# This behaviour caused a serious bug and did not initialize our shared memory. See https://github.com/DLR-AMR/t8code/issues/1985. +set(CMAKE_REQUIRED_FLAGS "-w") + include( cmake/GitProjectVersion.cmake ) project( From d6a3a9bd36eeb5300a570187aba3478e4b7ae4db Mon Sep 17 00:00:00 2001 From: Johannes Holke Date: Thu, 27 Nov 2025 11:28:42 +0100 Subject: [PATCH 5/5] Added a clear error message when shared memory is not initialized --- src/t8_data/t8_shmem.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/t8_data/t8_shmem.c b/src/t8_data/t8_shmem.c index 16642b970b..728abe982c 100644 --- a/src/t8_data/t8_shmem.c +++ b/src/t8_data/t8_shmem.c @@ -69,7 +69,8 @@ t8_shmem_init (sc_MPI_Comm comm) return 1; #endif #ifndef SC_ENABLE_MPICOMMSHARED - SC_ABORT ("Trying to use shared memory but SC_ENABLE_MPICOMMSHARED is not set."); + SC_ABORT ("Trying to use shared memory but SC_ENABLE_MPICOMMSHARED is not set. This should not happen if you use MPI " + "v.3.0 or higher. Maybe related to https://github.com/DLR-AMR/t8code/pull/1996."); #endif /* Check whether intranode and internode comms are set * for the current communicator. */