Changes to allow GCHP to run in reverse mode for adjoint integration #1166
Conversation
This commit adds code to allow the CapGridComp to run in reverse while maintaining the ability to trigger history alarms. This may also require a small bugfix in the ESMF 8.0 code.
GMAO doesn't like preprocessor defines so I removed most of them but had forgotten to do the step_reverse function. This change fixes that.
I had made some changes in previous tests to get the reverse model running but forgot to remove these two.
|
@TerribleNews Thanks for submitting the PR. While many of your changes are very well protected and won't impact GEOS at all, some would require additional logic. My main concern are the changes in History, and in particular the usage of the existing variable "list(n)%backwards". GEOS also has an adjoint model, and this variable might be used there. My preference would be to leave the existing "backwards: logic intact, and add a new variable, for example "reverse_time". I have many more comments regarding style, for example, the usage of debugging prints, etc., but I will articulate them in separate comments |
|
Thanks for the feedback @atrayano. I am not thrilled with the changes I made to the history alarms in general; they were mostly trial and error, and possibly not all of them are necessary / useful. For example, when there's a new alarm declared, if it's backwards, it's declared as sticky, but I couldn't get my alarms to ring without
|
|
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
added
⌛ Long Term
|
Huh. I didn't know the stale bot could mark PRs as stale. I think I need to look at how it's configured! |
|
@bena-nasa Something to think about with ESMF New Alarm: Running in reverse for Adjoint |
| read(timestring( 1: 4),'(i4.4)') year | ||
| read(timestring( 6: 7),'(i2.2)') month | ||
| read(timestring( 9:10),'(i2.2)') day | ||
| read(timestring(12:13),'(i2.2)') hour | ||
| read(timestring(15:16),'(i2.2)') minute |
There was a problem hiding this comment.
Aren't there better ways to get these directly from ESMF_Time?
| if (MAPL_am_I_Root()) THEN | ||
| WRITE(*, 1999) n | ||
| ENDIF | ||
| 1999 FORMAT('Not Writing alarm ', i3, ' because end_alarm is ringing') |
There was a problem hiding this comment.
Prefer to avoid FORMAT statements:
| if (MAPL_am_I_Root()) THEN | |
| WRITE(*, 1999) n | |
| ENDIF | |
| 1999 FORMAT('Not Writing alarm ', i3, ' because end_alarm is ringing') | |
| if (MAPL_am_I_Root()) THEN | |
| WRITE(*, '("Not Writing arlarm ", i3, " because end alarm is ringing.") n | |
| ENDIF |
There was a problem hiding this comment.
Suggested one change and would like clarification from @bena-nasa on how to retrieve min/hr/... from an ESMF_Time.
I also note that the changes to CapGridComp are likely fragile under planned MAPL3 changes, and thus may require re-implementation down the road.
|
Did a quick attempt to resolve conflicts. May easily have done damage. |
tclune
added
the
0 Diff
|
Closing here. Replaced by #1795 . |
Description
This change adds a time loop to CapGridComp run the clock forward before the time loop that actually executes the grid components. There are also some small changes to handle issues with alarms not ringing properly in reverse.
Related Issue
#1144
Motivation and Context
This feature is to support to adjoint development in GCHP, which requires running the model in reverse. There may be other advantages to a generic capability to run MAPL in reverse as well.
This will be my first merge request to the team, so I am submitting mostly to start the conversation and get guidance / feedback.
How Has This Been Tested?
Unfortunately, because GCHP uses a fork of MAPL it is non-trivial to test code changes with the latest version. All these changes are patterned after changes that have been tested in GCHP using the verison of MAPL that is there, which is based on 2.6.3. Is there a way for me to test these exact changes?
Types of changes
Checklist: