Convert real(kind_phys) vegetation, slope and soil type arrays into integer arrays #730
Conversation
…ace_generic_post so that all output files are 100% identical
…r surface physics
climbfuji
requested review from
grantfirl,
JulieSchramm and
llpcarson
as code owners
climbfuji
requested review from
SMoorthi-emc,
junwang-noaa and
ligiabernardet
climbfuji
changed the title
…tation_soil_slope_type_integer
…tation_soil_slope_type_integer
…tation_soil_slope_type_integer
|
@yangfanglin @SMoorthi-emc @HelinWei-NOAA @tanyasmirnova @junwang-noaa @DusanJovic-NOAA @grantfirl @ligiabernardet @llpcarson - the next PRs to go into the ufs-weather-model, fv3atm and ccpp-physics are to convert #730 I had opened these PRs for reviews a while ago, but haven't received any feedback yet. The way it is implemented now is to make sure that there are no changes to the input and output streams in the UFS, and that all regression tests pass with b4b identical results. If, in the future, someone wants to use integer data in the input and output streams instead of real data, then this can be implemented easily. Can I please ask you to review my PRs and provide feedback? Please make sure to read the description in each of them, as they describe in detail what has been done and why. Thanks very much in advance! |
…tation_soil_slope_type_integer
|
|
||
| !$OMP parallel do num_threads(nthreads) default(none) private(i) & | ||
| !$OMP shared(im, isot, ivegsrc, islmsk, vtype, stype, slope) | ||
| do i=1,im |
There was a problem hiding this comment.
This doesn't have anything to do with this PR, but I'm curious why this logic is necessary (temporarily changing soil, vegetation, and slope for the case of sea ice). Is this a workaround because the soil, vegetation, and slope datasets don't have entries for sea-ice, so this is the next best way to handle that? This is probably a stupid question, but could this logic be avoided by adding values to the soil, vegetation, etc. datasets for sea-ice?
There was a problem hiding this comment.
I think that this data comes from the initial conditions. These have zeros for veg/soil/slop types over ice, but internally we need to match them with what the LSM lookup tables want.
| stype_save(:) = stype(:) | ||
| slope_save(:) = slope(:) | ||
|
|
||
| call update_vegetation_soil_slope_type(nthreads, im, isot, ivegsrc, islmsk, vtype, stype, slope) |
There was a problem hiding this comment.
Why are soil and vegetation types get updated?
There was a problem hiding this comment.
Maybe the name is misleading, as you figured out yourself below, it's just filling in water and ice values for slope/veg/soil type, because the input data only has zeros. If you don't do this, then RUC LSM for example will crash, because it will look for vegtype(0) at some place (which is out of bounds).
| !$OMP end parallel do | ||
|
|
||
| end subroutine update_vegetation_soil_slope_type | ||
|
|
There was a problem hiding this comment.
it looks like update_vegetation_soil_slope_type just fills in the values for water and ice?
There was a problem hiding this comment.
Correct - should I rename the subroutine?
| vtype(:) = vtype_save(:) | ||
| stype(:) = stype_save(:) | ||
| slope(:) = slope_save(:) | ||
|
|
There was a problem hiding this comment.
not clear why we need to return to original values with zero values for water, etc.?
There was a problem hiding this comment.
So that the output that gets written to disk is b4b identical with the current code. Otherwise there would be 17s, 9s etc instead of zeros. It wouldn't affect any ufs restartt run, but I don't know about other downstream applications.
There was a problem hiding this comment.
ok, thank you for the explanation.
| ! integer to real/double precision | ||
| slpfcs = real(slope) | ||
| vegfcs = real(vtype) | ||
| sltfcs = real(stype) |
There was a problem hiding this comment.
not clear why we have to do conversion to double-precision real.
There was a problem hiding this comment.
Because sfcsub.F expects real arrays and not integer arrays, unfortunately. Changing sfcsub.F to use integers for these arrays would be a lot of work.
There was a problem hiding this comment.
@climbfuji Dom, thank for answering the questions. It makes sense to me now.
| ! Save current values of vegetation, soil and slope type | ||
| vtype_save(:) = vtype(:) | ||
| stype_save(:) = stype(:) | ||
| slope_save(:) = slope(:) |
There was a problem hiding this comment.
@climbfuji Can you clarify when the GFS_surface_generic_pre_init will be called and when the GFS_surface_generic_pre_run will be called? I assume they can't be called together.
There was a problem hiding this comment.
GFS_surface_generic_pre_init will be called during the physics initialization phase, look for
! Initialize the CCPP physics
call CCPP_step (step="physics_init", nblks=Atm_block%nblks, ierr=ierr)
if (ierr/=0) call mpp_error(FATAL, 'Call to CCPP physics_init step failed')
in atmos_model.F90, around line 709 in subroutine atmos_model_init.
GFS_surface_generic_pre_init will be called in each time step, look for lines
call mpp_clock_begin(physClock)
call CCPP_step (step="physics", nblks=Atm_block%nblks, ierr=ierr)
if (ierr/=0) call mpp_error(FATAL, 'Call to CCPP physics step failed')
call mpp_clock_end(physClock)
in atmos_model.F90, around line 340 in subroutine update_atmos_radiation_physics.
GFS_surface_generic_pre_init and GFS_surface_generic_post_init will thus be called only once at the beginning of the run, while GFS_surface_generic_pre_run and GFS_surface_generic_post_run are called at every time step.
To be clear, that whole complicated code around saving the original values of soil/vegetation/slope type in the pre step and restoring them in the post step is only so that the output doesn't change compared to the current code. If I was allowed to change the output, i.e. have the update soil/vegetation/slope values over ice and water in the output files instead of zeros like in the initial conditions, this code would be a lot easier, faster, and would require less memory.
…ce code and metadata files
Description
This PR removes all persistent Fortran "real" vegetation, slope and soil type arrays and replaces them with the existing (interstitial) "integer" arrays (these become persistent).
There are code changes that need to be discussed, because they are only required for achieving 100% identical output. Without these code changes (related to adding the
vtype_saveetc variables), the model execution is still giving the same results (in terms of checksums at the end of the model runs), but thevtype,stype,slopedata in the surface restart/diag files are different. This is because of code that was previously called inGFS_surface_generic_pre_runand was duplicated insfc_drv_ruc_initand that modified the interstitial variables (but not the persistent variables). Using the_saveroutines makes it possible to get the same (b4b) data in all output files.The code that modifies the
vtypeetc. variables is now contained in one subroutine that gets called byGFS_surface_generic_pre_initandGFS_surface_generic_pre_runand no longer insfc_drv_ruc_init, which is much more consistent and less confusing (all code in one place).Changes in this PR:
GFS_surface_generic_prethat now gets called only byGFS_surface_generic_pre_initandGFS_surface_generic_pre_run, no longer bysfc_drv_ruc_initAdditional last minute changes (no code changes, no effect on results etc):
Issues
Fixes #704
Associated PRs
#730
NOAA-EMC/fv3atm#388
ufs-community/ufs-weather-model#804
Testing
For regression testing, see ufs-community/ufs-weather-model#804