From ad40e35b13422fd523849162a7e16404bda27274 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Paul=20T=2E=20K=C3=BChner?=
 <56360279+schnellerhase@users.noreply.github.com>
Date: Wed, 11 Sep 2024 09:40:53 +0200
Subject: [PATCH] `Timer` interface improvement (#3328)

* Adapt Timer

* Remove space

* Switch to member variable being optional

* clang-format

* Fix python export

* Remove move

* Remove none from python export

* Re add nb::none

* Update python/dolfinx/wrappers/common.cpp

---------

Co-authored-by: Garth N. Wells <gnw20@cam.ac.uk>
---
 cpp/dolfinx/common/Timer.cpp       | 17 +++++------------
 cpp/dolfinx/common/Timer.h         | 13 +++++++------
 python/dolfinx/common.py           |  5 +----
 python/dolfinx/wrappers/common.cpp | 10 +++++-----
 4 files changed, 18 insertions(+), 27 deletions(-)

diff --git a/cpp/dolfinx/common/Timer.cpp b/cpp/dolfinx/common/Timer.cpp
index 3b2e9d53c4e..7b3db92c94e 100644
--- a/cpp/dolfinx/common/Timer.cpp
+++ b/cpp/dolfinx/common/Timer.cpp
@@ -7,21 +7,14 @@
 #include "Timer.h"
 #include "TimeLogManager.h"
 #include "TimeLogger.h"
+#include <optional>
 #include <stdexcept>
 
 using namespace dolfinx;
 using namespace dolfinx::common;
 
 //-----------------------------------------------------------------------------
-Timer::Timer() : Timer::Timer("")
-{
-  // Do nothing
-}
-//-----------------------------------------------------------------------------
-Timer::Timer(const std::string& task) : _task(task)
-{
-  // Do nothing
-}
+Timer::Timer(std::optional<std::string> task) : _task(task) {}
 //-----------------------------------------------------------------------------
 Timer::~Timer()
 {
@@ -33,7 +26,7 @@ void Timer::start() { _timer.start(); }
 //-----------------------------------------------------------------------------
 void Timer::resume()
 {
-  if (!_task.empty())
+  if (_task.has_value())
   {
     throw std::runtime_error(
         "Resuming is not well-defined for logging timer. Only "
@@ -46,8 +39,8 @@ double Timer::stop()
 {
   _timer.stop();
   const auto [wall, user, system] = this->elapsed();
-  if (!_task.empty())
-    TimeLogManager::logger().register_timing(_task, wall, user, system);
+  if (_task.has_value())
+    TimeLogManager::logger().register_timing(_task.value(), wall, user, system);
   return wall;
 }
 //-----------------------------------------------------------------------------
diff --git a/cpp/dolfinx/common/Timer.h b/cpp/dolfinx/common/Timer.h
index db1d44d690c..8b966f2b8cb 100644
--- a/cpp/dolfinx/common/Timer.h
+++ b/cpp/dolfinx/common/Timer.h
@@ -8,6 +8,7 @@
 
 #include <array>
 #include <boost/timer/timer.hpp>
+#include <optional>
 #include <string>
 
 namespace dolfinx::common
@@ -30,11 +31,11 @@ namespace dolfinx::common
 class Timer
 {
 public:
-  /// Create timer without logging
-  Timer();
-
-  /// Create timer with logging
-  Timer(const std::string& task);
+  /// Create timer
+  ///
+  /// If a task name is provided this enables logging to logger, otherwise (i.e.
+  /// no task provided) nothing gets logged.
+  Timer(std::optional<std::string> task = std::nullopt);
 
   /// Destructor
   ~Timer();
@@ -54,7 +55,7 @@ class Timer
 
 private:
   // Name of task
-  std::string _task;
+  std::optional<std::string> _task;
 
   // Implementation of timer
   boost::timer::cpu_timer _timer;
diff --git a/python/dolfinx/common.py b/python/dolfinx/common.py
index be70ce239ab..3fd71f1e2fc 100644
--- a/python/dolfinx/common.py
+++ b/python/dolfinx/common.py
@@ -84,10 +84,7 @@ class Timer:
     _cpp_object: _cpp.common.Timer
 
     def __init__(self, name: typing.Optional[str] = None):
-        if name is None:
-            self._cpp_object = _cpp.common.Timer()
-        else:
-            self._cpp_object = _cpp.common.Timer(name)
+        self._cpp_object = _cpp.common.Timer(name)
 
     def __enter__(self):
         self._cpp_object.start()
diff --git a/python/dolfinx/wrappers/common.cpp b/python/dolfinx/wrappers/common.cpp
index 4c237fe190e..6844ad719cb 100644
--- a/python/dolfinx/wrappers/common.cpp
+++ b/python/dolfinx/wrappers/common.cpp
@@ -6,6 +6,7 @@
 
 #include <complex>
 #include <memory>
+#include <optional>
 #include <span>
 #include <string>
 #include <vector>
@@ -13,18 +14,18 @@
 #include <nanobind/nanobind.h>
 #include <nanobind/ndarray.h>
 #include <nanobind/stl/array.h>
+#include <nanobind/stl/optional.h>
 #include <nanobind/stl/pair.h>
 #include <nanobind/stl/string.h>
 #include <nanobind/stl/tuple.h>
 #include <nanobind/stl/vector.h>
 
-#include <dolfinx/common/defines.h>
-#include <dolfinx/common/defines.h>
 #include <dolfinx/common/IndexMap.h>
-#include <dolfinx/common/log.h>
 #include <dolfinx/common/Scatterer.h>
 #include <dolfinx/common/Table.h>
 #include <dolfinx/common/Timer.h>
+#include <dolfinx/common/defines.h>
+#include <dolfinx/common/log.h>
 #include <dolfinx/common/timing.h>
 #include <dolfinx/common/utils.h>
 
@@ -152,8 +153,7 @@ void common(nb::module_& m)
           nb::arg("global"));
   // dolfinx::common::Timer
   nb::class_<dolfinx::common::Timer>(m, "Timer", "Timer class")
-      .def(nb::init<>())
-      .def(nb::init<std::string>(), nb::arg("task"))
+      .def(nb::init<std::optional<std::string>>(), nb::arg("task").none())
       .def("start", &dolfinx::common::Timer::start, "Start timer")
       .def("stop", &dolfinx::common::Timer::stop, "Stop timer")
       .def("resume", &dolfinx::common::Timer::resume)