Added log entries for stage start/finish in MAPL.GENERIC #1009

LiamBindle · 2021-08-30T23:39:49Z

Description

This PR adds logging of the start/finish of gridcomp stages. For example

0000: MAPL.GENERIC: INFO: Started the  'SetServices' stage of the gridded component 'GCHP'
0000: MAPL.GENERIC: INFO: Started the  'SetServices' stage of the gridded component 'GCHPctmEnv'
0000: MAPL.GENERIC: INFO: Finished the 'SetServices' stage of the gridded component 'GCHPctmEnv'
0000: MAPL.GENERIC: INFO: Started the  'SetServices' stage of the gridded component 'GCHPchem'
0000: MAPL.GENERIC: INFO: Finished the 'SetServices' stage of the gridded component 'GCHPchem'
0000: MAPL.GENERIC: INFO: Started the  'SetServices' stage of the gridded component 'DYNAMICS'
0000: MAPL.GENERIC: INFO: Finished the 'SetServices' stage of the gridded component 'DYNAMICS'
0000: MAPL.GENERIC: INFO: Finished the 'SetServices' stage of the gridded component 'GCHP'
0000: MAPL.GENERIC: INFO: Started the  'SetServices' stage of the gridded component 'HIST'
0000: MAPL.GENERIC: INFO: Finished the 'SetServices' stage of the gridded component 'HIST'
0000: MAPL.GENERIC: INFO: Started the  'SetServices' stage of the gridded component 'EXTDATA'
0000: MAPL.GENERIC: INFO: Finished the 'SetServices' stage of the gridded component 'EXTDATA'
0000: MAPL.GENERIC: INFO: Started the  'Initialize' stage of the gridded component 'GCHP'
...

Note that one-time stages like SetServices, Initialize, and Finalize are going to MAPL.GENERIC info, and many-time stages like Run and WriteRestartFile go to debug.

Motivation and Context

I'm working on making it easier to troubleshoot errors in GCHP. Logging the start/end of each stage helps keep track of what is going on. If the model crashes for some reason, the log will show what stage of what component it happened in.

How Has This Been Tested?

I've tested it with GCHP.

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)

(I'm not too sure if this fits into trivial, but that seemed like the best fit to me)

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

LiamBindle · 2021-08-30T23:43:11Z

Still working on the CLA. The admins sent it to the lawyers today.

Im submitting this PR here and now because I'm working on the GCHP version of this PR (so that we can use it immediately).

LiamBindle · 2021-08-31T17:31:53Z

Are there any concerns with adding this? Just wondering because I'll move ahead with the GCHP version of it if it looks okay, while we wait for the lawyers.

tclune · 2021-09-01T12:46:25Z

I think it is fine in principle. I don't think you need the logical flag though. You can just set the description to the empty string and then overwrite in the cases that you've identified. And then always include the description in the log message. Eliminates the conditionals (big win) and a few other lines of code (small win).

LiamBindle · 2021-09-01T14:36:57Z

Thanks for the suggestion, I'll do that.

…t PE) re: GEOS-ESM/MAPL#1009

base/MAPL_Generic.F90

tclune · 2021-09-01T17:24:42Z

base/MAPL_Generic.F90

@@ -1813,7 +1810,6 @@ subroutine MAPL_GenericWrapper ( GC, IMPORT, EXPORT, CLOCK, RC)
 !     timers => timers_writereastart
     NULLIFY(timers)
     sbrtn = 'WriteRestart'
-     stage_is_one_time = .false.
  endif MethodBlock
  stage_description = ''''//trim(sbrtn)//''' stage of the gridded component '''//trim(COMP_NAME)//''''


Cool. I did not know that triple/quad quotes would do this.

tclune · 2021-09-01T17:25:03Z

base/MAPL_Generic.F90

-  else 
-    call lgr%debug('Started the  %a', trim(stage_description))
-  end if
+  call lgr%info('Started the  %a', trim(stage_description))


Suggested change

call lgr%info('Started the %a', trim(stage_description))

call lgr%info('Started the %a', stage_description)

tclune · 2021-09-01T17:25:19Z

base/MAPL_Generic.F90

-  else 
-    call lgr%debug('Finished the %a', trim(stage_description))
-  end if
+  call lgr%info('Finished the %a', trim(stage_description))


Suggested change

call lgr%info('Finished the %a', trim(stage_description))

call lgr%info('Finished the %a', stage_description)

mathomp4 · 2021-09-13T18:08:43Z

@LiamBindle A big MAPL Generic refactoring just went into develop. As GitHub is informing you, conflicts have happened, though hopefully it's pretty simple to figure out how to transition. If not, @Gvilla1000-nasa can help as he did the refactoring.

…t PE) re: GEOS-ESM/MAPL#1009

stale · 2021-11-12T18:46:05Z

This issue has been automatically marked as stale because it has not had recent activity. If there are no updates within 7 days, it will be closed. You can add the "long term" tag to prevent the Stale bot from closing this issue.

LiamBindle · 2021-11-12T20:08:06Z

bump

As of December 1st I'll technically be a private consulatant (still working full-time on GCHP though), so I should be able to sign the CLA then.

