Keep track of whether runoff has been added to stf #246
Conversation
|
@andrew-c-ross This seems reasonable, but appears to be missing a call to reset this flag to False after coupler updates. |
|
I reset it to False over on the ocean_BGC side (https://github.com/NOAA-GFDL/ocean_BGC/pull/20/files#diff-ae60cbf2eb5c7a9f11effd4929dde30efc21183655cdc945d067bb67a19d6eceR2247); is there a place in MOM6 that I've missed where it should also be set to False (or do you think it is better to do it entirely within MOM6)? |
|
After discussions with @andrew-c-ross and with input from @nikizadehgfdl , the recommendation is to move this PR forward. Existing GFDL-based ocean_BGC configuratons (ESM2G) are not impacted by this change (dt_therm>=dt_cpld). We were unable to identify other existing configurations which would be impacted by this bugfix. The proposed fix would correctly insert the runoff tracer fluxes into the ocean_BGC arrays, and those tracer fluxes would be applied as a surface flux through the tridiagnoal solver. It is worth noting that the ocean_BGC river tracer fluxes are not being vertically distributed when DO_RIVERMIX=True (as is the case for physical tracers ). |
|
Correction regarding DO_RIVERMIX = True. In this case, the diffusive entrainment velocities reflect mixing due to the rivermixing routine, so the ocean_BGC tracers should be vertically mixed consistently with the physical tracers (per @Hallberg-NOAA communication). |
Hallberg-NOAA
force-pushed
the
runoff-addition-fix
branch
from
355ec3d to
d0c656a
Compare
Codecov Report
@@ Coverage Diff @@
## dev/gfdl #246 +/- ##
============================================
- Coverage 37.13% 37.13% -0.01%
============================================
Files 263 263
Lines 73443 73444 +1
Branches 13674 13674
============================================
- Hits 27272 27271 -1
- Misses 41146 41147 +1
- Partials 5025 5026 +1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
This parsimonious PR appears to properly address the problems noted in #217, as noted in the review by @MJHarrison-GFDL. At some point, though, it might make sense to revise the generic tracer code to avoid having a variable (g_tracer%runoff_added_to_stf) that is set and then reset in code from two separate repositories (MOM6 and ocean_BGC).
This PR has passed TC testing and pipeline testing at https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/17711.
|
It appears we needed to wait for NOAA-GFDL/ocean_BGC#20 before accepting this. @nikizadehgfdl any chance you can handles the ocean_BGC PRs? |
This PR is my attempt to fix the issue #217, which affected generic tracers that had fluxes into the ocean from runoff. During a thermodynamics time step, MOM6 adds the river tracer flux to the surface tracer flux
stf. If more than one thermodynamics time step is done per coupler time step, the river input gets added tostfmultiple times. This PR fixes this bug by checking each tracer for a new logical flagrunoff_added_to_stf. If this flag is false, the runoff is added tostfand then the flag is set to true.For this PR to work in configurations using generic tracers, it will need to be paired with a PR I have to ocean_BGC (NOAA-GFDL/ocean_BGC#20) that adds
runoff_added_to_stftog_tracer_typeand resets the flag to false after updates tostfusingg_tracer_set_2Dorg_tracer_set_real. For configurations not using generic tracers, this PR to MOM6 also adds therunoff_added_to_stfflag tog_tracer_typein config_src/external/GFDL_ocean_BGC/generic_tracer_utils.F90, so things should compile as before and no answers should change.I have submitted this as a draft PR to see if the solution makes sense, and so the ocean_BGC PR can be merged first if so.