From: David Wells <wellsd2@rpi.edu>
Date: Wed, 16 Aug 2017 02:18:12 +0000 (-0400)
Subject: Fix some #ifdef logic in the Timer class.
X-Git-Tag: v9.0.0-rc1~1236^2
X-Git-Url: https://gitweb.dealii.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c0a95eeb70223a5f24c88c49e88c8c12b7d5dfb9;p=dealii.git

Fix some #ifdef logic in the Timer class.

This fixes a bug due to rewritting an else statement where the 'else' was in an
'ifdef' and the body of the else statement (one line) was outside of it. This
caused, after a rewrite, the body of the former else statement to be executed
twice.

Before 9ef67983fd the end of this function was:

#ifdef DEAL_II_WITH_MPI
      if (sync_wall_time && Utilities::MPI::job_supports_mpi())
        {
          this->mpi_data
            = Utilities::MPI::min_max_avg (last_lap_time, mpi_communicator);
          last_lap_time = this->mpi_data.max;
          cumulative_wall_time += last_lap_time;
        }
      else
#endif
        cumulative_wall_time += last_lap_time;
    }
  return cumulative_time;

where the 'else' statement was run unconditionally if MPI was not present. The
previous (prior to this patch) version was:

#ifdef DEAL_II_WITH_MPI
      this->mpi_data = Utilities::MPI::min_max_avg (last_lap_time,
                                                    mpi_communicator);
      if (sync_wall_time && Utilities::MPI::job_supports_mpi())
        {
          last_lap_time = this->mpi_data.max;
          last_lap_cpu_time = Utilities::MPI::min_max_avg (last_lap_cpu_time,
                                                           mpi_communicator).max;
        }
      cumulative_wall_time += last_lap_time;
      cumulative_time += last_lap_cpu_time;
      this->mpi_total_data = Utilities::MPI::min_max_avg (cumulative_wall_time,
                                                          mpi_communicator);
#endif
      cumulative_wall_time += last_lap_time;
      cumulative_time += last_lap_cpu_time;
    }
  return cumulative_time;

which will, if MPI is available, double count both the cumulative wall time and
cummulative time.
---

diff --git a/source/base/timer.cc b/source/base/timer.cc
index 05745a8bad..ab17fbcd98 100644
--- a/source/base/timer.cc
+++ b/source/base/timer.cc
@@ -213,9 +213,10 @@ double Timer::stop ()
       cumulative_time += last_lap_cpu_time;
       this->mpi_total_data = Utilities::MPI::min_max_avg (cumulative_wall_time,
                                                           mpi_communicator);
-#endif
+#else
       cumulative_wall_time += last_lap_time;
       cumulative_time += last_lap_cpu_time;
+#endif
     }
   return cumulative_time;
 }
diff --git a/tests/base/timer_06.cc b/tests/base/timer_06.cc
new file mode 100644
index 0000000000..79e2e99014
--- /dev/null
+++ b/tests/base/timer_06.cc
@@ -0,0 +1,43 @@
+// ---------------------------------------------------------------------
+//
+// Copyright (C) 2017 by the deal.II authors
+//
+// This file is part of the deal.II library.
+//
+// The deal.II library is free software; you can use it, redistribute
+// it, and/or modify it under the terms of the GNU Lesser General
+// Public License as published by the Free Software Foundation; either
+// version 2.1 of the License, or (at your option) any later version.
+// The full text of the license can be found in the file LICENSE at
+// the top level of the deal.II distribution.
+//
+// ---------------------------------------------------------------------
+
+// Test that the wall time calculated by a timer agrees with the value
+// calculated by the system clock. This verifies that we fixed a bug where the
+// wall time was doubled.
+
+#include "../tests.h"
+#include <deal.II/base/timer.h>
+
+#include <chrono>
+#include <thread>
+
+int main (int argc, char **argv)
+{
+  initlog();
+
+  Timer timer;
+  timer.start();
+  const auto t0 = std::chrono::system_clock::now();
+  std::this_thread::sleep_for(std::chrono::seconds(4));
+  timer.stop();
+  const auto t1 = std::chrono::system_clock::now();
+
+  // verify that the timer wall time is not double the manually calculated one
+  AssertThrow(std::abs(double(std::chrono::duration_cast<std::chrono::seconds>(t1 - t0).count()) -
+                       timer.wall_time()) < 0.5,
+              ExcMessage("The measured times should be close."));
+
+  deallog << "OK" << std::endl;
+}
diff --git a/tests/base/timer_06.output b/tests/base/timer_06.output
new file mode 100644
index 0000000000..0fd8fc12f0
--- /dev/null
+++ b/tests/base/timer_06.output
@@ -0,0 +1,2 @@
+
+DEAL::OK