Add a Hierarchical timer class

[michaelbynum]

Summary/Motivation:

This Pr adds a class for hierarchical timing.

Legal Acknowledgement

By contributing to this software project, I agree to the following terms and conditions for my contribution:

1. I agree my contributions are submitted under the BSD license.
2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

[coveralls]

Coverage increased (+0.8%) to 61.847% when pulling 2feaa83b4afb80586d5e94b4e0deb4a7388a76d6 on michaelbynum:hierarchical_timer into ad6043c52aff125d135413f36e0db39dcd54bf83 on PyUtilib:master.

[codecov-io]

Codecov Report

Merging #96 into master will increase coverage by 0.79%. The diff coverage is 93.75%.

@@            Coverage Diff             @@
##           master      #96      +/-   ##
==========================================
+ Coverage   63.13%   63.93%   +0.79%     
==========================================
  Files          87       87              
  Lines        8788     8916     +128     
==========================================
+ Hits         5548     5700     +152     
+ Misses       3240     3216      -24     

Impacted Files	Coverage Δ
pyutilib/misc/timing.py	76.76% <93.75%> (+76.76%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ad6043c...913df89. Read the comment docs.

[michaelbynum]

@jsiirola Great suggestions. I never realized the TicTocTimer tracked cumulative time and number of calls. I also did not know about time.perf_counter or time.clock, so thanks. Let me know what you think of the new changes.

[michaelbynum]

@jsiirola I think this is ready now. Thanks for all the great suggestions. I am very happy with this timer now.

[jsiirola]

@michaelbynum: some questions/comments as I go through this:

• Why scientific notation for timing? profile uses '%.3f', and I think that is a good precedent: timings under a millisecond are meaningless, and I (at least) get lost tracking exponents
  • I would like to propose the table be in '%8d %8.3f %8.3f %5.1f': this makes things more compact, yet ensures in the odd chance that timings run over 10000 seconds that there is still a space between numbers.
  • (This is a nit): why use .format()? It is verbose, harder to parse, and generally 2x slower that classic formatting (using '%'). We can move to f-strings when we finally drop 2.x and 3.5 (f-strings are faster than '%' and more concise, but not available until 3.6)
  • Is there a reason you did not have _HierarchicalHelper directly inherit from TicTocTimer? If you did then you would only have to add the timers dict and the printing function.
  • Similarly, I feel like the HierarchicalTimer should itself be a _HierarchicalHelper object, that just overwrites the start / stop methods to take an optional argument. Maybe everything could be implemented in a single class? That way you could get reports from anywhere in the hierarchy.
  • Related to the above, I feel like the info methods (like get_total_time and get relative_precent_time could be calculated locally within the Helper. This actually motivated the previous comment, as I was working through the complexity of the printer (the logic is basically duplicated in both the HierarchicalTimer and in the _HierarchicalHelper
  • You should not directly access the _cumul attribute, as it will return the wrong answer if the timer is currently running. Always access it with toc(msg='')

I have some ideas around addressing the above. Would you like me to draft something as a PR to your branch?

[michaelbynum]

@jsiirola

• I agreed with most of your formatting comments, so I made some updates. I used 12 and 10 rather than 8 and 5 (8 was to short for the titles).
• I disagree about format - I think it is clear, not verbose. It is my personal preference. Also, I would argue that the performance of printing a HierarchicalTimer is not terribly important.
• You seem to be favoring inheritance over containment. In this situation, I do not really see the benefit of one over the other, but I am not opposed to inheritance if you want to make some changes an create a PR.
• The only problem with putting get_relative_percent_time in the helper object is that the helper object does not currently have a pointer to its parent. Again, I am fine with these changes if you want to make a PR.
• I fixed the issue with accessing _cumul.

[michaelbynum]

@jsiirola Looks good. Thanks for these changes.

[carldlaird]

I agree. Thank you both for this - this looks great - and will definitely get used in a few of our projects.