From: David Wells 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=refs%2Fpull%2F4868%2Fhead;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 + +#include +#include + +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(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