Skip to content

Commit

Permalink
+FIX_UNSPLIT_DT_VISC_BUG is now UNSPLIT_DT_VISC_BUG
Browse files Browse the repository at this point in the history
  Renamed the runtime parameter FIX_UNSPLIT_DT_VISC_BUG to UNSPLIT_DT_VISC_BUG
(with a switch between the meanings of true and false for the two parameters)
for consistency with the syntax of other bug-fix flags in MOM6 and to partially
address dev/gfdl MOM6 issue #237.  Input parameter files need not be changed
right away because MOM6 will still work if FIX_UNSPLIT_DT_VISC_BUG is specified
instead of UNSPLIT_DT_VISC_BUG, but UNSPLIT_DT_VISC_BUG will be logged, so there
are changes to the MOM_parameter_doc files.   By default or with existing input
parameter files, all answers are bitwise identical, and there is error handling
if inconsistent settings of FIX_UNSPLIT_DT_VISC_BUG and UNSPLIT_DT_VISC_BUG are
both specified.
  • Loading branch information
Hallberg-NOAA committed Dec 17, 2023
1 parent 4e8fbe1 commit 7ded1d8
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 28 deletions.
57 changes: 45 additions & 12 deletions src/core/MOM_dynamics_unsplit.F90
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ module MOM_dynamics_unsplit
use MOM_domains, only : pass_vector, pass_vector_start, pass_vector_complete
use MOM_domains, only : To_South, To_West, To_All, CGRID_NE, SCALAR_PAIR
use MOM_error_handler, only : MOM_error, MOM_mesg, FATAL, WARNING, is_root_pe
use MOM_file_parser, only : get_param, log_version, param_file_type
use MOM_file_parser, only : get_param, log_param, log_version, param_file_type
use MOM_get_input, only : directories
use MOM_time_manager, only : time_type, real_to_time, operator(+)
use MOM_time_manager, only : operator(-), operator(>), operator(*), operator(/)
Expand Down Expand Up @@ -116,13 +116,13 @@ module MOM_dynamics_unsplit
real, pointer, dimension(:,:) :: tauy_bot => NULL() !< frictional y-bottom stress from the ocean
!! to the seafloor [R L Z T-2 ~> Pa]

logical :: use_correct_dt_visc !< If true, use the correct timestep in the viscous terms applied
!! in the first predictor step with the unsplit time stepping scheme,
!! and in the calculation of the turbulent mixed layer properties
!! for viscosity. The default should be true, but it is false.
logical :: debug !< If true, write verbose checksums for debugging purposes.
logical :: calculate_SAL !< If true, calculate self-attraction and loading.
logical :: use_tides !< If true, tidal forcing is enabled.
logical :: dt_visc_bug !< If false, use the correct timestep in viscous terms applied in the
!! first predictor step and in the calculation of the turbulent mixed
!! layer properties for viscosity. If this is true, an older incorrect
!! setting is used.
logical :: debug !< If true, write verbose checksums for debugging purposes.
logical :: calculate_SAL !< If true, calculate self-attraction and loading.
logical :: use_tides !< If true, tidal forcing is enabled.

logical :: module_is_initialized = .false. !< Record whether this module has been initialized.

