Fixes to Timer logic

[garth-wells]

The logic behind the interaction between Timer and TimeLogger was messed up. This attempts to fix it without interface changes.

Improves and fixes tests.

[jorgensd]

Shouldn't "stop" imply a "flush" if we want to maintain the timing API?

There isn't a way to "pause" a timer, as "start" resets the accumulated time. Thus stop should imply flush.

[garth-wells]

Shouldn't "stop" imply a "flush" if we want to maintain the timing API?

The logic has always been that durations are logged with the Timer is destroyed. flush forces this to happen earlier.

There isn't a way to "pause" a timer, as "start" resets the accumulated time. Thus stop should imply flush.

There was a resume - @schnellerhase removed it because it was untested. I could now be added back (I think it should).

[jorgensd]

Shouldn't "stop" imply a "flush" if we want to maintain the timing API?

The logic has always been that durations are logged with the Timer is destroyed. flush forces this to happen earlier.

That is a bit tedious on the python side, as a user would manually have to delete a timer if it is not made with a context manager, or call flush explicitly. This assumption has at least never been documented on the Python-side of things.

I am ok with forcing a flush on the python side, but then that has to be documented as a prerequisite in the C++ and Python documentation of the timing function.

While looking through this, I also observe that the docs are wrong in C++:
https://github.com/FEniCS/dolfinx/blob/main/cpp/dolfinx/common/timing.h#L30-L34
as it still talks about a timing triplet.

[garth-wells]

The Python wrappers could call stop and flush.

[schnellerhase]

There was a resume - @schnellerhase removed it because it was untested. I could now be added back (I think it should).

This also allowed for one less state to manage in the Timer and thus simpler logic. I think keeping the states to a minimum is best here.

What do we need the resume/restart functionality for? Together with the TimeLogger I think we can do without.

[jorgensd]

There was a resume - @schnellerhase removed it because it was untested. I could now be added back (I think it should).

This also allowed for one less state to manage in the Timer and thus simpler logic. I think keeping the states to a minimum is best here.

What do we need the resume/restart functionality for? Together with the TimeLogger I think we can do without.

If we use the TimeLogger for resuming timings, then stop should definitely flush, as there is no clear benefit of keeping the object around then?

[garth-wells]

There was a resume - @schnellerhase removed it because it was untested. I could now be added back (I think it should).

This also allowed for one less state to manage in the Timer and thus simpler logic. I think keeping the states to a minimum is best here.
What do we need the resume/restart functionality for? Together with the TimeLogger I think we can do without.

If we use the TimeLogger for resuming timings, then stop should definitely flush, as there is no clear benefit of keeping the object around then?

There are users who use stop/resume. It wasn't covered by tests (it is now), but that doesn't mean it's not used.

Timings should only be logged to when finished (by the destructor, or when manually flushed), otherwise the logger counter get messed up.