-
Notifications
You must be signed in to change notification settings - Fork 75
Add spatially-varying mean sea level #1000
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
base: dev/gfdl
Are you sure you want to change the base?
Conversation
The spatially varying time mean sea level meanSL is used as a reference height to calculate, e.g., time mean ocean column thickness max(meanSL + bathyT, 0.0). This field allows the model run in a domain with spatically varying mean height, e.g. the Great Lakes system. This first commit insulates the changes from the rest of the model. It only adds the field to ocean_grid_type and dyn_horgrid_type, the transcription between the two types, and a routine to read it from a file. The field is not yet used by the rest of the code.
This commit uses G%meanSL in 13 modules. The change is essentially replacing G%bathyT + G%Z_ref with G%meanSL + G%bathyT. Note that this does NOT mean parameter G%Z_ref is replaced by G%meanSL. G%Z_ref is factored in both G%meanSL and G%bathyT and it is kept as a useful consistency testing tool. Another cosmetic change is made by using G%meanSL + G%bathyT, instead of G%bathyT + G%meanSL, which (hopefully) can be easily interpreted as G%meanSL - (-G%bathyT).
| else | ||
| max_depth = diagnoseMaximumDepth(D,G) | ||
| if (present(meanSL)) then | ||
| max_depth = diagnoseMaximumDepth(D+meanSL, G) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is doing an array-syntax sum of two fields, including working in the halo regions. This is not permitted in MOM6, as is explained at https://github.com/NOAA-GFDL/MOM6/wiki/Code-style-guide#array-syntax. Please replace this with an explicit calculation of the total water column thickness in its own array.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. I think It should be noted that in this particular case, halo is explicitly excluded in the calculation diagnoseMaximumDepth. But I agree that this abundance of caution is warranted in the long run.
| if (trim(config) /= "DOME") then | ||
| call limit_topography(D, G, PF, max_depth, US) | ||
| if (trim(config) /= "DOME") then | ||
| ! MAXIMUM_DEPTH is not set and topogrpahy does not need to be trimmed by its maximum depth. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"topogrpahy" should be "topography".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Hallberg-NOAA
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my view, this PR will be ready to go once the instance of an inappropriate use of array syntax math (noted in a specific comment on the line in question) has been addressed.
max_depth is really used as a maximum static thickness throughout the model, so meanSL needs to be considered.
This PR adds a new 2D field
meanSLto grid control structure.meanSLis primarily used to handle a domain with spatially uneven long term mean surface level, with the Great Lakes being a prime example.bathyT,meanSLis referenced toz_refinocean_grid_type(and to zero indyn_horgrid_type).meanSLadopts height unit and is positive abovez_refor zero.meanSLis NOT replacingz_ref.z_refcan still be used as a uniform reference height for debugging/testing.z_refinto as a 2D field, topography will have to change to column thickness, which may not be very user-friendly._init, so this is probably not a lot of extra work. The exception isfind_face_areaunder certain configurations in barotropic solver.meanSLis uniformly zero (orz_refinternally), so there is no answer change.Edit: sorry for claiming the #1000 issue/pr/discussion.