Expand Down Expand Up @@ -346,11 +346,11 @@ subroutine step_MOM_dyn_unsplit(u, v, h, tv, visc, Time_local, dt, forces, &
! up <- up + dt/2 d/dz visc d/dz up
call cpu_clock_begin(id_clock_vertvisc)
call enable_averages(dt, Time_local, CS%diag)
dt_visc = 0.5*dt ; if (CS%use_correct_dt_visc) dt_visc = dt
dt_visc = dt ; if (CS%dt_visc_bug) dt_visc = 0.5*dt
call set_viscous_ML(u, v, h_av, tv, forces, visc, dt_visc, G, GV, US, CS%set_visc_CSp)
call disable_averaging(CS%diag)

dt_visc = 0.5*dt ; if (CS%use_correct_dt_visc) dt_visc = dt_pred
dt_visc = dt_pred ; if (CS%dt_visc_bug) dt_visc = 0.5*dt
call thickness_to_dz(h_av, tv, dz, G, GV, US, halo_size=1)
call vertvisc_coef(up, vp, h_av, dz, forces, visc, tv, dt_visc, G, GV, US, CS%vertvisc_CSp, CS%OBC, VarMix)
call vertvisc(up, vp, h_av, forces, visc, dt_visc, CS%OBC, CS%ADp, CS%CDp, &
Expand Down Expand Up @@ -630,6 +630,9 @@ subroutine initialize_dyn_unsplit(u, v, h, Time, G, GV, US, param_file, diag, CS
character(len=48) :: flux_units
! This include declares and sets the variable "version".
# include "version_variable.h"
logical :: use_correct_dt_visc
logical :: test_value ! This is used to determine whether a logical parameter is being set explicitly.
logical :: explicit_bug, explicit_fix ! These indicate which parameters are set explicitly.
integer :: isd, ied, jsd, jed, nz, IsdB, IedB, JsdB, JedB
isd = G%isd ; ied = G%ied ; jsd = G%jsd ; jed = G%jed ; nz = GV%ke
IsdB = G%IsdB ; IedB = G%IedB ; JsdB = G%JsdB ; JedB = G%JedB
Expand All @@ -646,11 +649,41 @@ subroutine initialize_dyn_unsplit(u, v, h, Time, G, GV, US, param_file, diag, CS
CS%diag => diag

call log_version(param_file, mdl, version, "")
call get_param(param_file, mdl, "FIX_UNSPLIT_DT_VISC_BUG", CS%use_correct_dt_visc, &

call get_param(param_file, mdl, "UNSPLIT_DT_VISC_BUG", CS%dt_visc_bug, &
"If false, use the correct timestep in the viscous terms applied in the first "//&
"predictor step with the unsplit time stepping scheme, and in the calculation "//&
"of the turbulent mixed layer properties for viscosity with unsplit or "//&
"unsplit_RK2. If true, an older incorrect value is used.", &
default=.false., do_not_log=.true.)
! This is used to test whether UNSPLIT_DT_VISC_BUG is being actively set.
call get_param(param_file, mdl, "UNSPLIT_DT_VISC_BUG", test_value, default=.true., do_not_log=.true.)
explicit_bug = CS%dt_visc_bug .eqv. test_value
call get_param(param_file, mdl, "FIX_UNSPLIT_DT_VISC_BUG", use_correct_dt_visc, &
"If true, use the correct timestep in the viscous terms applied in the first "//&
"predictor step with the unsplit time stepping scheme, and in the calculation "//&
"of the turbulent mixed layer properties for viscosity with unsplit or "//&
"unsplit_RK2.", default=.true.)
"unsplit_RK2.", default=.true., do_not_log=.true.)
call get_param(param_file, mdl, "FIX_UNSPLIT_DT_VISC_BUG", test_value, default=.false., do_not_log=.true.)
explicit_fix = use_correct_dt_visc .eqv. test_value

if (explicit_bug .and. explicit_fix .and. (use_correct_dt_visc .eqv. CS%dt_visc_bug)) then
! UNSPLIT_DT_VISC_BUG is being explicitly set, and should not be changed.
call MOM_error(FATAL, "UNSPLIT_DT_VISC_BUG and FIX_UNSPLIT_DT_VISC_BUG are both being set "//&
"with inconsistent values. FIX_UNSPLIT_DT_VISC_BUG is an obsolete "//&
"parameter and should be removed.")

Check warning on line 674 in src/core/MOM_dynamics_unsplit.F90

View check run for this annotation

Codecov / codecov/patch

src/core/MOM_dynamics_unsplit.F90#L674

Added line #L674 was not covered by tests
elseif (explicit_fix) then
call MOM_error(WARNING, "FIX_UNSPLIT_DT_VISC_BUG is an obsolete parameter. "//&
"Use UNSPLIT_DT_VISC_BUG instead (noting that it has the opposite sense).")
CS%dt_visc_bug = .not.use_correct_dt_visc
endif
call log_param(param_file, mdl, "UNSPLIT_DT_VISC_BUG", CS%dt_visc_bug, &
"If false, use the correct timestep in the viscous terms applied in the first "//&
"predictor step with the unsplit time stepping scheme, and in the calculation "//&
"of the turbulent mixed layer properties for viscosity with unsplit or "//&
"unsplit_RK2. If true, an older incorrect value is used.", &
default=.false.)

call get_param(param_file, mdl, "DEBUG", CS%debug, &
"If true, write out verbose debugging data.", &
default=.false., debuggingParam=.true.)
Expand Down
65 changes: 49 additions & 16 deletions src/core/MOM_dynamics_unsplit_RK2.F90
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ module MOM_dynamics_unsplit_RK2
use MOM_domains, only : To_South, To_West, To_All, CGRID_NE, SCALAR_PAIR
use MOM_error_handler, only : MOM_error, MOM_mesg, FATAL, WARNING, is_root_pe
use MOM_error_handler, only : MOM_set_verbosity
use MOM_file_parser, only : get_param, log_version, param_file_type
use MOM_file_parser, only : get_param, log_param, log_version, param_file_type
use MOM_get_input, only : directories
use MOM_time_manager, only : time_type, time_type_to_real, operator(+)
use MOM_time_manager, only : operator(-), operator(>), operator(*), operator(/)
Expand Down Expand Up @@ -114,18 +114,18 @@ module MOM_dynamics_unsplit_RK2
real, pointer, dimension(:,:) :: tauy_bot => NULL() !< frictional y-bottom stress from the ocean
!! to the seafloor [R L Z T-2 ~> Pa]

real :: be !< A nondimensional number from 0.5 to 1 that controls
!! the backward weighting of the time stepping scheme [nondim].
real :: begw !< A nondimensional number from 0 to 1 that controls
!! the extent to which the treatment of gravity waves
!! is forward-backward (0) or simulated backward
!! Euler (1) [nondim]. 0 is often used.
logical :: use_correct_dt_visc !< If true, use the correct timestep in the calculation of the
!! turbulent mixed layer properties for viscosity.
!! The default should be true, but it is false.
logical :: debug !< If true, write verbose checksums for debugging purposes.
logical :: calculate_SAL !< If true, calculate self-attraction and loading.
logical :: use_tides !< If true, tidal forcing is enabled.
real :: be !< A nondimensional number from 0.5 to 1 that controls
!! the backward weighting of the time stepping scheme [nondim].
real :: begw !< A nondimensional number from 0 to 1 that controls
!! the extent to which the treatment of gravity waves
!! is forward-backward (0) or simulated backward
!! Euler (1) [nondim]. 0 is often used.
logical :: dt_visc_bug !< If false, use the correct timestep in the calculation of the
!! turbulent mixed layer properties for viscosity. Otherwise if
!! this is true, an older incorrect setting is used.
logical :: debug !< If true, write verbose checksums for debugging purposes.
logical :: calculate_SAL !< If true, calculate self-attraction and loading.
logical :: use_tides !< If true, tidal forcing is enabled.

logical :: module_is_initialized = .false. !< Record whether this module has been initialized.

Expand Down Expand Up @@ -344,7 +344,7 @@ subroutine step_MOM_dyn_unsplit_RK2(u_in, v_in, h_in, tv, visc, Time_local, dt,
! up[n-1/2] <- up*[n-1/2] + dt/2 d/dz visc d/dz up[n-1/2]
call cpu_clock_begin(id_clock_vertvisc)
call enable_averages(dt, Time_local, CS%diag)
dt_visc = dt_pred ; if (CS%use_correct_dt_visc) dt_visc = dt
dt_visc = dt ; if (CS%dt_visc_bug) dt_visc = dt_pred
call set_viscous_ML(u_in, v_in, h_av, tv, forces, visc, dt_visc, G, GV, US, CS%set_visc_CSp)
call disable_averaging(CS%diag)

Expand Down Expand Up @@ -578,6 +578,9 @@ subroutine initialize_dyn_unsplit_RK2(u, v, h, Time, G, GV, US, param_file, diag
character(len=48) :: flux_units
! This include declares and sets the variable "version".
# include "version_variable.h"
logical :: use_correct_dt_visc
logical :: test_value ! This is used to determine whether a logical parameter is being set explicitly.
logical :: explicit_bug, explicit_fix ! These indicate which parameters are set explicitly.
integer :: isd, ied, jsd, jed, nz, IsdB, IedB, JsdB, JedB
isd = G%isd ; ied = G%ied ; jsd = G%jsd ; jed = G%jed ; nz = GV%ke
IsdB = G%IsdB ; IedB = G%IedB ; JsdB = G%JsdB ; JedB = G%JedB
Expand Down Expand Up @@ -610,11 +613,41 @@ subroutine initialize_dyn_unsplit_RK2(u, v, h, Time, G, GV, US, param_file, diag
"If SPLIT is false and USE_RK2 is true, BEGW can be "//&
"between 0 and 0.5 to damp gravity waves.", &
units="nondim", default=0.0)
call get_param(param_file, mdl, "FIX_UNSPLIT_DT_VISC_BUG", CS%use_correct_dt_visc, &

call get_param(param_file, mdl, "UNSPLIT_DT_VISC_BUG", CS%dt_visc_bug, &
"If false, use the correct timestep in the viscous terms applied in the first "//&
"predictor step with the unsplit time stepping scheme, and in the calculation "//&
"of the turbulent mixed layer properties for viscosity with unsplit or "//&
"unsplit_RK2. If true, an older incorrect value is used.", &
default=.false., do_not_log=.true.)
! This is used to test whether UNSPLIT_DT_VISC_BUG is being explicitly set.
call get_param(param_file, mdl, "UNSPLIT_DT_VISC_BUG", test_value, default=.true., do_not_log=.true.)
explicit_bug = CS%dt_visc_bug .eqv. test_value
call get_param(param_file, mdl, "FIX_UNSPLIT_DT_VISC_BUG", use_correct_dt_visc, &
"If true, use the correct timestep in the viscous terms applied in the first "//&
"predictor step with the unsplit time stepping scheme, and in the calculation "//&
"of the turbulent mixed layer properties for viscosity with unsplit or "//&
"unsplit_RK2.", default=.true.)
"unsplit_RK2.", default=.true., do_not_log=.true.)
call get_param(param_file, mdl, "FIX_UNSPLIT_DT_VISC_BUG", test_value, default=.false., do_not_log=.true.)
explicit_fix = use_correct_dt_visc .eqv. test_value

if (explicit_bug .and. explicit_fix .and. (use_correct_dt_visc .eqv. CS%dt_visc_bug)) then
! UNSPLIT_DT_VISC_BUG is being explicitly set, and should not be changed.
call MOM_error(FATAL, "UNSPLIT_DT_VISC_BUG and FIX_UNSPLIT_DT_VISC_BUG are both being set "//&
"with inconsistent values. FIX_UNSPLIT_DT_VISC_BUG is an obsolete "//&
"parameter and should be removed.")

Check warning on line 638 in src/core/MOM_dynamics_unsplit_RK2.F90

View check run for this annotation

Codecov / codecov/patch

src/core/MOM_dynamics_unsplit_RK2.F90#L638

Added line #L638 was not covered by tests
elseif (explicit_fix) then
call MOM_error(WARNING, "FIX_UNSPLIT_DT_VISC_BUG is an obsolete parameter. "//&
"Use UNSPLIT_DT_VISC_BUG instead (noting that it has the opposite sense).")
CS%dt_visc_bug = .not.use_correct_dt_visc
endif
call log_param(param_file, mdl, "UNSPLIT_DT_VISC_BUG", CS%dt_visc_bug, &
"If false, use the correct timestep in the viscous terms applied in the first "//&
"predictor step with the unsplit time stepping scheme, and in the calculation "//&
"of the turbulent mixed layer properties for viscosity with unsplit or "//&
"unsplit_RK2. If true, an older incorrect value is used.", &
default=.false.)

call get_param(param_file, mdl, "DEBUG", CS%debug, &
"If true, write out verbose debugging data.", &
default=.false., debuggingParam=.true.)
Expand Down

0 comments on commit 7ded1d8

Please sign in to comment.