]> https://gitweb.dealii.org/ - dealii.git/commitdiff
Fix some #ifdef logic in the Timer class. 4868/head
authorDavid Wells <wellsd2@rpi.edu>
Wed, 16 Aug 2017 02:18:12 +0000 (22:18 -0400)
committerDavid Wells <wellsd2@rpi.edu>
Wed, 16 Aug 2017 02:48:22 +0000 (22:48 -0400)
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.

source/base/timer.cc
tests/base/timer_06.cc [new file with mode: 0644]
tests/base/timer_06.output [new file with mode: 0644]

index 05745a8bad706f27b4abd77371ef5906e27cfd62..ab17fbcd9830a1629fd4b6a2523ed6dfa95b3961 100644 (file)
@@ -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 (file)
index 0000000..79e2e99
--- /dev/null
@@ -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 (file)
index 0000000..0fd8fc1
--- /dev/null
@@ -0,0 +1,2 @@
+
+DEAL::OK

In the beginning the Universe was created. This has made a lot of people very angry and has been widely regarded as a bad move.

Douglas Adams


Typeset in Trocchi and Trocchi Bold Sans Serif.