From c87f89f6448b19f4e694e72bff47bf3ba35c40aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-No=C3=ABl=20Grad?= Date: Tue, 27 Feb 2024 12:36:52 +0100 Subject: [PATCH 1/2] Revert "Restrict Boost range" This reverts commit f4590fd59824efb68c48eacdcb0ae43ba56c94bd. --- .github/workflows/push_pull.yml | 2 +- CMakeLists.txt | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/.github/workflows/push_pull.yml b/.github/workflows/push_pull.yml index 94d9bf24d9..1a2eb4e39f 100644 --- a/.github/workflows/push_pull.yml +++ b/.github/workflows/push_pull.yml @@ -10,7 +10,7 @@ permissions: jobs: macos: runs-on: macos-12 - if: false + if: ${{ github.repository == 'espressomd/espresso' }} steps: - name: Checkout uses: actions/checkout@main diff --git a/CMakeLists.txt b/CMakeLists.txt index 5c643b7d4b..7c40cb72a7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -435,9 +435,6 @@ if(ESPRESSO_BUILD_TESTS) endif() find_package(Boost 1.74.0 REQUIRED ${BOOST_COMPONENTS}) -if(${Boost_VERSION} VERSION_GREATER_EQUAL 1.84.0) - message(FATAL_ERROR "Boost version ${Boost_VERSION} is unsupported.") -endif() # # Paths From 5776a20aa90d3c8b0425ca6a7ecfeeae7d9e7058 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-No=C3=ABl=20Grad?= Date: Tue, 27 Feb 2024 12:45:48 +0100 Subject: [PATCH 2/2] Fix several occurrences of undefined behavior All MPI-related static global variables were removed, replaced by non-owning pointers, or replaced by owning pointers that are reset at normal program termination. Objects that need a MPI environment during destruction (FFT MPI plans, parallel file writers, etc.) now keep a shared pointer to the MPI environment. MpiCallbacks unit tests now call `MpiCallbacks::loop()` on worker nodes to avoid a non-empty receive queue at destruction of the MPI environment (fatal error in MPICH 4.1+). --- src/core/MpiCallbacks.hpp | 31 ++++++++++-------- src/core/communication.cpp | 20 +++++++----- src/core/communication.hpp | 15 ++++++--- src/core/errorhandling.cpp | 13 ++++---- src/core/errorhandling.hpp | 3 +- .../tests/ReactionAlgorithm_test.cpp | 2 +- src/core/system/System.cpp | 7 ---- src/core/system/System.hpp | 7 ---- .../EspressoSystemStandAlone_test.cpp | 2 +- src/core/unit_tests/EspressoSystem_test.cpp | 2 +- src/core/unit_tests/MpiCallbacks_test.cpp | 23 +++++++------ src/core/unit_tests/Verlet_list_test.cpp | 2 +- src/core/unit_tests/ek_interface_test.cpp | 2 +- .../unit_tests/lb_particle_coupling_test.cpp | 2 +- src/python/espressomd/_init.pyx | 12 ++++++- src/python/espressomd/communication.pxd | 2 ++ src/python/espressomd/script_interface.pxd | 5 +-- src/python/espressomd/script_interface.pyx | 7 +++- src/script_interface/ContextManager.cpp | 9 +++--- src/script_interface/ContextManager.hpp | 2 +- src/script_interface/GlobalContext.hpp | 16 +++++----- .../electrostatics/CoulombScafacos.hpp | 12 +++++++ src/script_interface/h5md/h5md.hpp | 10 ++++++ .../magnetostatics/DipolarScafacos.hpp | 12 +++++++ .../tests/Accumulators_test.cpp | 32 +++++++++++-------- .../tests/ConstantpHEnsemble_test.cpp | 3 +- .../tests/GlobalContext_test.cpp | 25 +++++++++------ .../tests/ParallelExceptionHandler_test.cpp | 12 +++++-- .../tests/ReactionEnsemble_test.cpp | 3 +- src/script_interface/walberla/EKFFT.hpp | 17 ++++++++++ .../walberla_bridge/utils/ResourceManager.hpp | 5 +-- 31 files changed, 207 insertions(+), 108 deletions(-) diff --git a/src/core/MpiCallbacks.hpp b/src/core/MpiCallbacks.hpp index 5ea39c0e55..0174076a74 100644 --- a/src/core/MpiCallbacks.hpp +++ b/src/core/MpiCallbacks.hpp @@ -41,6 +41,7 @@ #include #include +#include #include #include @@ -201,8 +202,8 @@ class MpiCallbacks { template ::argument_types, std::tuple>>> - CallbackHandle(MpiCallbacks *cb, F &&f) - : m_id(cb->add(std::forward(f))), m_cb(cb) {} + CallbackHandle(std::shared_ptr cb, F &&f) + : m_id(cb->add(std::forward(f))), m_cb(std::move(cb)) {} CallbackHandle(CallbackHandle const &) = delete; CallbackHandle(CallbackHandle &&rhs) noexcept = default; @@ -211,7 +212,7 @@ class MpiCallbacks { private: int m_id; - MpiCallbacks *m_cb; + std::shared_ptr m_cb; public: /** @@ -237,7 +238,6 @@ class MpiCallbacks { m_cb->remove(m_id); } - MpiCallbacks *cb() const { return m_cb; } int id() const { return m_id; } }; @@ -255,9 +255,9 @@ class MpiCallbacks { } public: - explicit MpiCallbacks(boost::mpi::communicator comm, - bool abort_on_exit = true) - : m_abort_on_exit(abort_on_exit), m_comm(std::move(comm)) { + MpiCallbacks(boost::mpi::communicator comm, + std::shared_ptr mpi_env) + : m_comm(std::move(comm)), m_mpi_env(std::move(mpi_env)) { /* Add a dummy at id 0 for loop abort. */ m_callback_map.add(nullptr); @@ -268,7 +268,7 @@ class MpiCallbacks { ~MpiCallbacks() { /* Release the clients on exit */ - if (m_abort_on_exit && (m_comm.rank() == 0)) { + if (m_comm.rank() == 0) { try { abort_loop(); } catch (...) { @@ -447,22 +447,25 @@ class MpiCallbacks { */ boost::mpi::communicator const &comm() const { return m_comm; } + std::shared_ptr share_mpi_env() const { + return m_mpi_env; + } + private: /** * @brief Id for the @ref abort_loop. Has to be 0. */ - enum { LOOP_ABORT = 0 }; + static constexpr int LOOP_ABORT = 0; /** - * @brief If @ref abort_loop should be called on destruction - * on the head node. + * The MPI communicator used for the callbacks. */ - bool m_abort_on_exit; + boost::mpi::communicator m_comm; /** - * The MPI communicator used for the callbacks. + * The MPI environment used for the callbacks. */ - boost::mpi::communicator m_comm; + std::shared_ptr m_mpi_env; /** * Internal storage for the callback functions. diff --git a/src/core/communication.cpp b/src/core/communication.cpp index 00bfe20a0f..cbccb98384 100644 --- a/src/core/communication.cpp +++ b/src/core/communication.cpp @@ -35,6 +35,7 @@ #include #include +#include #include @@ -47,10 +48,7 @@ boost::mpi::communicator comm_cart; Communicator communicator{}; namespace Communication { -static auto const &mpi_datatype_cache = - boost::mpi::detail::mpi_datatype_cache(); -static std::shared_ptr mpi_env; -static std::unique_ptr m_callbacks; +static std::shared_ptr m_callbacks; /* We use a singleton callback class for now. */ MpiCallbacks &mpiCallbacks() { @@ -58,6 +56,12 @@ MpiCallbacks &mpiCallbacks() { return *m_callbacks; } + +std::shared_ptr mpiCallbacksHandle() { + assert(m_callbacks && "Mpi not initialized!"); + + return m_callbacks; +} } // namespace Communication using Communication::mpiCallbacks; @@ -66,14 +70,12 @@ int this_node = -1; namespace Communication { void init(std::shared_ptr mpi_env) { - Communication::mpi_env = std::move(mpi_env); - communicator.full_initialization(); Communication::m_callbacks = - std::make_unique(comm_cart); + std::make_shared(comm_cart, mpi_env); - ErrorHandling::init_error_handling(mpiCallbacks()); + ErrorHandling::init_error_handling(Communication::m_callbacks); #ifdef WALBERLA walberla::mpi_init(); @@ -83,6 +85,8 @@ void init(std::shared_ptr mpi_env) { cuda_on_program_start(); #endif } + +void deinit() { Communication::m_callbacks.reset(); } } // namespace Communication Communicator::Communicator() diff --git a/src/core/communication.hpp b/src/core/communication.hpp index bfaa9e5815..dfeee2a5f0 100644 --- a/src/core/communication.hpp +++ b/src/core/communication.hpp @@ -83,6 +83,7 @@ namespace Communication { * @brief Returns a reference to the global callback class instance. */ MpiCallbacks &mpiCallbacks(); +std::shared_ptr mpiCallbacksHandle(); } // namespace Communication /************************************************** @@ -124,12 +125,18 @@ namespace Communication { /** * @brief Init globals for communication. * - * and calls @ref cuda_on_program_start. Keeps a copy of - * the pointer to the mpi environment to keep it alive - * while the program is loaded. - * * @param mpi_env MPI environment that should be used */ void init(std::shared_ptr mpi_env); +void deinit(); } // namespace Communication + +struct MpiContainerUnitTest { + std::shared_ptr m_mpi_env; + MpiContainerUnitTest(int argc, char **argv) { + m_mpi_env = mpi_init(argc, argv); + Communication::init(m_mpi_env); + } + ~MpiContainerUnitTest() { Communication::deinit(); } +}; #endif diff --git a/src/core/errorhandling.cpp b/src/core/errorhandling.cpp index f6370dd11d..fd3329333b 100644 --- a/src/core/errorhandling.cpp +++ b/src/core/errorhandling.cpp @@ -34,6 +34,7 @@ #include #include #include +#include #include namespace ErrorHandling { @@ -44,13 +45,13 @@ namespace ErrorHandling { static std::unique_ptr runtimeErrorCollector; /** The callback loop we are on. */ -static Communication::MpiCallbacks *m_callbacks = nullptr; +static std::weak_ptr m_callbacks; -void init_error_handling(Communication::MpiCallbacks &cb) { - m_callbacks = &cb; +void init_error_handling(std::weak_ptr callbacks) { + m_callbacks = std::move(callbacks); runtimeErrorCollector = - std::make_unique(m_callbacks->comm()); + std::make_unique(m_callbacks.lock()->comm()); } RuntimeErrorStream _runtimeMessageStream(RuntimeError::ErrorLevel level, @@ -67,7 +68,7 @@ static void mpi_gather_runtime_errors_local() { REGISTER_CALLBACK(mpi_gather_runtime_errors_local) std::vector mpi_gather_runtime_errors() { - m_callbacks->call(mpi_gather_runtime_errors_local); + m_callbacks.lock()->call(mpi_gather_runtime_errors_local); return runtimeErrorCollector->gather(); } @@ -81,7 +82,7 @@ std::vector mpi_gather_runtime_errors_all(bool is_head_node) { } // namespace ErrorHandling void errexit() { - ErrorHandling::m_callbacks->comm().abort(1); + ErrorHandling::m_callbacks.lock()->comm().abort(1); std::abort(); } diff --git a/src/core/errorhandling.hpp b/src/core/errorhandling.hpp index 105613a471..83714a249a 100644 --- a/src/core/errorhandling.hpp +++ b/src/core/errorhandling.hpp @@ -31,6 +31,7 @@ #include "error_handling/RuntimeError.hpp" #include "error_handling/RuntimeErrorStream.hpp" +#include #include #include @@ -85,7 +86,7 @@ namespace ErrorHandling { * * @param callbacks Callbacks system the error handler should be on. */ -void init_error_handling(Communication::MpiCallbacks &callbacks); +void init_error_handling(std::weak_ptr callbacks); RuntimeErrorStream _runtimeMessageStream(RuntimeError::ErrorLevel level, const std::string &file, int line, diff --git a/src/core/reaction_methods/tests/ReactionAlgorithm_test.cpp b/src/core/reaction_methods/tests/ReactionAlgorithm_test.cpp index 5d3b08c11a..07b264cc2a 100644 --- a/src/core/reaction_methods/tests/ReactionAlgorithm_test.cpp +++ b/src/core/reaction_methods/tests/ReactionAlgorithm_test.cpp @@ -330,7 +330,7 @@ BOOST_FIXTURE_TEST_CASE(ReactionAlgorithm_test, ParticleFactory) { } int main(int argc, char **argv) { - mpi_init_stand_alone(argc, argv); + auto const mpi_handle = MpiContainerUnitTest(argc, argv); espresso::system = System::System::create(); espresso::system->set_cell_structure_topology(CellStructureType::REGULAR); ::System::set_system(espresso::system); diff --git a/src/core/system/System.cpp b/src/core/system/System.cpp index 66c0e35d78..a87f56ca34 100644 --- a/src/core/system/System.cpp +++ b/src/core/system/System.cpp @@ -463,10 +463,3 @@ unsigned System::get_global_ghost_flags() const { } } // namespace System - -void mpi_init_stand_alone(int argc, char **argv) { - auto mpi_env = mpi_init(argc, argv); - - // initialize the MpiCallbacks framework - Communication::init(mpi_env); -} diff --git a/src/core/system/System.hpp b/src/core/system/System.hpp index 4a5b1e853a..377cd8f1db 100644 --- a/src/core/system/System.hpp +++ b/src/core/system/System.hpp @@ -308,10 +308,3 @@ void reset_system(); bool is_system_set(); } // namespace System - -/** - * @brief Initialize MPI global state to run ESPResSo in stand-alone mode. - * Use this function in simulations written in C++, such as unit tests. - * The script interface has its own MPI initialization mechanism. - */ -void mpi_init_stand_alone(int argc, char **argv); diff --git a/src/core/unit_tests/EspressoSystemStandAlone_test.cpp b/src/core/unit_tests/EspressoSystemStandAlone_test.cpp index 257a2d7db7..19836a57bb 100644 --- a/src/core/unit_tests/EspressoSystemStandAlone_test.cpp +++ b/src/core/unit_tests/EspressoSystemStandAlone_test.cpp @@ -483,7 +483,7 @@ BOOST_FIXTURE_TEST_CASE(espresso_system_stand_alone, ParticleFactory) { } int main(int argc, char **argv) { - mpi_init_stand_alone(argc, argv); + auto const mpi_handle = MpiContainerUnitTest(argc, argv); espresso::system = System::System::create(); espresso::system->set_cell_structure_topology(CellStructureType::REGULAR); ::System::set_system(espresso::system); diff --git a/src/core/unit_tests/EspressoSystem_test.cpp b/src/core/unit_tests/EspressoSystem_test.cpp index 0d27ba8947..7b66a3827b 100644 --- a/src/core/unit_tests/EspressoSystem_test.cpp +++ b/src/core/unit_tests/EspressoSystem_test.cpp @@ -151,7 +151,7 @@ BOOST_FIXTURE_TEST_CASE(check_with_gpu, ParticleFactory, } int main(int argc, char **argv) { - mpi_init_stand_alone(argc, argv); + auto const mpi_handle = MpiContainerUnitTest(argc, argv); return boost::unit_test::unit_test_main(init_unit_test, argc, argv); } diff --git a/src/core/unit_tests/MpiCallbacks_test.cpp b/src/core/unit_tests/MpiCallbacks_test.cpp index 27884057eb..b189ddef22 100644 --- a/src/core/unit_tests/MpiCallbacks_test.cpp +++ b/src/core/unit_tests/MpiCallbacks_test.cpp @@ -29,6 +29,7 @@ #include "MpiCallbacks.hpp" #include +#include #include #include @@ -36,6 +37,7 @@ #include #include +static std::weak_ptr mpi_env; static bool called = false; BOOST_AUTO_TEST_CASE(invoke_test) { @@ -111,7 +113,7 @@ BOOST_AUTO_TEST_CASE(callback_model_t) { BOOST_AUTO_TEST_CASE(adding_function_ptr_cb) { boost::mpi::communicator world; - Communication::MpiCallbacks cb(world); + Communication::MpiCallbacks cb(world, ::mpi_env.lock()); void (*fp)(int, const std::string &) = [](int i, const std::string &s) { BOOST_CHECK_EQUAL(537, i); @@ -143,7 +145,7 @@ BOOST_AUTO_TEST_CASE(RegisterCallback) { Communication::RegisterCallback{fp}; boost::mpi::communicator world; - Communication::MpiCallbacks cb(world); + Communication::MpiCallbacks cb(world, ::mpi_env.lock()); called = false; @@ -157,11 +159,12 @@ BOOST_AUTO_TEST_CASE(RegisterCallback) { BOOST_AUTO_TEST_CASE(CallbackHandle) { boost::mpi::communicator world; - Communication::MpiCallbacks cbs(world); + auto const cbs = + std::make_shared(world, ::mpi_env.lock()); bool m_called = false; Communication::CallbackHandle cb( - &cbs, [&m_called](std::string s) { + cbs, [&m_called](std::string s) { BOOST_CHECK_EQUAL("CallbackHandle", s); m_called = true; @@ -170,7 +173,7 @@ BOOST_AUTO_TEST_CASE(CallbackHandle) { if (0 == world.rank()) { cb(std::string("CallbackHandle")); } else { - cbs.loop(); + cbs->loop(); BOOST_CHECK(called); } } @@ -184,7 +187,7 @@ BOOST_AUTO_TEST_CASE(call) { Communication::MpiCallbacks::add_static(fp); boost::mpi::communicator world; - Communication::MpiCallbacks cbs(world); + Communication::MpiCallbacks cbs(world, ::mpi_env.lock()); if (0 == world.rank()) { cbs.call(fp); @@ -205,7 +208,7 @@ BOOST_AUTO_TEST_CASE(call_all) { Communication::MpiCallbacks::add_static(fp); boost::mpi::communicator world; - Communication::MpiCallbacks cbs(world); + Communication::MpiCallbacks cbs(world, ::mpi_env.lock()); if (0 == world.rank()) { cbs.call_all(fp); @@ -226,7 +229,7 @@ BOOST_AUTO_TEST_CASE(check_exceptions) { Communication::MpiCallbacks::add_static(fp1); boost::mpi::communicator world; - Communication::MpiCallbacks cbs(world); + Communication::MpiCallbacks cbs(world, ::mpi_env.lock()); if (0 == world.rank()) { // can't call an unregistered callback @@ -234,11 +237,13 @@ BOOST_AUTO_TEST_CASE(check_exceptions) { } else { // can't call a callback from worker nodes BOOST_CHECK_THROW(cbs.call(fp1), std::logic_error); + cbs.loop(); } } int main(int argc, char **argv) { - boost::mpi::environment mpi_env(argc, argv); + auto const mpi_env = std::make_shared(argc, argv); + ::mpi_env = mpi_env; return boost::unit_test::unit_test_main(init_unit_test, argc, argv); } diff --git a/src/core/unit_tests/Verlet_list_test.cpp b/src/core/unit_tests/Verlet_list_test.cpp index 54a7fed4db..ae2bcdc1cb 100644 --- a/src/core/unit_tests/Verlet_list_test.cpp +++ b/src/core/unit_tests/Verlet_list_test.cpp @@ -273,7 +273,7 @@ BOOST_DATA_TEST_CASE_F(ParticleFactory, verlet_list_update, } int main(int argc, char **argv) { - mpi_init_stand_alone(argc, argv); + auto const mpi_handle = MpiContainerUnitTest(argc, argv); espresso::system = System::System::create(); espresso::system->set_cell_structure_topology(CellStructureType::REGULAR); ::System::set_system(espresso::system); diff --git a/src/core/unit_tests/ek_interface_test.cpp b/src/core/unit_tests/ek_interface_test.cpp index bff055e845..d70f9a714f 100644 --- a/src/core/unit_tests/ek_interface_test.cpp +++ b/src/core/unit_tests/ek_interface_test.cpp @@ -232,7 +232,7 @@ BOOST_AUTO_TEST_CASE(ek_interface_none) { BOOST_AUTO_TEST_SUITE_END() int main(int argc, char **argv) { - mpi_init_stand_alone(argc, argv); + auto const mpi_handle = MpiContainerUnitTest(argc, argv); espresso::system = System::System::create(); espresso::system->set_box_l(params.box_dimensions); espresso::system->set_time_step(params.time_step); diff --git a/src/core/unit_tests/lb_particle_coupling_test.cpp b/src/core/unit_tests/lb_particle_coupling_test.cpp index 542587dc89..536d9101a3 100644 --- a/src/core/unit_tests/lb_particle_coupling_test.cpp +++ b/src/core/unit_tests/lb_particle_coupling_test.cpp @@ -635,7 +635,7 @@ BOOST_AUTO_TEST_CASE(lb_exceptions) { } int main(int argc, char **argv) { - mpi_init_stand_alone(argc, argv); + auto const mpi_handle = MpiContainerUnitTest(argc, argv); espresso::system = System::System::create(); espresso::system->set_box_l(params.box_dimensions); espresso::system->set_time_step(params.time_step); diff --git a/src/python/espressomd/_init.pyx b/src/python/espressomd/_init.pyx index 0ef6ee52f9..03cd3a44d7 100644 --- a/src/python/espressomd/_init.pyx +++ b/src/python/espressomd/_init.pyx @@ -17,6 +17,7 @@ # along with this program. If not, see . # import sys +import atexit from . cimport script_interface from . cimport communication from libcpp.memory cimport shared_ptr @@ -28,7 +29,16 @@ communication.init(mpi_env) # Initialize script interface # Has to be _after_ mpi_init -script_interface.init(communication.mpiCallbacks()) +script_interface.init(communication.mpiCallbacksHandle()) + + +def session_shutdown(): + mpi_env.reset() + communication.deinit() + script_interface.deinit() + + +atexit.register(session_shutdown) # Block the worker nodes in the callback loop. # The head node is just returning to the user script. diff --git a/src/python/espressomd/communication.pxd b/src/python/espressomd/communication.pxd index aeb4f43adf..7065f96a9d 100644 --- a/src/python/espressomd/communication.pxd +++ b/src/python/espressomd/communication.pxd @@ -30,4 +30,6 @@ cdef extern from "communication.hpp": cdef extern from "communication.hpp" namespace "Communication": MpiCallbacks & mpiCallbacks() + shared_ptr[MpiCallbacks] mpiCallbacksHandle() void init(shared_ptr[environment]) + void deinit() diff --git a/src/python/espressomd/script_interface.pxd b/src/python/espressomd/script_interface.pxd index 7a78a2daf7..bf8e3e03fd 100644 --- a/src/python/espressomd/script_interface.pxd +++ b/src/python/espressomd/script_interface.pxd @@ -62,7 +62,7 @@ cdef extern from "script_interface/ContextManager.hpp" namespace "ScriptInterfac cdef extern from "script_interface/ContextManager.hpp" namespace "ScriptInterface": cdef cppclass ContextManager: - ContextManager(MpiCallbacks & , const Factory[ObjectHandle] & ) + ContextManager(const shared_ptr[MpiCallbacks] & , const Factory[ObjectHandle] & ) shared_ptr[ObjectHandle] make_shared(CreationPolicy, const string &, const VariantMap) except + shared_ptr[ObjectHandle] deserialize(const string &) except + string serialize(const ObjectHandle *) except + @@ -76,4 +76,5 @@ cdef extern from "script_interface/get_value.hpp" namespace "ScriptInterface": cdef extern from "script_interface/code_info/CodeInfo.hpp" namespace "ScriptInterface::CodeInfo": void check_features(const vector[string] & features) except + -cdef void init(MpiCallbacks &) +cdef void init(const shared_ptr[MpiCallbacks] &) +cdef void deinit() diff --git a/src/python/espressomd/script_interface.pyx b/src/python/espressomd/script_interface.pyx index 948a070f18..13469ab34b 100644 --- a/src/python/espressomd/script_interface.pyx +++ b/src/python/espressomd/script_interface.pyx @@ -571,10 +571,15 @@ def script_interface_register(c): return c -cdef void init(MpiCallbacks & cb): +cdef void init(const shared_ptr[MpiCallbacks] & cb): cdef Factory[ObjectHandle] f initialize(& f) global _om _om = make_shared[ContextManager](cb, f) + + +cdef void deinit(): + global _om + _om.reset() diff --git a/src/script_interface/ContextManager.cpp b/src/script_interface/ContextManager.cpp index 7bb3dfd37c..e7f20c3475 100644 --- a/src/script_interface/ContextManager.cpp +++ b/src/script_interface/ContextManager.cpp @@ -53,15 +53,16 @@ std::string ContextManager::serialize(const ObjectHandle *o) const { return Utils::pack(std::make_pair(policy(ctx), o->serialize())); } -ContextManager::ContextManager(Communication::MpiCallbacks &callbacks, - const Utils::Factory &factory) { +ContextManager::ContextManager( + std::shared_ptr const &callbacks, + const Utils::Factory &factory) { auto local_context = - std::make_shared(factory, callbacks.comm()); + std::make_shared(factory, callbacks->comm()); /* If there is only one node, we can treat all objects as local, and thus * never invoke any callback. */ m_global_context = - (callbacks.comm().size() > 1) + (callbacks->comm().size() > 1) ? std::make_shared(callbacks, local_context) : std::static_pointer_cast(local_context); diff --git a/src/script_interface/ContextManager.hpp b/src/script_interface/ContextManager.hpp index 7f5a098a59..47fa10a73b 100644 --- a/src/script_interface/ContextManager.hpp +++ b/src/script_interface/ContextManager.hpp @@ -67,7 +67,7 @@ class ContextManager { GLOBAL }; - ContextManager(Communication::MpiCallbacks &callbacks, + ContextManager(std::shared_ptr const &callbacks, const Utils::Factory &factory); /** diff --git a/src/script_interface/GlobalContext.hpp b/src/script_interface/GlobalContext.hpp index 090c811ef5..2188026b26 100644 --- a/src/script_interface/GlobalContext.hpp +++ b/src/script_interface/GlobalContext.hpp @@ -71,8 +71,8 @@ class GlobalContext : public Context { std::shared_ptr m_node_local_context; - bool m_is_head_node; boost::mpi::communicator const &m_comm; + bool m_is_head_node; ParallelExceptionHandler m_parallel_exception_handler; @@ -88,28 +88,28 @@ class GlobalContext : public Context { Communication::CallbackHandle cb_delete_handle; public: - GlobalContext(Communication::MpiCallbacks &callbacks, + GlobalContext(std::shared_ptr const &callbacks, std::shared_ptr node_local_context) : m_local_objects(), m_node_local_context(std::move(node_local_context)), - m_is_head_node(callbacks.comm().rank() == 0), m_comm(callbacks.comm()), + m_comm(callbacks->comm()), m_is_head_node(m_comm.rank() == 0), // NOLINTNEXTLINE(bugprone-throw-keyword-missing) - m_parallel_exception_handler(callbacks.comm()), - cb_make_handle(&callbacks, + m_parallel_exception_handler(m_comm), + cb_make_handle(callbacks, [this](ObjectId id, const std::string &name, const PackedMap ¶meters) { make_handle(id, name, parameters); }), - cb_set_parameter(&callbacks, + cb_set_parameter(callbacks, [this](ObjectId id, std::string const &name, PackedVariant const &value) { set_parameter(id, name, value); }), - cb_call_method(&callbacks, + cb_call_method(callbacks, [this](ObjectId id, std::string const &name, PackedMap const &arguments) { call_method(id, name, arguments); }), - cb_delete_handle(&callbacks, + cb_delete_handle(callbacks, [this](ObjectId id) { delete_handle(id); }) {} private: diff --git a/src/script_interface/electrostatics/CoulombScafacos.hpp b/src/script_interface/electrostatics/CoulombScafacos.hpp index 827942a38b..8ae3c52174 100644 --- a/src/script_interface/electrostatics/CoulombScafacos.hpp +++ b/src/script_interface/electrostatics/CoulombScafacos.hpp @@ -25,6 +25,8 @@ #include "Actor.hpp" +#include "core/MpiCallbacks.hpp" +#include "core/communication.hpp" #include "core/electrostatics/scafacos.hpp" #include "core/scafacos/ScafacosContextBase.hpp" @@ -32,6 +34,7 @@ #include "script_interface/scafacos/scafacos.hpp" #include +#include #include #include #include @@ -42,6 +45,8 @@ namespace ScriptInterface { namespace Coulomb { class CoulombScafacos : public Actor { + std::shared_ptr m_mpi_env_lock; + public: CoulombScafacos() { add_parameters({ @@ -77,6 +82,11 @@ class CoulombScafacos : public Actor { }); } + ~CoulombScafacos() override { + m_actor.reset(); + m_mpi_env_lock.reset(); + } + void do_construct(VariantMap const ¶ms) override { auto const method_name = get_value(params, "method_name"); auto const param_list = params.at("method_params"); @@ -89,6 +99,8 @@ class CoulombScafacos : public Actor { actor()->set_prefactor(prefactor); }); set_charge_neutrality_tolerance(params); + // MPI communicator is needed to destroy the FFT plans + m_mpi_env_lock = ::Communication::mpiCallbacksHandle()->share_mpi_env(); } Variant do_call_method(std::string const &name, diff --git a/src/script_interface/h5md/h5md.hpp b/src/script_interface/h5md/h5md.hpp index 8316bb8be7..6b7ed939fb 100644 --- a/src/script_interface/h5md/h5md.hpp +++ b/src/script_interface/h5md/h5md.hpp @@ -26,6 +26,8 @@ #ifdef H5MD +#include "core/MpiCallbacks.hpp" +#include "core/communication.hpp" #include "io/writer/h5md_core.hpp" #include "script_interface/ScriptInterface.hpp" @@ -66,8 +68,16 @@ class H5md : public AutoParameters { params, "file_path", "script_path", "fields", "mass_unit", "length_unit", "time_unit", "force_unit", "velocity_unit", "charge_unit"); + // MPI communicator is needed to close parallel file handles + m_mpi_env_lock = ::Communication::mpiCallbacksHandle()->share_mpi_env(); } + ~H5md() override { + m_h5md.reset(); + m_mpi_env_lock.reset(); + } + + std::shared_ptr m_mpi_env_lock; std::shared_ptr<::Writer::H5md::File> m_h5md; std::vector m_output_fields; }; diff --git a/src/script_interface/magnetostatics/DipolarScafacos.hpp b/src/script_interface/magnetostatics/DipolarScafacos.hpp index 41a188d5b8..11e42b159d 100644 --- a/src/script_interface/magnetostatics/DipolarScafacos.hpp +++ b/src/script_interface/magnetostatics/DipolarScafacos.hpp @@ -25,11 +25,15 @@ #include "Actor.hpp" +#include "core/MpiCallbacks.hpp" +#include "core/communication.hpp" #include "core/magnetostatics/scafacos.hpp" #include "core/scafacos/ScafacosContextBase.hpp" #include "script_interface/scafacos/scafacos.hpp" +#include +#include #include #include @@ -37,6 +41,7 @@ namespace ScriptInterface { namespace Dipoles { class DipolarScafacos : public Actor { + std::shared_ptr m_mpi_env_lock; public: DipolarScafacos() { @@ -50,6 +55,11 @@ class DipolarScafacos : public Actor { }); } + ~DipolarScafacos() override { + m_actor.reset(); + m_mpi_env_lock.reset(); + } + void do_construct(VariantMap const ¶ms) override { auto const method_name = get_value(params, "method_name"); auto const param_list = params.at("method_params"); @@ -64,6 +74,8 @@ class DipolarScafacos : public Actor { m_actor = make_dipolar_scafacos(method_name, method_params); actor()->set_prefactor(prefactor); }); + // MPI communicator is needed to destroy the FFT plans + m_mpi_env_lock = ::Communication::mpiCallbacksHandle()->share_mpi_env(); } Variant do_call_method(std::string const &name, diff --git a/src/script_interface/tests/Accumulators_test.cpp b/src/script_interface/tests/Accumulators_test.cpp index f5dafa478a..8cba30ee3f 100644 --- a/src/script_interface/tests/Accumulators_test.cpp +++ b/src/script_interface/tests/Accumulators_test.cpp @@ -39,6 +39,7 @@ #include #include +#include #include #include @@ -46,6 +47,8 @@ #include #include +static std::weak_ptr mpi_env; + namespace Observables { class MockObservable : public Observable { public: @@ -72,8 +75,7 @@ using MeanVarianceCalculator = using namespace ScriptInterface; -auto make_global_context(boost::mpi::communicator const &comm, - Communication::MpiCallbacks &cb) { +auto make_global_context(std::shared_ptr &cb) { Utils::Factory factory; factory.register_new("TestObs"); factory.register_new("TimeSeries"); @@ -81,16 +83,17 @@ auto make_global_context(boost::mpi::communicator const &comm, factory.register_new("MeanVarianceCalculator"); return std::make_shared( - cb, std::make_shared(factory, comm)); + cb, std::make_shared(factory, cb->comm())); } BOOST_AUTO_TEST_CASE(time_series) { boost::mpi::communicator world; - Communication::MpiCallbacks cb(world); - auto ctx = make_global_context(world, cb); + auto cb = + std::make_shared(world, ::mpi_env.lock()); + auto ctx = make_global_context(cb); if (world.rank() != 0) { - cb.loop(); + cb->loop(); } else { auto const obs = ctx->make_shared("TestObs", {}); BOOST_REQUIRE(obs != nullptr); @@ -131,11 +134,12 @@ BOOST_AUTO_TEST_CASE(time_series) { BOOST_AUTO_TEST_CASE(correlator) { boost::mpi::communicator world; - Communication::MpiCallbacks cb(world); - auto ctx = make_global_context(world, cb); + auto cb = + std::make_shared(world, ::mpi_env.lock()); + auto ctx = make_global_context(cb); if (world.rank() != 0) { - cb.loop(); + cb->loop(); } else { auto const obs = ctx->make_shared("TestObs", {}); BOOST_REQUIRE(obs != nullptr); @@ -204,11 +208,12 @@ BOOST_AUTO_TEST_CASE(correlator) { BOOST_AUTO_TEST_CASE(mean_variance) { boost::mpi::communicator world; - Communication::MpiCallbacks cb(world); - auto ctx = make_global_context(world, cb); + auto cb = + std::make_shared(world, ::mpi_env.lock()); + auto ctx = make_global_context(cb); if (world.rank() != 0) { - cb.loop(); + cb->loop(); } else { auto const obs = ctx->make_shared("TestObs", {}); @@ -260,7 +265,8 @@ BOOST_AUTO_TEST_CASE(mean_variance) { } int main(int argc, char **argv) { - boost::mpi::environment mpi_env(argc, argv); + auto const mpi_env = std::make_shared(argc, argv); + ::mpi_env = mpi_env; return boost::unit_test::unit_test_main(init_unit_test, argc, argv); } diff --git a/src/script_interface/tests/ConstantpHEnsemble_test.cpp b/src/script_interface/tests/ConstantpHEnsemble_test.cpp index c74451ba12..9b97ace966 100644 --- a/src/script_interface/tests/ConstantpHEnsemble_test.cpp +++ b/src/script_interface/tests/ConstantpHEnsemble_test.cpp @@ -32,6 +32,7 @@ #include "core/Particle.hpp" #include "core/cell_system/CellStructureType.hpp" +#include "core/communication.hpp" #include "core/particle_node.hpp" #include "core/unit_tests/ParticleFactory.hpp" @@ -131,7 +132,7 @@ BOOST_FIXTURE_TEST_CASE(ConstantpHEnsemble_test, ParticleFactory) { } int main(int argc, char **argv) { - mpi_init_stand_alone(argc, argv); + auto const mpi_handle = MpiContainerUnitTest(argc, argv); espresso::system = System::System::create(); espresso::system->set_cell_structure_topology(CellStructureType::REGULAR); ::System::set_system(espresso::system); diff --git a/src/script_interface/tests/GlobalContext_test.cpp b/src/script_interface/tests/GlobalContext_test.cpp index a1ef31d0a8..da0401ad41 100644 --- a/src/script_interface/tests/GlobalContext_test.cpp +++ b/src/script_interface/tests/GlobalContext_test.cpp @@ -26,12 +26,15 @@ #include #include +#include #include #include #include #include +static std::weak_ptr mpi_env; + namespace si = ScriptInterface; struct Dummy : si::ObjectHandle { @@ -54,19 +57,19 @@ struct Dummy : si::ObjectHandle { } }; -auto make_global_context(boost::mpi::communicator const &comm, - Communication::MpiCallbacks &cb) { +auto make_global_context(std::shared_ptr &cb) { Utils::Factory factory; factory.register_new("Dummy"); return std::make_shared( - cb, std::make_shared(factory, comm)); + cb, std::make_shared(factory, cb->comm())); } BOOST_AUTO_TEST_CASE(GlobalContext_make_shared) { boost::mpi::communicator world; - Communication::MpiCallbacks cb{world}; - auto ctx = make_global_context(world, cb); + auto cb = + std::make_shared(world, ::mpi_env.lock()); + auto ctx = make_global_context(cb); if (world.rank() == 0) { auto res = ctx->make_shared("Dummy", {}); @@ -74,14 +77,15 @@ BOOST_AUTO_TEST_CASE(GlobalContext_make_shared) { BOOST_CHECK_EQUAL(res->context(), ctx.get()); BOOST_CHECK_EQUAL(ctx->name(res.get()), "Dummy"); } else { - cb.loop(); + cb->loop(); } } BOOST_AUTO_TEST_CASE(GlobalContext_serialization) { boost::mpi::communicator world; - Communication::MpiCallbacks cb{world}; - auto ctx = make_global_context(world, cb); + auto cb = + std::make_shared(world, ::mpi_env.lock()); + auto ctx = make_global_context(cb); if (world.rank() == 0) { auto const serialized = [&]() { @@ -108,12 +112,13 @@ BOOST_AUTO_TEST_CASE(GlobalContext_serialization) { BOOST_REQUIRE(d3); BOOST_CHECK_EQUAL(boost::get(d3->get_parameter("id")), 3); } else { - cb.loop(); + cb->loop(); } } int main(int argc, char **argv) { - boost::mpi::environment mpi_env(argc, argv); + auto const mpi_env = std::make_shared(argc, argv); + ::mpi_env = mpi_env; return boost::unit_test::unit_test_main(init_unit_test, argc, argv); } diff --git a/src/script_interface/tests/ParallelExceptionHandler_test.cpp b/src/script_interface/tests/ParallelExceptionHandler_test.cpp index 49bea8104d..936d0b5c70 100644 --- a/src/script_interface/tests/ParallelExceptionHandler_test.cpp +++ b/src/script_interface/tests/ParallelExceptionHandler_test.cpp @@ -45,6 +45,7 @@ #include #include +#include #include #include @@ -52,6 +53,8 @@ namespace utf = boost::unit_test; +static std::weak_ptr mpi_env; + namespace Testing { struct Error : public std::exception {}; struct Warning : public std::exception {}; @@ -68,7 +71,8 @@ struct if_parallel_test { BOOST_TEST_DECORATOR(*utf::precondition(if_parallel_test())) BOOST_AUTO_TEST_CASE(parallel_exceptions) { boost::mpi::communicator world; - Communication::MpiCallbacks callbacks{world}; + auto callbacks = + std::make_shared(world, ::mpi_env.lock()); ErrorHandling::init_error_handling(callbacks); auto handler = ScriptInterface::ParallelExceptionHandler{world}; @@ -130,10 +134,14 @@ BOOST_AUTO_TEST_CASE(parallel_exceptions) { // runtime error messages are printed to stderr and cleared BOOST_CHECK_EQUAL(check_runtime_errors_local(), 0); } + if (world.rank() != 0) { + callbacks->loop(); + } } int main(int argc, char **argv) { - boost::mpi::environment mpi_env(argc, argv); + auto const mpi_env = std::make_shared(argc, argv); + ::mpi_env = mpi_env; return boost::unit_test::unit_test_main(init_unit_test, argc, argv); } diff --git a/src/script_interface/tests/ReactionEnsemble_test.cpp b/src/script_interface/tests/ReactionEnsemble_test.cpp index 932419b9fd..1f5ab5b5eb 100644 --- a/src/script_interface/tests/ReactionEnsemble_test.cpp +++ b/src/script_interface/tests/ReactionEnsemble_test.cpp @@ -33,6 +33,7 @@ #include "core/Particle.hpp" #include "core/cell_system/CellStructureType.hpp" +#include "core/communication.hpp" #include "core/particle_node.hpp" #include "core/unit_tests/ParticleFactory.hpp" @@ -262,7 +263,7 @@ BOOST_FIXTURE_TEST_CASE(ReactionEnsemble_test, ParticleFactory) { } int main(int argc, char **argv) { - mpi_init_stand_alone(argc, argv); + auto const mpi_handle = MpiContainerUnitTest(argc, argv); espresso::system = System::System::create(); espresso::system->set_cell_structure_topology(CellStructureType::REGULAR); ::System::set_system(espresso::system); diff --git a/src/script_interface/walberla/EKFFT.hpp b/src/script_interface/walberla/EKFFT.hpp index 6ff34dcbb6..5775024e14 100644 --- a/src/script_interface/walberla/EKFFT.hpp +++ b/src/script_interface/walberla/EKFFT.hpp @@ -27,7 +27,11 @@ #include "EKPoissonSolver.hpp" #include "LatticeWalberla.hpp" +#include "core/MpiCallbacks.hpp" +#include "core/communication.hpp" + #include +#include #include #include @@ -39,6 +43,7 @@ namespace ScriptInterface::walberla { class EKFFT : public EKPoissonSolver { + std::unique_ptr m_resources_lock; std::shared_ptr<::walberla::PoissonSolver> m_instance; std::shared_ptr m_lattice; double m_conv_permittivity; @@ -57,7 +62,13 @@ class EKFFT : public EKPoissonSolver { m_instance = ::walberla::new_ek_poisson_fft( m_lattice->lattice(), permittivity, m_single_precision); + m_resources_lock = std::make_unique(); + // MPI communicator is needed to destroy the FFT plans + m_resources_lock->acquire_lock( + Communication::mpiCallbacksHandle()->share_mpi_env()); + } + EKFFT() { add_parameters({ {"permittivity", [this](Variant const &v) { @@ -73,6 +84,12 @@ class EKFFT : public EKPoissonSolver { }); } + ~EKFFT() override { + m_lattice.reset(); + m_instance.reset(); + m_resources_lock.reset(); + } + [[nodiscard]] std::shared_ptr<::walberla::PoissonSolver> get_instance() const noexcept override { return m_instance; diff --git a/src/walberla_bridge/include/walberla_bridge/utils/ResourceManager.hpp b/src/walberla_bridge/include/walberla_bridge/utils/ResourceManager.hpp index 356b48557b..0ad77fa227 100644 --- a/src/walberla_bridge/include/walberla_bridge/utils/ResourceManager.hpp +++ b/src/walberla_bridge/include/walberla_bridge/utils/ResourceManager.hpp @@ -22,6 +22,7 @@ #include #include #include +#include /** * @brief Manager to control the lifetime of shared resources. @@ -58,8 +59,8 @@ class ResourceManager { std::shared_ptr m_resource; public: - explicit ResourceLockImpl(std::shared_ptr const &resource) - : m_resource(resource) {} + explicit ResourceLockImpl(std::shared_ptr resource) + : m_resource(std::move(resource)) {} ~ResourceLockImpl() override { m_resource.reset(); } };