Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add utility methods to HierarchicalTimer #2651

Merged
merged 14 commits into from
Dec 9, 2022
Merged

Conversation

Robbybp
Copy link
Contributor

@Robbybp Robbybp commented Dec 3, 2022

Fixes most of #2503

Summary/Motivation:

I am using HierarchicalTimer to collect fairly detailed timing information in ExternalPyomoModel, and when comparing different configurations, I sometimes want to highlight different timing categories. I want to do this by (a) removing redundant information from the timer and (b) flattening the timer so that categories I care about are easy to compare.

For example, I would like to convert the first timer shown into the second timer shown for the purpose of comparing the amount of time spent in NLP Newton solvers.

Identifier                                        ncalls   cumtime   percall      %
-----------------------------------------------------------------------------------
root                                                   1     8.908     8.908  100.0
     ------------------------------------------------------------------------------
     __init__                                         10     1.930     0.193   21.7
               --------------------------------------------------------------------
               PyomoNLP                               10     0.277     0.028   14.4
               __init__                               10     1.467     0.147   76.0
                             ------------------------------------------------------
                             NlpSolver                10     0.027     0.003    1.9
                             PyomoNLP                 10     0.181     0.018   12.3
                             partition                10     0.415     0.042   28.3
                             other                   n/a     0.843       n/a   57.5
                             ======================================================
               other                                 n/a     0.187       n/a    9.7
               ====================================================================
     hessian                                          90     1.303     0.014   14.6
     jacobian                                        100     0.568     0.006    6.4
     set_inputs                                      120     4.549     0.038   51.1
               --------------------------------------------------------------------
               set_parameters                        120     4.522     0.038   99.4
                             ------------------------------------------------------
                             solve                 44520     4.256     0.000   94.1
                                      ---------------------------------------------
                                      calc_var     42120     2.293     0.000   53.9
                                      solve_nlp     2400     1.886     0.001   44.3
                                      other          n/a     0.077       n/a    1.8
                                      =============================================
                             other                   n/a     0.267       n/a    5.9
                             ======================================================
               other                                 n/a     0.027       n/a    0.6
               ====================================================================
     other                                           n/a     0.557       n/a    6.3
     ==============================================================================
===================================================================================

Identifier                 ncalls   cumtime   percall      %
------------------------------------------------------------
root                            1     8.908     8.908  100.0
     -------------------------------------------------------
     __init__                  10     1.930     0.193   21.7
     hessian                   90     1.303     0.014   14.6
     jacobian                 100     0.568     0.006    6.4
     set_inputs               120     4.549     0.038   51.1
               ---------------------------------------------
               solve_nlp     2400     1.886     0.001   41.5
               other          n/a     2.663       n/a   58.5
               =============================================
     other                    n/a     0.557       n/a    6.3
     =======================================================
============================================================

With the changes proposed in this PR, I can do this with the following few lines:

root = timer.timers["root"]
# Clear __init__ to get rid of some visual cruft
root.timers["__init__"].timers.clear()
# Clear solve_nlp to make it a leaf node so that it will retain all of its
# total_time when we flatten
set_params = root.timers["set_inputs"].timers["set_parameters"]
set_params.timers["solve"].timers["solve_nlp"].timers.clear()
# Flatten
root.timers["set_inputs"].flatten()
# Clear everything under set_inputs except solve_nlp, which we want to compare
root.timers["set_inputs"].clear_except("solve_nlp")

Changes proposed in this PR:

  • Add the flatten method to HierarchicalTimer. This recursively flattens the "child timers" into a single layer. If any timers in this layer have the same identifier, their total_time and n_calls fields are combined.
  • Add the clear_except method to HierarchicalTimer. This pops timers from the dict of "child timers" except those whose identifier match one of the identifiers provided as arguments
  • Add an example of this to the HierarchicalTimer documentation

Legal Acknowledgement

By contributing to this software project, I have read the contribution guide and 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.

@codecov
Copy link

codecov bot commented Dec 6, 2022

Codecov Report

Base: 87.03% // Head: 87.03% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (2b4c569) compared to base (da48977).
Patch coverage: 97.22% of modified lines in pull request are covered.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2651   +/-   ##
=======================================
  Coverage   87.03%   87.03%           
=======================================
  Files         754      754           
  Lines       84051    84087   +36     
=======================================
+ Hits        73151    73185   +34     
- Misses      10900    10902    +2     
Flag Coverage Δ
linux 84.45% <97.22%> (+<0.01%) ⬆️
osx 74.85% <97.22%> (+<0.01%) ⬆️
other 84.63% <97.22%> (+<0.01%) ⬆️
win 81.80% <97.22%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pyomo/common/timing.py 92.18% <97.22%> (+0.54%) ⬆️
pyomo/core/expr/visitor.py 93.24% <0.00%> (-0.16%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@mrmundt mrmundt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a neat addition to the timing utility.

@Robbybp
Copy link
Contributor Author

Robbybp commented Dec 6, 2022

With the changes proposed in this PR, I can do this with the following few lines:

root = timer.timers["root"]
# Clear __init__ to get rid of some visual cruft
root.timers["__init__"].timers.clear()
# Clear solve_nlp to make it a leaf node so that it will retain all of its
# total_time when we flatten
set_params = root.timers["set_inputs"].timers["set_parameters"]
set_params.timers["solve"].timers["solve_nlp"].timers.clear()
# Flatten
root.timers["set_inputs"].flatten()
# Clear everything under set_inputs except solve_nlp, which we want to compare
root.timers["set_inputs"].clear_except("solve_nlp")

If we had a get_timer_from_string method, I could potentially do this with some slightly more simple code:

# Clear __init__
timer.get_timer_from_string("root.__init__").clear_except()
# Clear solve_nlp
timer.get_timer_from_string("root.set_inputs.set_parameters.solve.solve_nlp").clear_except()
# Flatten
timer.get_timer_from_string("root.set_inputs").flatten()
# Clear set_inputs except for solve_nlp
timer.get_timer_from_string("root.set_inputs").clear_except("solve_nlp")

This would require some naming convention where the "base name" of a timer is required not to have a period in it, however.

@mrmundt mrmundt merged commit 62d8b26 into Pyomo:main Dec 9, 2022
@Robbybp Robbybp deleted the htimer-utils branch December 9, 2022 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants