Obc setup plus segment update period #198
Conversation
- These are updates required to setup OBC segments for OBGC tracers. - Since COBALT package has more than 50 tracers using the MOM6 table mechanism for setting up OBC segments is not feasible. Rather, this update delegates such setup to mechanims used in ocean_BGS tracers leaving MOM6 mechanism for native tracers intact.
- These are updates required to setup OBC segments for OBGC tracers. - Since COBALT package has more than 50 tracers using the MOM6 table mechanism for setting up OBC segments is not feasible. Rather, this update delegates such setup to mechanims used in ocean_BGS tracers leaving MOM6 mechanism for native tracers intact. - Fixed issues caught by MOM6 githubCI
- These are updates required to setup OBC segments for OBGC tracers.
- Since COBALT package has more than 50 tracers using the MOM6 table
mechanism for setting up OBC segments is not feasible. Rather, this
update delegates such setup to mechanims used in ocean_BGS tracers
leaving MOM6 mechanism for native tracers intact.
- Fixed issues caught by MOM6 githubCI
- These are updates required to setup OBC segments for OBGC tracers. - Since COBALT package has more than 50 tracers using the MOM6 table mechanism for setting up OBC segments is not feasible. Rather, this update delegates such setup to mechanims used in ocean_BGS tracers leaving MOM6 mechanism for native tracers intact. - Fixed issues caught by MOM6 githubCI
- These are updates required to setup OBC segments for OBGC tracers. - Since COBALT package has more than 50 tracers using the MOM6 table mechanism for setting up OBC segments is not feasible. Rather, this update delegates such setup to mechanims used in ocean_BGS tracers leaving MOM6 mechanism for native tracers intact. - Fixed issues caught by MOM6 githubCI
- These are updates required to setup OBC segments for OBGC tracers.
- Since COBALT package has more than 50 tracers using the MOM6 table
mechanism for setting up OBC segments is not feasible. Rather, this
update delegates such setup to mechanims used in ocean_BGS tracers
leaving MOM6 mechanism for native tracers intact.
- Fixed issues caught by MOM6 githubCI
- COBALT tracers do not need as frequent segment bc updates and can use a larger update period to speed up the model. This commit introduces a new parameter DT_OBC_SEG_UPDATE_OBGC that can be adjusted for obc segment update period. - This commit applies the change only to BGC tracers but can easily be changed to apply for all.
Codecov Report
@@ Coverage Diff @@
## dev/gfdl #198 +/- ##
============================================
- Coverage 37.23% 37.21% -0.02%
============================================
Files 263 263
Lines 73060 72938 -122
Branches 13605 13588 -17
============================================
- Hits 27201 27144 -57
+ Misses 40841 40795 -46
+ Partials 5018 4999 -19
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
- COBALT tracers do not need as frequent segment bc updates and can use a larger update period to speed up the model. This commit introduces a new parameter DT_OBC_SEG_UPDATE_OBGC that can be adjusted for obc segment update period. - This commit applies the change only to BGC tracers but can easily be changed to apply for all.
- COBALT tracers do not need as frequent segment bc updates and can use a larger update period to speed up the model. This commit introduces a new parameter DT_OBC_SEG_UPDATE_OBGC that can be adjusted for obc segment update period. - This commit applies the change only to BGC tracers but can easily be changed to apply for all.
nikizadehgfdl
changed the title
|
The Bering domain with all the scale factors failed for me in real_to_time(CS%dt_obc_seg_period) where the dt variable was 1.3e15 instead of 600. This is in MOM.F90#L2895. |
|
@kshedstrom did you set CS%dt_obc_seg_period to something? Where does 1.3e15 come from? How can I repeat your test? Thanks. |
|
Hi Niki, the files for the Bering domain are on gaea in /lustre/f2/dev/Katherine.Hedstrom/ESMG/ESMG-configs/Bering and the INPUT subdirectory. Check the MOM_override file for all the scalings I was testing. It's fine if you turn off all the scalings. Sorry, we're traveling, so I might not be able to get back to you right away if you need more from me. |
| !Set OBC segment data update period | ||
| if (associated(CS%OBC) .and. CS%dt_obc_seg_period > 0.0) then | ||
| CS%dt_obc_seg_interval = real_to_time(CS%dt_obc_seg_period) | ||
| CS%dt_obc_seg_time = Time + CS%dt_obc_seg_interval |
There was a problem hiding this comment.
I need to test this, but I think setting this to Time + CS%dt_obc_seg_interval results in the tracer reservoirs not being initialized at the first time step and only getting initialized at the first obc segment update period. In both NWA and NEP regional domains, water inflowing from the boundary is very low in alkalinity, oxygen, and other tracers when the model first starts running, as if the reservoirs were initialized at 0.
There was a problem hiding this comment.
I guess the reservoirs should also be initialized by fill_obgc_segments. Need to look into this more.
There was a problem hiding this comment.
I found a way to fix this issue with two comments that I've left below.
src/core/MOM_open_boundary.F90
Outdated
| endif | ||
| segment%tr_Reg%Tr(nt)%tres(:,:,:) = segment%tr_Reg%Tr(nt)%t(:,:,:) | ||
| enddo | ||
| ! call setup_OBC_tracer_reservoirs(GV, OBC, .true.) ! Skip T and S to avoid restart bug |
There was a problem hiding this comment.
This commented line is a leftover from old debugging and can be deleted. But, there is a necessary addition to this subroutine. The generic tracer segment data was not set up yet during MOM_initialize_state() -> fill_temp_salt_segments() -> setup_obc_tracer_reservoirs(), leaving OBC%tres_* 0 and causing low values to propagate in from the boundary initially. One way to fix this is to copy to OBC%tres_* again in this subroutine. I tried this here and it works.
- The unit conversion factor was missing causing a crash in a newer test.
- Avoid low initial values in the tracer reservoirs
|
I made the following updates to this PR
|
|
Thanks Niki. I'm still getting tracer reservoirs that are initialized with zeros. I think we still need to add my first comment, replacing |
There was a problem hiding this comment.
This PR appears to be close to being functionally correct (apart from addressing the issue with line 402 of MOM_generic_tracer.F90. However by using module variables, it disrupts the ability that MOM6 would otherwise have of potentially running multiple ensemble members within the same memory space. Moreover, there are many places where this contribution does not follow the code style guidance at https://github.com/NOAA-GFDL/MOM6/wiki/Code-style-guide. Please correct these issues in a revision to this PR.
- Reviewer asked to avoid using module variables with "save" attributes. - This commit hides the module variables inside the existing OBC type.
| @@ -2152,6 +2167,13 @@ subroutine initialize_MOM(Time, Time_init, param_file, dirs, CS, restart_CSp, & | |||
| units="s", default=default_val, do_not_read=(dtbt > 0.0)) | |||
| endif | |||
|
|
|||
| CS%dt_obc_seg_period = -1.0 | |||
| call get_param(param_file, "MOM", "DT_OBC_SEG_UPDATE_OBGC", CS%dt_obc_seg_period, & | |||
There was a problem hiding this comment.
As written, this new parameter is being logged in every MOM_parameter_doc.all file, including in many where it is meaningless. Please add the argument do_not_log=.not.associated(CS%OBC) to this call or wrap it in on logical test for whether open boundary conditions are actually being used.
There was a problem hiding this comment.
This PR conditionally passed the pipeline testing at https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/17365, but every MOM_parameter_doc.all file was changed, including those for which these change are not pertinent.
Please modify this PR as described in a separate comment on the relevant line to avoid adding the new runtime parameter DT_OBC_SEG_UPDATE_OBGC to the MOM_parameter_doc.all files when it is irrelevant.
|
This PR is undergoing another repeat of the pipeline testing at https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/17407. |
|
Because of all of the merges and commits with minor corrections with this PR, I am going to handle this via a squash merge. |
This is two sets of updates for OBC in COBALT/OBGC tracers combined.
Setup OBC segments for COBALT/OBGC tracers
Add capability to change obc segment update period