tclune · 2021-11-12T20:10:03Z

The lengths some people will go to just to avoid lawyers. :-)

stale · 2022-01-11T20:54:31Z

This issue has been automatically marked as stale because it has not had recent activity. If there are no updates within 7 days, it will be closed. You can add the "long term" tag to prevent the Stale bot from closing this issue.

mathomp4 · 2022-01-12T17:17:17Z

Un-staling. This is a good PR, just waiting on CLA bits.

LiamBindle · 2022-05-11T19:22:47Z

Similar to #1002, I don't plan on pursuing the CLA. I still think this is a useful capability (already it's helped troubleshoot multiple GCHP issues rapidly), but I think this is the end of the line of this PR.

mathomp4 · 2022-05-17T11:13:09Z

Closing in favor of #1510

LiamBindle added 2 commits August 30, 2021 22:52

Added log entries for stage start/finish in MAPL.GENERIC

9790bc5

Updated CHANGELOG.md

ea317d6

LiamBindle requested a review from a team as a code owner August 30, 2021 23:39

mathomp4 added the 🚫 Contingent - DNA Do Not Approve (DNA). These changes are contingent on other PRs label Aug 31, 2021

LiamBindle added 2 commits September 1, 2021 15:49

Removed stage_is_one_time conditional.

00ff65b

Added informative error message about what gridcomp and stage failed

645d6fe

LiamBindle added a commit to geoschem/geos-chem that referenced this pull request Sep 1, 2021

Changed default log level for MAPL to WARNING (all PEs) and info (roo…

388d1ad

…t PE) re: GEOS-ESM/MAPL#1009

LiamBindle added a commit to geoschem/geos-chem that referenced this pull request Sep 1, 2021

Changed default log level for MAPL to WARNING (all PEs) and info (roo…

c94f2bc

…t PE) re: GEOS-ESM/MAPL#1009

LiamBindle mentioned this pull request Sep 1, 2021

Added log entries for stage start/finish in MAPL.GENERIC geoschem/MAPL#15

Merged

tclune reviewed Sep 1, 2021

View reviewed changes

base/MAPL_Generic.F90 Show resolved Hide resolved

tclune reviewed Sep 1, 2021

View reviewed changes

tclune approved these changes Sep 1, 2021

View reviewed changes

tclune self-requested a review September 1, 2021 17:28

LiamBindle added a commit to geoschem/geos-chem that referenced this pull request Oct 14, 2021

Changed default log level for MAPL to WARNING (all PEs) and info (roo…

7db7d27

…t PE) re: GEOS-ESM/MAPL#1009

stale bot added the ❄️ Stale This issue has been marked stale label Nov 12, 2021

stale bot removed the ❄️ Stale This issue has been marked stale label Nov 12, 2021

stale bot added the ❄️ Stale This issue has been marked stale label Jan 11, 2022

stale bot removed the ❄️ Stale This issue has been marked stale label Jan 12, 2022

mathomp4 added the ⌛ Long Term Long term issues label Jan 12, 2022

mathomp4 closed this May 17, 2022

LiamBindle deleted the feature/log-stage-upstream branch September 7, 2022 18:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added log entries for stage start/finish in MAPL.GENERIC #1009

Added log entries for stage start/finish in MAPL.GENERIC #1009

LiamBindle commented Aug 30, 2021 •

edited

Loading

LiamBindle commented Aug 30, 2021

LiamBindle commented Aug 31, 2021

tclune commented Sep 1, 2021

LiamBindle commented Sep 1, 2021

tclune Sep 1, 2021

tclune Sep 1, 2021

tclune Sep 1, 2021

mathomp4 commented Sep 13, 2021

stale bot commented Nov 12, 2021

LiamBindle commented Nov 12, 2021

tclune commented Nov 12, 2021

stale bot commented Jan 11, 2022

mathomp4 commented Jan 12, 2022

LiamBindle commented May 11, 2022

mathomp4 commented May 17, 2022

	call lgr%info('Started the %a', trim(stage_description))
	call lgr%info('Started the %a', stage_description)

	call lgr%info('Finished the %a', trim(stage_description))
	call lgr%info('Finished the %a', stage_description)

Added log entries for stage start/finish in MAPL.GENERIC #1009

Added log entries for stage start/finish in MAPL.GENERIC #1009

Conversation

LiamBindle commented Aug 30, 2021 • edited Loading

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

LiamBindle commented Aug 30, 2021

LiamBindle commented Aug 31, 2021

tclune commented Sep 1, 2021

LiamBindle commented Sep 1, 2021

tclune Sep 1, 2021

Choose a reason for hiding this comment

tclune Sep 1, 2021

Choose a reason for hiding this comment

tclune Sep 1, 2021

Choose a reason for hiding this comment

mathomp4 commented Sep 13, 2021

stale bot commented Nov 12, 2021

LiamBindle commented Nov 12, 2021

tclune commented Nov 12, 2021

stale bot commented Jan 11, 2022

mathomp4 commented Jan 12, 2022

LiamBindle commented May 11, 2022

mathomp4 commented May 17, 2022

LiamBindle commented Aug 30, 2021 •

edited

Loading