Introduce Timer context manager #995
Conversation
Originally tried to make it do logging, too, but I couldn't figure out how to get it to log in the correct scope. Separating timing from logging substantially simplifies the logic at a modest cost in LoC at point of use.
There was a problem hiding this comment.
The code LGTM. The output looks like (with input blocks escaped):
$ export FIPY_LOG_CONFIG=fipy/tools/logging/serial_config.json
$ python3 simple.py
Solver suite is scipy
{'python': '3.11.5 (main, Sep 11 2023, 13:54:46) [GCC 11.2.0]', 'fipy': '3.4.4+10.g626e154ca', 'numpy': '1.26.3', 'pysparse': 'not installed', 'scipy': '1.11.4', 'matplotlib': '3.8.2', 'mpi4py': 'not installed', 'petsc4py': 'not installed', 'pyamgx': 'not installed', 'PyTrilinos': 'not installed', 'mayavi': 'not installed', 'gmsh': '4.12.0', 'solver': 'scipy'}
False
True
True
True
True
True
True
$ grep -E "[[:digit:]]+[[:space:]]ns" fipy.log
2024-01-19 16:35:15,377 - DEBUG - fipy.terms.binaryTerm._BinaryTerm - _prepareLinearSystem - END _prepareLinearSystem - 7706122 ns
2024-01-19 16:35:15,377 - DEBUG - fipy.solvers.scipy.linearGMRESSolver.LinearGMRESSolver - _solve_ - END precondition - 512 ns
2024-01-19 16:35:15,377 - DEBUG - fipy.solvers.scipy.linearGMRESSolver.LinearGMRESSolver - _solve_ - END solve - 64355 ns
2024-01-19 16:35:15,377 - DEBUG - fipy.terms.binaryTerm._BinaryTerm - solve - END solve - 7903306 ns
2024-01-19 16:35:15,385 - DEBUG - fipy.terms.binaryTerm._BinaryTerm - _prepareLinearSystem - END _prepareLinearSystem - 7594736 ns
2024-01-19 16:35:15,385 - DEBUG - fipy.solvers.scipy.linearLUSolver.LinearLUSolver - _solve_ - END precondition - 33168 ns
2024-01-19 16:35:15,386 - DEBUG - fipy.solvers.scipy.linearLUSolver.LinearLUSolver - _solve_ - END solve - 299729 ns
2024-01-19 16:35:15,386 - DEBUG - fipy.terms.binaryTerm._BinaryTerm - solve - END solve - 8239543 ns|
LGTM, I get similar to @tkphd. I see that the Timer context is being repeated in many different places. Suggests that the context should be in a single place and then what's inside the context should be what is unique to each subclass. |
I see your point. The thing is that what's inside the context is highly dependent on the broader context of |
Originally tried to make
Timer()do logging, too, but I couldn't figure out how to get it to log in the correct scope.Separating timing from logging substantially simplifies the logic at a modest cost in LoC at point of use.