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

Closes #1420. Extend profiler width #1498

Merged
merged 2 commits into from
May 3, 2022

Conversation

mathomp4
Copy link
Member

@mathomp4 mathomp4 commented May 3, 2022

Description

This change extends the profiler width per #1420 for use with GCHP.

Related Issue

Closes #1420

Motivation and Context

GCHP runs with loooong wall times and this lets the profiler work with that.

How Has This Been Tested?

I built the change in GEOS and it didn't crash. Should be safe.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Trivial change (affects only documentation or cleanup)

Checklist:

  • I have tested this change with a run of GEOSgcm (if non-trivial)
  • I have added one of the required labels (0 diff, 0 diff trivial, 0 diff structural, non 0-diff)
  • I have updated the CHANGELOG.md accordingly following the style of Keep a Changelog

@mathomp4 mathomp4 added the 0 Diff Trivial The changes in this pull request are trivially zero-diff (documentation, build failure, &c.) label May 3, 2022
@mathomp4 mathomp4 self-assigned this May 3, 2022
@mathomp4
Copy link
Member Author

mathomp4 commented May 3, 2022

This seems to work. Before:

 Times for component <EXTDATA>
                                       Min                          Mean                         Max                    PE         # cycles
                            ==========================   ==========================   ==========================   =============   ========
Name                          %    inclusive exclusive     %    inclusive exclusive     %    inclusive exclusive    max    min
                          | ------ --------- --------- | ------ --------- --------- | ------ --------- --------- | ------ ------ |
EXTDATA                   |   0.00     22.17      0.00 |   0.00     22.19      0.00 |   0.00     22.21      0.00 |  00012  00094 |       99
--SetService              |   0.00      0.00      0.00 |   0.00      0.00      0.00 |   0.00      0.00      0.00 |  00001  00091 |        1

After:

 Times for component <EXTDATA>
                                        Min                            Mean                           Max                     PE         # cycles
                            ============================   ============================   ============================   =============   ========
Name                          %    inclusive  exclusive      %    inclusive  exclusive      %    inclusive  exclusive     max    min
                          | ------ ---------- ---------- | ------ ---------- ---------- | ------ ---------- ---------- | ------ ------ |
EXTDATA                   |   0.00      20.97       0.00 |   0.00      20.99       0.00 |   0.00      21.01       0.00 |  00069  00094 |       99
--SetService              |   0.00       0.00       0.00 |   0.00       0.00       0.00 |   0.00       0.00       0.00 |  00078  00091 |        1

I'll let @tclune take a look at this and see if he thinks I did it right.

@mathomp4 mathomp4 marked this pull request as ready for review May 3, 2022 19:21
@mathomp4 mathomp4 requested a review from a team as a code owner May 3, 2022 19:21
@mathomp4 mathomp4 requested a review from tclune May 3, 2022 19:21
Copy link
Collaborator

@tclune tclune left a comment

Choose a reason for hiding this comment

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

Good enough for now. We probably want a named parameter for the width so that these repeated expressions can be derived and then modified more easily in the future.

OTOH, the long term plan is to have a yaml config file for all this ...

@tclune tclune merged commit cf92368 into develop May 3, 2022
@tclune tclune deleted the feature/mathomp4/1420-extend-profiler-width branch May 3, 2022 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0 Diff Trivial The changes in this pull request are trivially zero-diff (documentation, build failure, &c.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extend format width for inclusive and exclusive times in profile reporting
2 participants