Fixed "Choosing an appropriate time step" to match ice_dyn_shared.F90

[aekiss]

Documentation change only: dt_dyn should have been dt in two places.

Developer(s):
Is the documentation being updated with this PR? (Y)

[eclare108213]

@aekiss, thank you for pointing this out. It's actually correct in the documentation, and inconsistent in the code. dt is used as both the thermodynamic timestep and the dynamic timestep, defined as it is passed through subroutine arguments. E.g.
in CICE_RunMod.F90

  !-----------------------------------------------------------------
  ! dynamics, transport, ridging
  !-----------------------------------------------------------------

     do k = 1, ndtd

        ! momentum, stress, transport
        call step_dyn_horiz (dt_dyn)

        ! ridging
        !$OMP PARALLEL DO PRIVATE(iblk)
        do iblk = 1, nblocks
           call step_dyn_ridge (dt_dyn, ndtd, iblk)
        enddo
        !$OMP END PARALLEL DO

        ! clean up, update tendency diagnostics
        offset = c0
        call update_state (dt_dyn, daidtd, dvidtd, dagedtd, offset)

     enddo

in ice_step_mod.F90

  subroutine step_dyn_horiz (dt)

  use ice_dyn_evp, only: evp
  use ice_dyn_eap, only: eap
  use ice_dyn_shared, only: kdyn
  use ice_flux, only: init_history_dyn
  use ice_transport_driver, only: advection, transport_upwind, transport_remap

  real (kind=dbl_kind), intent(in) :: &
     dt      ! dynamics time step

This is the result of relatively slow code evolution -- the dt_dyn loop was added much later than evp itself, and the original code in evp was kept rather than renaming dt everywhere. If it's confusing, we should probably clean that up.

[aekiss]

Oops, that's embarrassing, sorry I didn't read the code more carefully before posting. Thanks for your quick response.

So in section 3.1.5
https://cice-consortium.github.io/CICE/cice_3_user_guide.html#choosing-an-appropriate-time-step
should

In practice, the ratio
:math:`\Delta t_e ~:~ T ~:~ \Delta t`  = 1 : 40 : 120 provides both
stability and acceptable efficiency for time steps (:math:`\Delta t`) on
the order of 1 hour.

actually be written in terms of dt_dyn, as

In practice, the ratio
:math:`\Delta t_e ~:~ T ~:~ \Delta t_{dyn}`  = 1 : 40 : 120 provides both
stability and acceptable efficiency for dynamic time steps (:math:`\Delta t_{dyn}`) on
the order of 1 hour.

so that the ratio depends only on eyc and ndte, rather than eyc, ndte and ndtd? This makes more physical sense to me.

If so, the first sentence of the final paragraph would be clearer as

Note that only :math:`T` and :math:`\Delta t_e` figure into the
stability of the dynamics component; :math:`\Delta t` and :math:`\Delta t_{dyn}` do not.

[eclare108213]

Yes, I agree with your suggested changes. I'll reopen the PR so that you can fix them! Thanks much.

[aekiss]

Thanks.

I also wonder whether \Delta t should be \Delta t_dyn throughout section 2.2.3 for clarity, or at least mention at the start of section 2.2.3 that \Delta t should be interpreted as \Delta t_dyn if ndtd>1?
https://cice-consortium.github.io/CICE/cice_2_science_guide.html#dynamics
I guess dt is intended to be interpreted heuristically, not necessarily as the thermodynamic timestep, but it makes it difficult to dip and and out of different sections.

For example section 2.2.3.2 defines T=eyc*dt (not eyc*dt_dyn)
https://cice-consortium.github.io/CICE/cice_2_science_guide.html#internal-stress

[eclare108213]

@dabail10, since you implemented the extra loop around the dynamics and I'm not aware of any other group actually using it, is this something that you would still consider useful? I'm wondering if we should just remove it from the code. The dynamics should converge better once we get Martin Losch's changes for the revp option in there.

[dabail10]

We do need the extra loop around the dynamics code still. This way we can use a larger timestep for the thermo and a smaller timestep for the dynamics. This is particularly key for high resolution.

Dave

[aekiss]

Yes, we are considering using this extra loop in our 0.1deg model.

[eclare108213]

Then it makes sense to make the code and the documentation consistent. We can certainly use dt_dyn in the documentation instead of dt, in reference to the dynamics. It's true that dt is used both as the thermodynamic time step and as a generic one. I'm open to creating an explicit dt_therm time step but also happy to stick with dt, since the way the code is written, the thermo step is the main one and others cycle around it.

[apcraig]

Just checking on the status of this PR. Should this be moved to an issue and closed? Is this ready to be merged?

[eclare108213]

This PR should not be merged as it is. The documentation needs to be changed, but differently from this. I recommend closing this one and creating an issue for it.

[apcraig]

OK, will do. Would love to move this PR to an issue to preserve the discussion. not sure if I can but will try.

[eclare108213]

You can just reference the PR in the issue, and it'll come up...