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

Generate report for Timers #1882

Merged
merged 14 commits into from
Jan 17, 2020
Merged

Conversation

ZedThree
Copy link
Member

@ZedThree ZedThree commented Jan 9, 2020

Adds two methods to Timer:

  • getTotalTime to see the total time elapsed for a timer (i.e. doesn't get reset with resetTime)
  • listAllInfo generates a table reporting all the timers, like:
Timer name | Current time (s) | Total time (s) | Hits
-----------|------------------|----------------|-----
comms      | 0                | 0.028078329    | 546 
evolve_rho | 0.499829339      | 0.499829339    | 62  
invert     | 0                | 0.539184925    | 62  
io         | 0.010658816      | 0.090785536    | 57  
rhs        | 0                | 4.15958722     | 62  
run        | 0.014304425      | 4.32613615     | 1   

Adds an option write_final_timings which prints the above table at the end of a simulation.

I'm not so sure the "current time" column is particularly useful, probably a "mean time per hit" would be better. Also I'm not overly keen on the name write_final_timings at global scope, so suggestions welcome!

d7919 and others added 10 commits October 22, 2019 16:57
this.

Just convenience methods that can be useful for rough profiling etc.
* next: (243 commits)
  Ignore file copied by configure
  Fix fmt include path not getting expanded correctly
  Add option to use external fmt instead of bundled version
  Install fmt from subdirectory correctly
  Fix Chinese translated strings
  Fixes for locale DE
  Hardcode flags for non-PETSc Laplace in test
  Replace deprecated global flags with new flags for preconditioners
  Fix PETSc Laplace MAST grid test
  Add header file containing the wrapper
  Ran clang-format and added mpi_wrapper.hxx to CMakeLists.txt
  Ensured MpiWrapper initialised before mesh is created
  Add breaking change entry to changelog
  Add redundant out-of-line definitions for static Factory members
  Use fmt's positional arguments to avoid repeating variable
  Apply fmt fixes to i18n
  Apply fmt fixes to remaining tests and examples
  Use fmt for other printf formatting in bout++.cxx
  Use fmt to format timestamp
  Fix uses of output to use fmt formatting
  ...
@ZedThree ZedThree added the backport candidiate Does not break backward compatibility, so can be back-ported to v4.4 label Jan 9, 2020
@ZedThree ZedThree added this to the BOUT-5.0 milestone Jan 9, 2020
Also:
- remove the "current time" column
- add comments
@ZedThree
Copy link
Member Author

ZedThree commented Jan 9, 2020

I've changed the columns to drop the current time and add mean time per hit:

Timer name | Total time (s) | Hits | Mean time/hit (s)
-----------|----------------|------|------------------
comms      | 0.019139448    | 546  | 3.50539341e-05   
evolve_rho | 0.357001864    | 62   | 0.00575809458    
invert     | 0.376253795    | 62   | 0.0060686096     
io         | 0.055955419    | 57   | 0.000981674018   
rhs        | 2.97283263     | 62   | 0.0479489133     
run        | 3.07722771     | 1    | 3.07722771   

Maybe the switch should be show_timing_report?

@bendudson
Copy link
Contributor

Would sorting by one of the columns be simple to do? e.g. sort by total time might be the most useful most of the time

@ZedThree
Copy link
Member Author

I've changed the option to time_report:show, sticking it in its own section for a couple of reasons: I'd prefer not to add stuff to the global section if possible, and to not preclude future customisation options.

Also added a column with fraction of the largest time. Now looks like:

Timer name | Total time (s) | % of top | Hits | Mean time/hit (s)
-----------|----------------|----------|------|------------------
run        | 4.37208176     | 100.00%  | 1    | 4.37208176       
rhs        | 4.18987402     | 95.83%   | 62   | 0.0675786133     
invert     | 0.543508674    | 12.43%   | 62   | 0.00876626894    
evolve_rho | 0.504323351    | 11.54%   | 62   | 0.0081342476     
io         | 0.101402551    | 2.32%    | 57   | 0.00177899212    
comms      | 0.029098885    | 0.67%    | 546  | 5.32946612e-05   

Copy link
Member

@d7919 d7919 left a comment

Choose a reason for hiding this comment

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

Approved, although I wrote some of this.

@bendudson bendudson merged commit 125278f into next Jan 17, 2020
@bendudson bendudson deleted the minor_feature_get_timers_and_report_timers branch January 17, 2020 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport candidiate Does not break backward compatibility, so can be back-ported to v4.4
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants