From a9baf524de42428e940b7d349b6cd925a90b8d90 Mon Sep 17 00:00:00 2001 From: Hans Ekkehard Plesser Date: Sun, 11 Feb 2024 19:01:34 +0100 Subject: [PATCH 1/5] Use locally available global network size instead of num connections to detect ResetKernel --- pynest/nest/lib/hl_api_types.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/pynest/nest/lib/hl_api_types.py b/pynest/nest/lib/hl_api_types.py index 27763a83e3..00118855f5 100644 --- a/pynest/nest/lib/hl_api_types.py +++ b/pynest/nest/lib/hl_api_types.py @@ -816,13 +816,16 @@ def get(self, keys=None, output=""): {'source': [1, 1, 1, 2, 2, 2, 3, 3, 3], 'weight': [1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0]} """ + pandas_output = output == "pandas" if pandas_output and not HAVE_PANDAS: raise ImportError("Pandas could not be imported") - # Return empty dictionary if we have no connections or if we have done a nest.ResetKernel() - num_conns = GetKernelStatus("num_connections") # Has to be called first because it involves MPI communication. - if self.__len__() == 0 or num_conns == 0: + # Return empty dictionary if we have no connections + # We also return if the network is empty after a ResetKernel. + # This avoids problems with invalid SynapseCollections. + # See also #3100. + if self.__len__() == 0 or GetKernelStatus("network_size") == 0: # Return empty tuple if get is called with an argument return {} if keys is None else () @@ -885,7 +888,9 @@ def set(self, params=None, **kwargs): # This was added to ensure that the function is a nop (instead of, # for instance, raising an exception) when applied to an empty - # SynapseCollection, or after having done a nest.ResetKernel(). + # SynapseCollection. We also return if the network is empty after a + # reset kernel. This avoids problems with invalid SynapseCollections. + # See also #3100. if self.__len__() == 0 or GetKernelStatus("network_size") == 0: return From 8c6a9bd302b41e333c4fd107191e8700991944e8 Mon Sep 17 00:00:00 2001 From: Hans Ekkehard Plesser Date: Sun, 11 Feb 2024 19:02:22 +0100 Subject: [PATCH 2/5] Perform MPI communication of delay extrema only if they could have changed --- nestkernel/connection_manager.cpp | 15 ++++++++++++++- nestkernel/connection_manager.h | 3 +-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/nestkernel/connection_manager.cpp b/nestkernel/connection_manager.cpp index cb43973d51..7345f07193 100644 --- a/nestkernel/connection_manager.cpp +++ b/nestkernel/connection_manager.cpp @@ -464,6 +464,13 @@ nest::ConnectionManager::connect( TokenArray sources, TokenArray targets, const void nest::ConnectionManager::update_delay_extrema_() { + if ( kernel().simulation_manager.has_been_simulated() ) + { + // Once simulation has started, min/max_delay can no longer change, + // so there is nothing to update. + return; + } + min_delay_ = get_min_delay_time_().get_steps(); max_delay_ = get_max_delay_time_().get_steps(); @@ -475,7 +482,13 @@ nest::ConnectionManager::update_delay_extrema_() max_delay_ = std::max( max_delay_, kernel().sp_manager.builder_max_delay() ); } - if ( kernel().mpi_manager.get_num_processes() > 1 ) + // If the user explicitly set min/max_delay, this happend on all MPI ranks, + // so all ranks are up to date already. Also, once the user has set min/max_delay + // explicitly, Connect() cannot induce new extrema. Thuse, we only need to communicate + // with other ranks if the user has not set the extrema and connections may have + // been created. + if ( not kernel().connection_manager.get_user_set_delay_extrema() + and kernel().connection_manager.connections_have_changed() and kernel().mpi_manager.get_num_processes() > 1 ) { std::vector< long > min_delays( kernel().mpi_manager.get_num_processes() ); min_delays[ kernel().mpi_manager.get_rank() ] = min_delay_; diff --git a/nestkernel/connection_manager.h b/nestkernel/connection_manager.h index 4ffa12d0a5..315e2a1305 100644 --- a/nestkernel/connection_manager.h +++ b/nestkernel/connection_manager.h @@ -481,8 +481,7 @@ class ConnectionManager : public ManagerInterface /** * Update delay extrema to current values. * - * Static since it only operates in static variables. This allows it to be - * called from const-method get_status() as well. + * @note This entails MPI communication. */ void update_delay_extrema_(); From 743e3e974933387694d9f6f2e7a5d40737ddb6af Mon Sep 17 00:00:00 2001 From: Hans Ekkehard Plesser Date: Sun, 11 Feb 2024 19:24:44 +0100 Subject: [PATCH 3/5] Add test for issue 3099 --- testsuite/pytests/mpi/2/test_issue_3099.py | 53 ++++++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 testsuite/pytests/mpi/2/test_issue_3099.py diff --git a/testsuite/pytests/mpi/2/test_issue_3099.py b/testsuite/pytests/mpi/2/test_issue_3099.py new file mode 100644 index 0000000000..5eb22b5a08 --- /dev/null +++ b/testsuite/pytests/mpi/2/test_issue_3099.py @@ -0,0 +1,53 @@ +# -*- coding: utf-8 -*- +# +# test_issue_3099.py +# +# This file is part of NEST. +# +# Copyright (C) 2004 The NEST Initiative +# +# NEST is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 2 of the License, or +# (at your option) any later version. +# +# NEST is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with NEST. If not, see . + + +import nest +import pytest + + +@pytest.fixture +def conns(): + nest.ResetKernel() + nrn = nest.Create("parrot_neuron") + nest.Connect(nrn, nrn) + return nest.GetConnections() + + +def test_conn_weight(conns): + """Test that operation does cause MPI deadlock.""" + + if conns: + conns.weight = 2.5 + + +def test_set_weight(conns): + """Test that operation does cause MPI deadlock.""" + + if conns: + conns.set({"weight": 2.5}) + + +def test_set_status_weight(conns): + """Test that operation does cause MPI deadlock.""" + + if conns: + nest.SetStatus(conns, "weight", 2.5) From c9bedddbb60ac92a4b62d2e60ba5d8d07f13b8a7 Mon Sep 17 00:00:00 2001 From: Hans Ekkehard Plesser Date: Wed, 14 Feb 2024 21:18:08 +0100 Subject: [PATCH 4/5] Update testsuite/pytests/mpi/2/test_issue_3099.py Co-authored-by: Nicolai Haug <39106781+nicolossus@users.noreply.github.com> --- testsuite/pytests/mpi/2/test_issue_3099.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testsuite/pytests/mpi/2/test_issue_3099.py b/testsuite/pytests/mpi/2/test_issue_3099.py index 5eb22b5a08..42392f9820 100644 --- a/testsuite/pytests/mpi/2/test_issue_3099.py +++ b/testsuite/pytests/mpi/2/test_issue_3099.py @@ -33,7 +33,7 @@ def conns(): def test_conn_weight(conns): - """Test that operation does cause MPI deadlock.""" + """Test that operation does not cause MPI deadlock.""" if conns: conns.weight = 2.5 From 83350d4c213085dc9dd66a118f6cbd798b2d846b Mon Sep 17 00:00:00 2001 From: Hans Ekkehard Plesser Date: Wed, 14 Feb 2024 21:19:05 +0100 Subject: [PATCH 5/5] Apply suggestions from code review --- testsuite/pytests/mpi/2/test_issue_3099.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testsuite/pytests/mpi/2/test_issue_3099.py b/testsuite/pytests/mpi/2/test_issue_3099.py index 42392f9820..880ffc7aca 100644 --- a/testsuite/pytests/mpi/2/test_issue_3099.py +++ b/testsuite/pytests/mpi/2/test_issue_3099.py @@ -40,14 +40,14 @@ def test_conn_weight(conns): def test_set_weight(conns): - """Test that operation does cause MPI deadlock.""" + """Test that operation does not cause MPI deadlock.""" if conns: conns.set({"weight": 2.5}) def test_set_status_weight(conns): - """Test that operation does cause MPI deadlock.""" + """Test that operation does not cause MPI deadlock.""" if conns: nest.SetStatus(conns, "weight", 2.5)