Merge pull request #3101 from heplesser/fix-3099

Avoid MPI deadlocks on SynapseCollection.get()/set()

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_;

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_();
 

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
 

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 <http://www.gnu.org/licenses/>.
+
+
+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 not cause MPI deadlock."""
+
+    if conns:
+        conns.weight = 2.5
+
+
+def test_set_weight(conns):
+    """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 not cause MPI deadlock."""
+
+    if conns:
+        nest.SetStatus(conns, "weight", 2.5)