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

Add option to use BT_CONT to calculate dtbt #823

Merged
merged 3 commits into from
Feb 13, 2025

Conversation

herrwang0
Copy link

This PR recovers an option to use BT_CONT to calculate dtbt in subroutine set_dtbt by adding a new runtime parameter SET_DTBT_USE_BT_CONT. The option to use BT_CONT in set_dtbt previously exists but is never used.

Currently, when USE_BT_CONT_TYPE is true (default), dtbt is effectively calculated using topography (i.e. with zero sea surface height). For example, eta in the following call is never used as its usage requires NONLINEAR_BT_CONTINUITY is true, which is by default false with USE_BT_CONT_TYPE=True.

if (calc_dtbt) call set_dtbt(G, GV, US, CS%barotropic_CSp, eta, CS%pbce)

With the old setup, the model fails to correctly estimate dtbt when the domain has an uneven mean surface height like the Great Lakes system.

This PR provides a fix to issue #660.

This PR also modifies the interface of subroutine set_dtbt for better readability.

There is no answer change but a new runtime parameter is added.

Copy link
Member

@Hallberg-NOAA Hallberg-NOAA left a comment

Choose a reason for hiding this comment

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

Overall, these changes make sense to me, including the reordering of the optional arguments to set_dtbt (which is something we would usually discourage). However, there are some very minor changes that I would like to see to the declaration lines for CS%dtbt_use_bt_cont (detailed separately in a specific comment) before this PR should be merged into dev/gfdl.

Copy link
Member

@Hallberg-NOAA Hallberg-NOAA left a comment

Choose a reason for hiding this comment

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

This PR looks to me like it has been properly implemented and is ready to be merged in.

Change the order of optional input arguments of subroutine set_dtbt so
that they are grouped logically. The decription of the subroutine is
also updated.
The runtime parameter allows to use BT_CONT optional input argument in
subroutine set_dtbt if BT_CONT type is available.

Originally, when BT_CONT is used, set_dtbt is estimated with zero
surface height. The eta optinoal input argument has no effect since it
only works with NONLINEAR_BT_CONTINUITY is true, which is by default
false when BT_CONT is used.

The added option allows accurate estimate of dtbt when the mean surface
height of the domain is not at the same level, while maintains old
answers.

Also, an unused field calc_dtbt in type MOM_dyn_split_RK2_CS is removed.
* Fix a bug that local variable calc_dtbt is not initialized.
* Rename dtbt_tmp to dtbt_restart for clarity
* Add a comment about the usage of subroutine call set_dtbt
* Remove an unused input argument eta in barotropic_init
@Hallberg-NOAA
Copy link
Member

This PR has passed pipeline testing at https://gitlab.gfdl.noaa.gov/ogrp/mom6ci/MOM6/-/pipelines/26378 with the expected warnings about a new runtime parameter in MOM_parameter_doc files.

@Hallberg-NOAA Hallberg-NOAA merged commit 075f8b3 into NOAA-GFDL:dev/gfdl Feb 13, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants