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

Set SGH coupling interval #125

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

alexolinhager
Copy link

Establishes config_SGH_coupling_interval, the interval at which the SGH model is coupled to the dynamics model. SGH output is then averaged over the coupling interval so that the dynamics model only sees a time-averaged hydrologic state, and not an arbitrary sampling of hydrologic conditions. config_SGH_coupling_interval must be able to divide evenly into config_dt or config_adaptive_timestep_force_interval, whichever one is used.

Established the initial framework to implement the SGH coupling interval
alarm. Not compilable at this point
@alexolinhager alexolinhager added the in progress Still being worked on, don't merge yet! label Sep 10, 2024
Introduces a new subroutine to the SGH model that ensures
config_adaptive_interval_force_interval divides evenly into
config_SGH_coupling_interval
Copy link

@matthewhoffman matthewhoffman left a comment

Choose a reason for hiding this comment

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

@alexolinhager , I know you are still working on this PR, but I thought I'd give it a quick review to help keep it moving. I know you are aware of some of these comments already.

@@ -20,7 +20,11 @@
description="The maximum allowable time step in seconds. If the allowable time step determined by the adaptive CFL calculation is longer than this, then the model will specify config_SGH_max_adaptive_timestep as the time step instead. Defaults to 100 years (in seconds)."
possible_values="Any non-negative real value."
/>
<nml_option name="config_SGH_tangent_slope_calculation" type="character" default_value="from_normal_slope" units="unitless"
<nml_option name="config_SGH_coupling_interval" type="character" default_value="0000-00-00_00:00:01" units="unitless"
description="Time interval at which the SGH model is called by MALI. If set to an interval less than the master timestep, config_SGH_coupling_interval will be superceded by the master timestep. If greater than the master timestep, config_adaptive_timestep_force_interval will be set equal to config_SGH_coupling_interval."

Choose a reason for hiding this comment

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

Rather than modifying config_adaptive_timestep_force_interval, it might be preferable to throw an error if it's incompatible with the new option. I'm thinking this because config_adaptive_timestep_force_interval is used for other things too and it might get wonky if some parts of the code or adjusting it.

call mpas_pool_get_array(meshPool, 'simulationStartTime', simulationStartTime)
call mpas_set_time(start_time, dateTimeString=simulationStartTime)
! define SGH coupling interval as a timeInterval type
call mpas_set_timeInterval(coupling_interval, timeString=config_SGH_coupling_interval, ierr=err_tmp)

Choose a reason for hiding this comment

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

need to retrieve config_SGH_coupling_interval before using it

!> \details
!> This routine checks that the SGH coupling interval is an even multiple
!> of the adaptive timestep force inverval and divides evenly into the
!> restart interval.

Choose a reason for hiding this comment

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

I think we settled on not checking the restart interval, because we don't need the SGH coupling interval to divide evenly into the restart interval if we add the effective pressure accumulator fields as restart fields. I realize this comment might be a holdover from where this was copied from, so just flagging this for follow up.




! Check SGH coupling interval is compatible with master timestep and restart interval

Choose a reason for hiding this comment

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

Suggested change
! Check SGH coupling interval is compatible with master timestep and restart interval
! Check SGH coupling interval is compatible with master timestep and timestepping force interval

do while (associated(block))

call mpas_pool_get_subpool(block % structs, 'hydro', hydroPool)
call mpas_pool_get_array(hydroPool, 'effectivePressureSGH', effectivePressureSGH)

Choose a reason for hiding this comment

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

Suggested change
call mpas_pool_get_array(hydroPool, 'effectivePressureSGH', effectivePressureSGH)
call mpas_pool_get_array(hydroPool, 'effectivePressure', effectivePressure)

@alexolinhager alexolinhager force-pushed the alexolinhager/set_SGH_coupling_interval branch from 785649e to 727dcd9 Compare September 16, 2024 15:33
Removes line that sets effectivePressure equal to zero at the start of
each timecycle; otherwise, effectivePressure was zero until the very end
of the timecycle. Instead, effectivePressure is now initialized to be equal
to effectivePressureSGH if there is no effectivePressure field in the input
file. A defualt value of -1e36 is given to effectivePressure to flag
whether the field is empty at the start of the run.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in progress Still being worked on, don't merge yet!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants