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

Adding temperature restore capability for SPEAR. #36

Merged
merged 6 commits into from
Dec 18, 2021

Conversation

wfcooke
Copy link

@wfcooke wfcooke commented Dec 10, 2021

Added parameter SPEAR_ECDA_SST_RESTORE_TFREEZE to allow activation of
sea surface salinity based modification of restoring of temperature.
The formula used is different from the Millero (default in SPEAR runs)
scheme.

Added parameter SPEAR_ECDA_SST_RESTORE_TFREEZE to allow activation of
sea surface salinity based modification of restoring of temperature.
The formula used is different from the Millero (default in SPEAR runs)
scheme.
@codecov
Copy link

codecov bot commented Dec 10, 2021

Codecov Report

Merging #36 (50c3f35) into dev/gfdl (50df270) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           dev/gfdl      #36   +/-   ##
=========================================
  Coverage     28.96%   28.96%           
=========================================
  Files           240      240           
  Lines         71212    71212           
=========================================
  Hits          20623    20623           
  Misses        50589    50589           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 50df270...50c3f35. Read the comment docs.

if ( CS%trestore_SPEAR_ECDA ) then
do j=js,je ; do i=is,ie
if (abs(data_restore(i,j)+1.8)<0.0001) then
data_restore(i,j) = -0.0539*sfc_state%SSS(i,j)
Copy link
Member

Choose a reason for hiding this comment

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

This particular expression has a hard-coded constant that may be specific to the particular choice of the expression (here linear) for the freezing point. Would it be better to replace this with a call to the MOM6 code that actually calculates the freezing point based on the salinity?

Also, elsewhere in this routine, I think that it would be a good to make it clear in the comments that this option is only changing the target temperatures when they appear to be very close to the freezing point.

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.

I see how this could be useful, but before accepting these changes, please consider making the following changes:

  • Expand the comments to make it clear that these changes only apply to temperatures near the nominal freezing point.
  • Add run-time parameters to specify the dimensional constants used by this new scheme. These parameters should only be logged if this new option used.

We are trying to actively discourage the introduction of any new hard-coded dimensional constants, and we will be moving to replace all existing significant dimensional constants (i.e., not absurdly huge or tiny fill values) in the MOM6 code with run-time variables. The hard-coded dimensional values are fine as the defaults for the run-time variables, because then they will have an explicit context and units that should be obvious to anyone reading the code.

The freezing temperature came from SIS2 code. Changing the default value here to be consistent with that. (-0.054 vs -0.0539)
The salinity restoring code used the -0.0539 value also so answers may change if using that code (RESTORE_SALINITY=T)
The freezing temperature came from SIS2 code. Changing the default value here to be consistent with that. (-0.054 vs -0.0539)
The salinity restoring code used the -0.0539 value also so answers may change if using that code (RESTORE_SALINITY=T)
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.

With the more recent updates, I think that this commit is looking right, and I agree that it should be merged in after the pipeline and TC testing have passed on the updated version.

It should be noted that this PR will change the MOM_parameter_doc.all files for some experiments, so there will be a manual update required for MOM6-examples as soon as this PR is accepted.

@marshallward
Copy link
Member

marshallward commented Dec 18, 2021

Gaea regression: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines ✔️ 🟡

New parameter:

  • SPEAR_DTFREEZE_DS

Squash-merging this one, since subsequent commit logs look incremental.

@marshallward marshallward merged commit 12f29f6 into NOAA-GFDL:dev/gfdl Dec 18, 2021
@Hallberg-NOAA Hallberg-NOAA added the enhancement New feature or request label Feb 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants