MAPL changes to support CICE6 rewind for replay

[zhaobin74]

Hi,

Some code needs to be implemented to support rewinding CICE6 states when coupled model runs in replay mode.
Specifically, the following patch is required in subroutine MAPL_GenericRefresh.

       character(len=ESMF_MAXSTR)                  :: CHILD_NAME
       character(len=14)                           :: datestamp ! YYYYMMDD_HHMMz
       integer                                     :: status
+      integer                                     :: UserRC
       integer                                     :: I
       type (MAPL_MetaComp), pointer               :: STATE
       character(len=1)                            :: separator
@@ -2920,6 +2921,12 @@ contains
          ! call the actual record method
          call MAPL_StateRefresh (GC, IMPORT, EXPORT, CLOCK, RC=status )
          _VERIFY(status)
+
+         if (associated(STATE%phase_coldstart)) then
+            call ESMF_GridCompReadRestart(GC, importState=import, &
+                    exportState=export, clock=CLOCK, phase=MAPL_MAX_PHASES+1, userRC=userRC, 
_RC)
+         endif
+
       endif
       call MAPL_TimerOff(STATE,"GenRefreshMine",_RC)
       call MAPL_TimerOff(STATE,"GenRefreshTot",_RC)

The above code was tested in V11.4.0 and shown to be working.
Could SI group add the implementation in the next MAPL version? This is required by @Dooruk for his coupled DA work with CICE6.

Thanks

[tclune]

@zhaobin74 We can certainly do this, but it also fine if you want to do a PR yourself (onto the develop branch).

[Dooruk]

Has there been any progress on this? Currently I'm using @zhaobin74's local GEOS build but I might need to switch to a newer version of GEOSgcm in order to use another feature (DataAtm run).

[tclune]

I thought @atrayano added this to his plate before he departed for Bulgaria. I think we wanted to do a slight variation ...

I'll email him separately to get a status update. (Not sure if he links his personal email to GitHub.)

[Dooruk]

Thanks @tclune!

[mathomp4]

Yes, @atrayano said in #2982 (comment) that:

@zhaobin74 I have several concerns regarding this PR. The most trivial one is that instead of hardcoding MAPL_MAX_PHASES+1, we could introduce a local variable "phase", which we should get by calling ESMG_GridCompGet(gc, currentPhase=phase, _RC). A far more serious concern is the fact that we are using cold_start method which was supposed to be only as a fallback strategy during Initialize. Luckily for us, thus method is not used anywhere in GEOS (with a possible exception of the dycore), but if another component legitimately tries to use it, this PR has the potential to break that imaginary component. Let's touch base on Monday to brainstorm and find a better solution.

For the time being, I will add "do not approve" label.

My guess is things got busy quickly as they do for Atanas. :)

[tclune]

@atrayano has a fix and will pass it along for testing in ~ 1 day.

[zhaobin74]

I have got the fix from @atrayano . Will test it out using my setup for the PR.

@atrayano's fix does not work. The replay run is not restoring CICE6 thermo states during rewinding.

[Dooruk]

During a meeting yesterday @zhaobin74 mentioned @atrayano's rewind fix didn't work. I wanted to write it here again just in case his update in the comment didn't generate any notifications 👀

[tclune]

