-
Notifications
You must be signed in to change notification settings - Fork 33
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
Add soil temperature and moisture IAU #222
base: ufs/dev
Are you sure you want to change the base?
Conversation
…into feature/lnd_iau * 'feature/lnd_iau' of https://github.com/tsga/ccpp-physics: correct the meta file correctly convert from flashes per five minutes to flashes per minute In physics/Interstitials/UFS_SCM_NEPTUNE/maximum_hourly_diagnostics.F90, scale lightning threat from flashes per 5 minutes to flashes per minute to match units in metadata In physics/Interstitials/UFS_SCM_NEPTUNE/maximum_hourly_diagnostics.meta: change units 'flashes 5 min-1' to 'flashes min-1' and update long name to make clear this is per 5 minutes
* tmp: fix arg_table_noahmpdrv_finalize
removed pointer attribute from "input_nml_file" declaration
remove commented out old lines
get rid unused var input_nml_length
comment out unused number of freeze/thaw counters
fix typo
make long names consistent with naming conventions
|
||
private | ||
|
||
real(kind=kind_phys), allocatable :: wk3_stc(:, :, :, :), wk3_slc(:, :, :, :) |
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.
@tsga @dustinswales @climbfuji I've made modifications to move the module-defined DDTs to GFS_typedefs (in the FV3 host). I haven't tested/pushed the changes yet. The last bit that could potentially cause a problem is with these variables wk3_stc, wk3_slc, wk3_slmsk
. They are read in during the init phase and used throughout the simulation in the timestep_init phase, so, basically like LUTs, but they are functions of time and x,y,z. I suppose that it's possible that different instances of the CCPP could read in different files and therefore these variables would be different across instances and therefore would need to get moved to the host as well. @tsga What do you think? Do you think that somewhere down the line (in potentially a different host model), that these variables would be different for each instance?
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.
@grantfirl yes, different instances are likely to have different wk3_stc and wk3_slc (but not wk3_slmsk). Can these also be read by the host at init and passed through to noahmpdrv by the host? (The size of the time dim is often 3, so size of wk3_slc ~ 3n_xn_y*4. z=4 is for soil layers). Alternatively, I can modify lnd_iau_mod.F90 to read the values from file (during timestep_init) then interpolate and store intermediate values in "Land_IAU_state". BTW, is it correct to assume that Land_IAU_state is not affected by the memory issue?
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.
See tsga#1. Any module-level variables that need to persist when descoped need to be declared by the host unless those variables can effectively be shared across instances. Since the wk3*
variables just contain the data read in by the host in the sfc_inc files (correct?), I'm not sure that they would need to be declared by the host. I.e., if different instances read in different sfc_inc files, then, yes, but otherwise, I think no.
|
||
! Read all increment files at iau init time (at beginning of cycle) | ||
! allocate (wk3_stc(n_t, 1:im,jbeg:jend, 1:km)) | ||
call read_iau_forcing_fv3(Land_IAU_Control, errmsg, errflg) !, wk3_stc, wk3_slc |
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 didn't address this in tsga#1, but the code isn't checking for errors after returning from read_iau_forcing_fv3
. When I try to run the new test with the data missing, instead of exiting with a nice error message, you get a seg fault because this code just continues and tries to access variables that were never allocated when the data files are missing.
return | ||
end if | ||
! Read Land IAU settings | ||
call land_iau_mod_set_control(Land_IAU_Control, fn_nml, input_nml_file, & |
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.
IMO, it's a little awkward that the control for this functionality isn't read in until the physics init time. This means that every physics suite that uses NoahMP will at least have to go into the land_iau_mod_set_control
routine to read the namelist. It would probably be cleaner to at least read in the do_land_iau variable in the host so that the physics can avoid doing anything with land IAU when it's not being used. I'm not aware of other physics schemes that force the host to read a namelist within the physics in order to know whether an option is used or not. I know that there is a GWD scheme that has a separate nml, but I think that there is also a namelist variable in the "main" physics namelist that controls whether the scheme namelist is even read. @dustinswales Do you have an opinion here?
|
||
! real (kind=kind_phys), dimension(max_soiltyp) :: maxsmc, bb, satpsi | ||
! real, dimension(30) :: maxsmc, bb, satpsi | ||
real(kind=kind_phys), parameter :: tfreez=273.16 !< con_t0c in physcons |
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.
Why are these physical constants being redefined here?
Move DDTs to host and fix compilation errors
Add Incremental Analysis Update (IAU) to soil temperature and moisture in the NoahMP land model of the UWM's CCPP. Addresses issue: /issues/190
Regression tests for ATM only and coupled (S2S) configurations pass in hera.
Additional land-IAU specific test to be added: control_c48_intel_land_iau (based on the "control_c48_intel" test, with the only difference being the land IAU increment files added to the INPUT directory (sample increment files at ./scratch2/NCEPDEV/land/data/DA/soil_moisture/LandIAU/INC/C48/INC09/).
The input.nml is modified to add "land_iau_nml" (do_land_iau=.true. if executing with land IAU)