-
Notifications
You must be signed in to change notification settings - Fork 10
implement flexible moist grid comp DT #527
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
Conversation
|
I am not sure what is this alarm for. Maybe we should consider how to combine these two alarms. @atrayano GEOSgcm_GridComp/GEOSagcm_GridComp/GEOSphysics_GridComp/GEOSmoist_GridComp/GEOS_MoistGridComp.F90 Lines 5422 to 5436 in fb1d28f
|
|
@weiyuan-jiang My intention is noty to modify MAPL's original alarm (and we cannot use the MAPL's alarm since it ring at the last time step of run_dt). Convention in GEOS is that every componet in the system is called every time step during the main time loop. The new alarm will be used to decide whether to run or to skip calculations. It is almost identical to the alarm in MAPL except for the offset (RingTime). I cannot see how we could combine the two alarms |
|
@atrayano It is strange that this PR crashes when I set MOIST_DT as double of HEARTBEAT. I am debugging... forrtl: severe (174): SIGSEGV, segmentation fault occurred |
|
@weiyuan-jiang When you create the new alarm (iMoistAlarm n Initialize), you need to explicitly set it to ringing. Although your ringTime is the same as currTime, the state of alarm will be changed only during the call ESMF_ClockAdvance (in Cap). So, just add this call as soon as the alarm is created |
Even without above call, the moist run at the first time. After I added, there is no difference and it still crashed at the same point. |
|
@atrayano I know why it crashes. When I set MOIST_DT = HEARTBEAT, it runs and the old alarm rings. If I set MOIST_DT = 2*HEARTBEAT, at the first time, the old alarm doesn't ring. So there is interference of the MoistAlarm and Alarm . GEOSgcm_GridComp/GEOSagcm_GridComp/GEOSphysics_GridComp/GEOSmoist_GridComp/GEOS_MoistGridComp.F90 Lines 5436 to 5444 in fb1d28f
|
|
After talking to @atrayano , this PR now runs as expected. ( runs at time 0 and every Moist_DT). But I am still confused why the new MoistAlarm affects the ringing time of the old ALARM in 5414 ( commented out now) @bena-nasa @tclune GEOSgcm_GridComp/GEOSagcm_GridComp/GEOSphysics_GridComp/GEOSmoist_GridComp/GEOS_MoistGridComp.F90 Line 5422 in e933dfb
|
|
After @bena-nasa walked me through the codes, I now understand why my new MoistAlarm affects the old generic ALARM. It is the parameter name "MOIST_DT" that gets into the generic call and changes the generic alarm's interval. ( it checks "DT", and eventually checks "MOIST_DT") . That really eases my mind and I am confident that this MoistAlarm will not change other components' alarms. @atrayano @tclune |
|
@weiyuan-jiang is this PR finalized? I'm unsure since this still has the DNA label. |
|
We can use this PR [https://github.com/GEOS-ESM/MAPL/pull/1369] in MAPL to achieve the same function @atrayano @tclune |
|
@weiyuan-jiang there haven't been any updates to this PR in quite some time. Is this ready for merging? If not, could you please convert this to a draft PR or close it if no longer being pursued. |
|
Do you have opinion on the PR in MAPL ? ( GEOS-ESM/MAPL#1369 ) ? We need to choose one. @tclune @bena-nasa @atrayano |
|
I have not looked closely at this ticket. If there are choices, I would largely defer to @atrayano as to the preferred path. |
|
@weiyuan-jiang can we close this now? There has been no movement for quite some time and things looks stale. I would like to clear out stagnant PRs that are just cluttering the queue. |
|
OK, let me close it for now. I will reopen it when we need it. |
Initial check in to address issue #526 .