I am forwarding this to Atanas’ gmeail account in the hopes he might have time to address this yet this week. From: Doruk Ardağ ***@***.***> Date: Wednesday, October 30, 2024 at 10:50 AM To: GEOS-ESM/MAPL ***@***.***> Cc: Clune, Thomas L. (GSFC-6101) ***@***.***>, Mention ***@***.***> Subject: [EXTERNAL] [BULK] Re: [GEOS-ESM/MAPL] MAPL changes to support CICE6 rewind for replay (Issue #2959) CAUTION: This email originated from outside of NASA. Please take care when clicking links or opening attachments. Use the "Report Message" button to report suspicious messages to the NASA SOC. During a meeting yesterday @zhaobin74<https://github.com/zhaobin74> mentioned @atrayano<https://github.com/atrayano>'s rewind fix didn't work. I wanted to write it here again just in case his update in the comment didn't generate any notifications 👀 — Reply to this email directly, view it on GitHub<#2959 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABPP7YEG76TCOTFGLCLDWG3Z6DWZZAVCNFSM6AAAAABMMWMODWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINBXGQYDMMBYGQ>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[atrayano]

@zhaobin74 I had to make 1 change iCICE6::Restoren CICE_90: the registration method now requires slightly different syntax:

instead of call MAPL_GridCompSetEntryPoint ( GC, ESMF_METHOD_READRESTART, Refresh, _RC)
we need to call
call MAPL_GridCompSetEntryPoint ( GC, MAPL_METHOD_REFRESH, Refresh, _RC)

I
I re-tested my "rewind fix", and it seems to be working. Note that no changes are required on my MAPL branch.

Could you please, verify it.

And by the way, while debugging, I noticed another (totally unrelated) bug: inside the Refresh subroutine the string variable "Iam" grows (i.e. it went from CICE6::Restore to CICE6::CICE6::Restore to CICE6::CICE6::CICE6::Restore, etc). Eventually this will lead to buffer overflow

[zhaobin74]

@zhaobin74 I had to make 1 change iCICE6::Restoren CICE_90: the registration method now requires slightly different syntax:

instead of call MAPL_GridCompSetEntryPoint ( GC, ESMF_METHOD_READRESTART, Refresh, _RC) we need to call call MAPL_GridCompSetEntryPoint ( GC, MAPL_METHOD_REFRESH, Refresh, _RC)

I I re-tested my "rewind fix", and it seems to be working. Note that no changes are required on my MAPL branch.

Could you please, verify it.

And by the way, while debugging, I noticed another (totally unrelated) bug: inside the Refresh subroutine the string variable "Iam" grows (i.e. it went from CICE6::Restore to CICE6::CICE6::Restore to CICE6::CICE6::CICE6::Restore, etc). Eventually this will lead to buffer overflow

@atrayano, I can verify the MAPL fix works after changing to MAPL_METHOD_REFRESH.
I don't get why Iam keeps growing. It is a local string prefixed by comp_name. Anyhow, I removed the prefix so no overflows.

I'll make a PR in GEOSgcm_gridComp repo with these updates.

Thanks.

[tclune]

I have not looked at the specif module, but am guessing that (1) Iam is a module variable and thus has the SAVE attribute (implicitly) and (2) the procedure is doing something like Iam = Iam // ... and thus keeps appending.

[zhaobin74]

I have not looked at the specif module, but am guessing that (1) Iam is a module variable and thus has the SAVE attribute (implicitly) and (2) the procedure is doing something like Iam = Iam // ... and thus keeps appending.

There is no such variable with name IAm at module level in CICE_plug. IAm is from
__Iam__('Restore') which is a macro defined in MAPL. It is equivalent to integer :: STATUS; character (len=255) :: Iam = 'Restore'

[tclune]

OK - I had the details slightly wrong. It is a local variable declared with the Iam macro. But since the macro gives an initial value, the variable as the SAVE attribute. (implicitly). Then

https://github.com/GEOS-ESM/GEOSgcm_GridComp/blob/fd538d53a85fdedca503d8a0bc2dab08cb2cc760/GEOSogcm_GridComp/GEOSseaice_GridComp/CICE_GEOSPlug/CICE_GEOSPlug.F90#L1023

Causes it to grow each time the refresh() method is called.

[zhaobin74]

OK - I had the details slightly wrong. It is a local variable declared with the Iam macro. But since the macro gives an initial value, the variable as the SAVE attribute. (implicitly). Then

https://github.com/GEOS-ESM/GEOSgcm_GridComp/blob/fd538d53a85fdedca503d8a0bc2dab08cb2cc760/GEOSogcm_GridComp/GEOSseaice_GridComp/CICE_GEOSPlug/CICE_GEOSPlug.F90#L1023

Causes it to grow each time the refresh() method is called.

Implicit SAVE when initialized? What a feature! Now I know how to fix it. Thanks, @tclune

[tclune]

Yes - this is an ancient rule inFortran. It really throws off the C programmers, but also catches us Fortran old timers occasionally.