-
Notifications
You must be signed in to change notification settings - Fork 153
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
Address Issue #863 (setting surface-related interstitial variables for SCM prescribed-surface-flux mode) #870
Conversation
grantfirl
commented
Mar 3, 2022
- Fixes scm_sfc_flux_spec needs to set the dry, icy, etc. interstitial variables #863
- Changes answer for prescribed-surface flux SCM cases
- Will not affect FV3/UFS at all
- Copy some of the code from GFS_surface_composites_pre_run into scm_sfc_flux_spec_run for setting some surface-related interstitial variables that are used in non-surface-related schemes after scm_sfc_flux_spec_run
- This requires executed scm_sfc_flux_spec_run prior to dcyc2 in the SDFs to work
@climbfuji Since this won't change the answer for UFS RTs, is there any way that this can be combined with Jeff Beck's bugfix for SPP and merged with NOAA-EMC/fv3atm#487? |
Fine with me. Note please that I will be on "NOAA leave" the entire next week March 7-11, so either this gets done tomorrow or from the 14th on. |
OK, understood. I'd be surprised if your PRs go in tomorrow since there is supposed to be one in between NSSL and yours. Depending on what happens tomorrow, maybe I'll see if I can get this in separately or as part of something else next week. |
@@ -135,7 +144,82 @@ subroutine scm_sfc_flux_spec_run (u1, v1, z1, t1, q1, p1, roughness_length, spec | |||
t2m(i) = 0.0 | |||
q2m(i) = 0.0 | |||
end do | |||
|
|||
|
|||
!GJF: The following code is from GFS_surface_composites.F90; only statements that are used in physics schemes outside of surface schemes are kept |
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 statement is a little confusing, I had to read it a few times. Generally, is it a good idea to copy & paste this kind of code which is subject to frequent changes and very difficult to keep in sync manually?
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.
I don't really have another solution, though. Except creating dummy variables for the surface physics part and calling into GFS_surface_composites directly, but this even worse.
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.
No, I agree that it is not a good idea. There are other places in the code (like the vertical grid) that rely on portions of the FV3 dycore that also require 1) recognition that something has changed in the "parent" code 2) time/motivation to propagate those changes to the SCM. It's definitely proved to be an ongoing maintenance item to stay in-sync. Another alternative is to move some of these variables to a persistent data type and set them once at initialization. Is one column really changing from dry -> wet during a run? Perhaps sea ice changes during a run.
Also, I will reword the comment to (hopefully) be less confusing.
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.
I don't know enough about these physics to have any deep insights, but the code looks good. I just had one question.
@@ -72,14 +79,16 @@ subroutine scm_sfc_flux_spec_run (u1, v1, z1, t1, q1, p1, roughness_length, spec | |||
|
|||
real(kind=kind_phys) :: rho, q1_non_neg, w_thv1, rho_cp_inverse, rho_hvap_inverse, Obukhov_length, thv1, tvs, & | |||
dtv, adtv, wind10m, u_fraction, roughness_length_m | |||
|
|||
real(kind=kind_phys), parameter :: timin = 173.0_kind_phys ! minimum temperature allowed for snow/ice |
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.
Is there a reason this is is hard-coded rather than being treated like other constant variables in the physics schemes? (e.g. "tgice")
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.
Good question. This change was copied from GFS_surface_composites_pre_run:
real(kind=kind_phys), parameter :: timin = 173.0_kind_phys ! minimum temperature allowed for snow/ice |
I have no idea why it was coded that way in that file, but since I'm copying the functionality from that file into this one, I copied this hard-coding as well.