Updating the event manager for MPI communication #136

cssherman · 2018-08-27T18:25:57Z

No description provided.

artv3 · 2018-08-27T18:30:33Z

@cssherman , is this ready for a review?

cssherman · 2018-08-27T20:47:53Z

I still need to check on a few things with sub-events, I'll let you know when I think its ready for review

cssherman · 2018-08-27T23:13:12Z

src/components/core/src/managers/Events/EventBase.cpp

@@ -240,6 +240,13 @@ void EventBase::CheckEvents(real64 const time,
    {
      subEvent->CheckEvents(time, dt, cycle, domain);
    });
+
+    // Synchronize


This needs to occur here (instead of in event manager) because we need to know the forecast for all events and their children (to allow nesting)

cssherman · 2018-08-27T23:13:53Z

src/components/core/src/managers/Events/EventBase.cpp

@@ -377,4 +386,15 @@ void EventBase::Cleanup(real64 const& time_n,



+integer EventBase::GetExitFlag()


We were not propagating any exit flags upwards to the main run loop.

cssherman · 2018-08-27T23:17:43Z

@artv3 I think this is ready for you to look at. Also, it looks like the code is only working with 1-2 partitions along each dimension?

artv3 · 2018-08-28T16:58:19Z

@cssherman did you merge with a different branch? There seems to be a lot of commits unrelated to the event manager.

rrsettgast · 2018-08-29T05:43:55Z

src/components/core/src/managers/EventManager.cpp

+
+    #if USE_MPI
+      real64 dt_global;
+      MPI_Allreduce(&dt, &dt_global, 1, MPI_DOUBLE, MPI_MIN, MPI_COMM_WORLD);


I think you can replace the two MPI_Allreduce calls with a single MPI_Gather for a buffer containing both variables, local reduction of each component that was gathered...then a single MPI_Bcast for a buffer containing both variables...then you can overwrite the local variables with the values in the buffer.

…for testing time-stepping behavior

cssherman · 2018-08-29T22:40:34Z

src/components/core/src/managers/EventManager.cpp

      }      
    });

    time += dt;
    ++cycle;
    dt = nextDt;
    dt = (time + dt > maxTime) ? (maxTime - time) : dt;
+
+    #if USE_MPI


This should be more efficient than the two allreduce calls for the dt and exitFlag

cssherman · 2018-08-29T22:42:41Z

src/components/core/src/managers/Events/HaltEvent.cpp

