NOAA-CEFI-Regional-Ocean-Modeling / ocean_BGC

3 stars 8 forks source link

Sed and coastal/clean code #89

Closed charliestock closed 3 months ago

charliestock commented 3 months ago

This pull request cleans up and adds commenting to sections of the sediment code. It also modifies the formulation of a previously used "coastal" iron source (jfe_coast). This source of iron was used in TOPAZ and early versions of COBALT to simulate coastal iron fluxes from the vertical land face. This was done because coarse resolution often prevented significant sediment iron fluxes from being introduced near the ocean surface in shallow waters: the grid was so coarse, even the first grid cell was far deeper than the euphotic zone in most places. Releasing iron from the land edges provided a means of generating "island wakes" even when the model resolution prevented resolution of shallow nearshore areas.

As the resolution of our ocean model configurations has increased, we have moved away from using this "coastal" source, instead relying on the benthic fluxes that are explicitly resolved by the model. This has worked for nearly all of our CEFI runs. Recent Pacific Island simulations done by @gabyneg, however, suggest that there still may be a role for this parameterized flux in areas with particularly narrow shelves and steep bathymetry. However, when I looked back at the old way the flux was formulated I found the underlying rationale lacking. I thus coded an alternative that made the coastal flux proportional to the benthic flux that would have arisen from the organic carbon flux and O2 in the adjacent water. Conceptually, one can think of this as a net flux that would have arisen from unresolved shallow regions within the first grid cell. The parameter "fe_coast" becomes a constant of proportionality which we need to explore but I suspect should be ~0.1 for cases where we feel it is necessary to use this parameterization.

The default value of fe_coast is 0 and I feel it should stay this way. With fe_coast = 0, this pull request should not change answers. In addition to the code review and commenting, it would be great if @gabyneg could run a few test cases in her Pacific Islands domain to see if we are satisfied with the new approach.

I still have a bit more work to do with the commenting of the sediment section, but I will complete this in a subsequent pull request.

charliestock commented 3 months ago

Hello @yichengt900, I made the small adjustments we discussed this morning. As expected, it did not seem to fix the cryptic restart issue blocking the pull request. Looking at the code changes, I can't imagine what might be tripping this off. When you get a chance, could you have a look at the restarts and see if they provide any clues?

yichengt900 commented 3 months ago

Hi @charliestock,

I reviewed the differences in the restart files, and the variables showing discrepancies after the model integrated the 3600s time step (the model ran with restart for an hour integration) are listed below:

nccmp -dfqS RESTART/MOM.res.nc RESTART_25hrs/MOM.res.nc
Variable Group Count          Sum      AbsSum         Min          Max       Range         Mean      StdDev
htotal   /        40 -4.40209e-19 4.40209e-19 -1.1162e-20 -1.08261e-20 3.35835e-22 -1.10052e-20 9.42072e-23
co3_ion  /        40  4.17849e-15 4.17849e-15 1.02782e-16  1.05723e-16  2.9409e-18  1.04462e-16 7.98836e-19
cased    /         8  1.36514e-14 1.36514e-14 1.70643e-15  1.70643e-15           0  1.70643e-15           0
dic      /        40  8.87918e-15 8.87918e-15  2.2031e-16  2.23346e-16 3.03577e-18   2.2198e-16 8.47963e-19
alk      /        40   1.7754e-14  1.7754e-14 4.42354e-16  4.45824e-16 3.46945e-18  4.43851e-16 7.54041e-19

I will investigate further.

charliestock commented 3 months ago

That seemed to do the trick YC, thanks!

charliestock commented 3 months ago

It looks like this has passed checks. Since the default fe_coast = 0 won't effect any answers, perhaps we can merge it in?