-
Notifications
You must be signed in to change notification settings - Fork 161
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
Enable customizable idealized, doubly-periodic tests #909
base: develop
Are you sure you want to change the base?
Enable customizable idealized, doubly-periodic tests #909
Conversation
…into feature/nsslmicro
…e/nsslmicro-merge
… match allocation/use
Feature/nsslmicro merge gjf
…NOAA/fv3atm into feature/ideal-new
…into feature/ideal-new
Included ccpp suite that turns off pbl and surface layer schemes for better idealized testing.
Eighth reconciliation PR from production/RRFS.v1 (NOAA-EMC#898)
This PR doesn't add any new subroutines or inputs for routines contained in fv3atm, but I did add an if statement branch specifically for idealized test cases (grid_type=4) in module_fcst_grid_comp.F90 that would probably need a unit test. I don't think this file is covered by any unit tests as of yet, @edwardhartnett how do you want to proceed here? Do we need a unit test that test all branches of that if block? |
Don't worry about the unit testing yet. |
module_fcst_grid_comp.F90
Outdated
!TODO: Consider aligning mask treatment with coordinates... especially if it requires updates for moving | ||
call addLsmask2grid(grid, rc=rc) | ||
if (ESMF_LogFoundError(rcToCheck=rc, msg=ESMF_LOGERR_PASSTHRU, line=__LINE__, file=__FILE__)) return | ||
if (.not. (Atmos%grid_type==4)) then |
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 would write this if tests as:
if (Atmos%grid_type /= 4) then
Also, why grid with grid_type == 4 does not need grid coordinates, as other grid types? Is grid_type == 4 special in that regard?
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.
So originally this was bypassed because the ESMF coordinates have no bearing when the write component grid is never used, and it can't be used with these doubly periodic domains as they can have unrealistic (e.g., uniform) lat/lon configurations. However, I've pushed a fix that creates a single tile non-periodic ESMF grid for grid_type==4, and this now fills the ESMF coordinates correctly even if they're never used here.
module_fcst_grid_comp.F90
Outdated
gridfile = "grid_spec.nc" ! default | ||
|
||
if (open_file(fileobj, "INPUT/grid_spec.nc", "read")) then | ||
if (variable_exists(fileobj, "atm_mosaic_file")) then | ||
call read_data(fileobj, "atm_mosaic_file", gridfile) | ||
endif | ||
call close_file(fileobj) | ||
endif |
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.
Looks to me gridfile
is not used anymore, other than in the error message below. If it's not, it should be removed.
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 block has been removed.
module_fcst_grid_comp.F90
Outdated
if (ESMF_LogFoundError(rcToCheck=rc, msg=ESMF_LOGERR_PASSTHRU,line=__LINE__, file=__FILE__)) return | ||
call mpp_error(NOTE, 'after create fcst grid for doubly periodic with INPUT/'//trim(gridfile)) | ||
|
||
call addLsmask2grid(grid, rc=rc) |
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 this call needed here? There is already the same call on line 306 below.
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.
Call removed.
module_fcst_grid_comp.F90
Outdated
call ESMF_InfoGet(info, key="nx", value=nx, rc=rc) | ||
if (ESMF_LogFoundError(rcToCheck=rc, msg=ESMF_LOGERR_PASSTHRU, line=__LINE__, file=__FILE__)) return | ||
call ESMF_InfoGet(info, key="ny", value=ny, rc=rc) | ||
if (ESMF_LogFoundError(rcToCheck=rc, msg=ESMF_LOGERR_PASSTHRU, line=__LINE__, file=__FILE__)) return | ||
grid = ESMF_GridCreateNoPeriDim( regDecomp=(/layout(1),layout(2)/), & | ||
minIndex=(/1,1/), & | ||
maxIndex=(/nx,ny/), & | ||
gridAlign=(/-1,-1/), & | ||
coordSys=ESMF_COORDSYS_SPH_RAD, & | ||
coordTypeKind=grid_typekind, & | ||
decompflag=(/ESMF_DECOMP_SYMMEDGEMAX,ESMF_DECOMP_SYMMEDGEMAX/), & | ||
name="fcst_grid", & | ||
indexflag=ESMF_INDEX_DELOCAL, & | ||
rc=rc) |
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 block looks to be identical to the code in else
block below. If it is indeed identical, these two cases should be combined into one.
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.
Logic has been combined for the two cases.
Description
This PR adds a doubly-periodic idealized test case to test_cases.F90 that is customizable by the user at run time via input.nml and that can be run within the UFS framework and is fully compatible with CCPP physics.
An important modification was made to make this case compatible with CCPP. To enable the user to choose the canned "SupeCell" sounding from GFDL's original supercell test, we copied over the routines from GFDL MP that were previously preventing this code from compiling with CCPP/"GFS physics".
We have added documentation to help the user, including some code to generate an idealized surface file that the user can customized to their liking. The PR at the UWM level will also have a corresponding regression test. It won't be baseline tested, but it will provide an easy route for user to generate a run directory.
Many thanks go to @MicroTed for all of his help and contributions to this effort as well. He's agreed to provide a test run directory repo that can be cloned by those who'd like to test the code inside a pre-generated run directory. You can find that here: https://github.com/MicroTed/ufs_ideal_run.git
Issue(s) addressed
Testing
Tested on Hera (intel and gnu, release and debug builds for both) and on Jet. Tests have been run to check that all the various types of sounding options can be used successfully.
The full suite of regression tests will be run at the weather model level. A regression test will be added in the WM.
No existing baseline changes are expected.
Dependencies
Requirements before merging