@@ -86,6 +86,14 @@ void HaltEvent::EstimateEventTiming(real64 const time,
  m_realDt = currentTime - m_lastTime;
  m_lastTime = currentTime;
  integer forecast = static_cast<integer>((maxRuntime - (currentTime - m_startTime)) / m_realDt);
+
+  // The timing for the ranks may differ slightly, so synchronize


This Allreduce can happen here, or we could store a common time-stamp for each of the ranks somewhere.

How will these be different? It seems that currentTime and m_lastTime will always be in sync?? You could sync up m_startTime in the constructor when it is set....wait what is going on here? Why do you care about the real world runtime? Is this for a maximum wall time?

cssherman · 2018-08-29T22:45:30Z

src/components/core/src/managers/Events/PeriodicEvent.cpp

+
+    // Because the function applied to an object may differ by rank, synchronize
+    // (Note: this shouldn't occur very often, since it is only called if the base forecast <= 0)
+    #if USE_MPI


This allreduce should be fairly uncommon, since it will only be applied when the following apply:

The base forecast is 0

The forecast is being modified via a function/threshold being applied to an object (not a simple time-series)

cssherman · 2018-08-29T22:47:29Z

src/components/core/src/physicsSolvers/src/DummySolver.cpp

@@ -0,0 +1,107 @@
+/*


This solver is meant to toy around with random time-step requests. I've been using it to make sure that the event manager, mpi communications are handling the time-stepping correctly.

rrsettgast · 2018-08-30T18:32:25Z

src/components/core/src/managers/EventManager.cpp

+  #if USE_MPI
+    MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+    MPI_Comm_size(MPI_COMM_WORLD, &comm_size);
+    send_buffer = static_cast<real64 *>(malloc(2 * sizeof(real64)));


Not deallocated anywhere that I can see??

It is better just to do something like
real64 send_buffer[2];
array1d< real64 > receive_buffer( 2 * comm_size )

rrsettgast · 2018-08-30T18:37:38Z

src/components/core/src/managers/EventManager.cpp

+      {
+        for (integer ii=0; ii<comm_size; ii++)
+        {
+          send_buffer[0] = std::min(send_buffer[0], receive_buffer[2*ii]);


just use dt instead of send_buffer[0] as the argument?

rrsettgast · 2018-08-30T18:43:52Z

src/components/core/src/managers/Events/HaltEvent.cpp

@@ -86,6 +86,14 @@ void HaltEvent::EstimateEventTiming(real64 const time,
  m_realDt = currentTime - m_lastTime;
  m_lastTime = currentTime;
  integer forecast = static_cast<integer>((maxRuntime - (currentTime - m_startTime)) / m_realDt);
+
+  // The timing for the ranks may differ slightly, so synchronize


How will these be different? It seems that currentTime and m_lastTime will always be in sync?? You could sync up m_startTime in the constructor when it is set....wait what is going on here? Why do you care about the real world runtime? Is this for a maximum wall time?

cssherman · 2018-08-31T23:07:13Z

src/coreComponents/managers/Events/HaltEvent.cpp

@@ -86,6 +86,14 @@ void HaltEvent::EstimateEventTiming(real64 const time,
  m_realDt = currentTime - m_lastTime;
  m_lastTime = currentTime;
  integer forecast = static_cast<integer>((maxRuntime - (currentTime - m_startTime)) / m_realDt);
+
+  // The timing for the ranks may differ slightly, so synchronize


@rrsettgast - This event type is meant to handle max wall time. If the forecast for this type of event is <=0, then it will set the exitFlag. Because the currentTime and m_lastTime on each rank will be slightly different, we need to do this check to make sure that all of the ranks will agree.

If you would rather restrict this event from having a executable target, we could skip the allreduce here because the global exitFlag is synced in the main run loop.

Adding some allreduce calls to the event manager

bbb025f

cssherman requested review from rrsettgast and artv3 August 27, 2018 18:26

cssherman added 2 commits August 27, 2018 16:05

Working on the event manager mpi handling

4334656

Removing some debug info

2aed13c

cssherman commented Aug 27, 2018

View reviewed changes

Merge branch 'develop' into eventManagerMPI

bc1b042

cssherman mentioned this pull request Aug 28, 2018

running simulation with a time step > 1e6 s #143

Closed

Removing a hard-coded maximum timestep value

b88a5f0

rrsettgast reviewed Aug 29, 2018

View reviewed changes

cssherman added 2 commits August 29, 2018 15:32

Being more conservative with use of mpi calls, adding a dummy solver …

bfdf835

…for testing time-stepping behavior

Merge branch 'develop' into eventManagerMPI

99c9a9e

cssherman commented Aug 29, 2018

View reviewed changes

Changing a hard-coded limit in eventBase

1adf510

rrsettgast requested changes Aug 30, 2018

View reviewed changes

cssherman added 3 commits August 31, 2018 14:49

Updatind the buffer in the event manager

ba28f93

Merge branch 'develop' into eventManagerMPI

a0c459b

Moving dummy solver to the correct location

3df819a

cssherman commented Aug 31, 2018

View reviewed changes

rrsettgast approved these changes Sep 5, 2018

View reviewed changes

artv3 approved these changes Sep 5, 2018

View reviewed changes

Merge branch 'develop' into eventManagerMPI

ef00dde

cssherman merged commit f7990d8 into develop Sep 6, 2018

cssherman deleted the eventManagerMPI branch September 10, 2018 19:33

rrsettgast added a commit that referenced this pull request May 14, 2024

table formatting attempt #136

653e93a

rrsettgast added a commit that referenced this pull request May 14, 2024

table formatting attempt #136

2abfbc8

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updating the event manager for MPI communication #136

Updating the event manager for MPI communication #136

cssherman commented Aug 27, 2018

artv3 commented Aug 27, 2018

cssherman commented Aug 27, 2018

cssherman Aug 27, 2018

cssherman Aug 27, 2018

cssherman commented Aug 27, 2018

artv3 commented Aug 28, 2018

rrsettgast Aug 29, 2018

cssherman Aug 29, 2018

cssherman Aug 29, 2018

rrsettgast Aug 30, 2018

cssherman Aug 29, 2018

cssherman Aug 29, 2018

rrsettgast Aug 30, 2018

rrsettgast Aug 30, 2018

rrsettgast Aug 30, 2018

cssherman Aug 31, 2018

		@@ -377,4 +386,15 @@ void EventBase::Cleanup(real64 const& time_n,



		integer EventBase::GetExitFlag()

Updating the event manager for MPI communication #136

Updating the event manager for MPI communication #136

Conversation

cssherman commented Aug 27, 2018

artv3 commented Aug 27, 2018

cssherman commented Aug 27, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cssherman commented Aug 27, 2018

artv3 commented Aug 28, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment