-
Notifications
You must be signed in to change notification settings - Fork 108
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 chgres_cube support for regional model (HRRR, RAP, and NAM) grib2 files and UFS_UTILS build support for Cheyenne, Stampede2, and Odin #146
Add chgres_cube support for regional model (HRRR, RAP, and NAM) grib2 files and UFS_UTILS build support for Cheyenne, Stampede2, and Odin #146
Conversation
… for sigio and sfcio
…te to find_package calls
ufs_release_1.0: cleanup work 2020/01/06
… for macOS with clang compilers
Catch unimplemented build method, add support for macOS compilers
As per conversation with Arun, since this affects the ufs_release_v1.0 branch only and not the main develop branch, I am going to merge this PR to allow the CIME people to progress with their work.
ufs_release_v1.0: re-add cmake module
Add macOS/clang support
….0' into ufs_release_v1.0
…UTILS into ufs_release_v1.0
Ufs release v1.0
…no, chgres_cube grib2 GFS data support); update submodule pointer for cmake
@GeorgeGayno-NOAA, sorry for the late reply. I was out this week on vacation. I pulled the latest develop branch code into my fork/branch, and it now points to 3eaddb8. |
@GeorgeGayno-NOAA, sure! Please point me to the 'gitdiff' file and I'll be happy to incorporate those changes. |
@GeorgeGayno-NOAA @LarissaReames-NOAA, the UFS_UTILS regression tests have passed for this PR on both Jet and Hera here: Jet: /lfs4/BMC/wrfruc/beck/UFS_UTILS_Jeff/reg_tests/chgres_cube/regression.log This leaves us with Orion and WCOSS Dell and Cray to test. |
Find the 'git diff' files on Hera: /scratch1/NCEPDEV/da/George.Gayno/ufs_utils.git/UFS_UTILS.jeffb/$machine.diff.txt. |
sorc/chgres_cube.fd/model_grid.F90
Outdated
if (mod(npets,num_tiles_input_grid) /= 0) then | ||
call error_handler("MUST RUN WITH A TASK COUNT THAT IS A MULTIPLE OF 6.", 1) | ||
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.
In this case, the number of input tiles will always be one. So this error message will never be tripped.
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.
@GeorgeGayno-NOAA, I merged in @LarissaReames-NOAA's change to remove this check in the latest commit below.
Hi @GeorgeGayno-NOAA, I just pushed a commit to add the diff files (including a minor tweak to make sure the c96 regional regression test succeeds, since the baseline files are still "tile1" instead of "tile7"). |
@@ -96,6 +98,9 @@ module surface | |||
type(esmf_field) :: terrain_from_input_grid | |||
! terrain height interpolated | |||
! from input grid | |||
type(esmf_field) :: terrain_from_input_grid_land | |||
! terrain height interpolated | |||
! from input grid at all land points | |||
|
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.
How is terrain_from_input_grid_land used? It is computed below. But I don't understand how it is used.
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.
We were experiencing issues where isolated land ice points weren't ever able to obtain a proper terrain height, so this array is passed through during the search/replace of land ice points. Since soil temperature adjustment for terrain height is not turned off when points on the target grid have the default terrain height replacement value (default_value = -99999.9), providing data interpolated to "land points" which the input data originally deemed these land ice points to be (since it doesn't have land ice) allows for these points to have a proper terrain height. It looks like this use actually got coded out, but I'm putting it back in by providing the data from terrain_from_input_grid as an optional argument to the search function on line 2049 of surface.F90.
All of the regression tests for chgres cube passed on the WCOSS Dell after I added Jeff's tile1/tile7 fix to the c96 regional regression test script. I'm waiting on the results from the WCOSS Cray but I believe they will also be successful. |
sorc/chgres_cube.fd/model_grid.F90
Outdated
if (localpet==0) then | ||
cmdline_msg = "wgrib2 "//trim(the_file)//" -d 1 -grid &> temp2.out" | ||
call system(cmdline_msg) | ||
open(4,file="temp2.out") |
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 thought this was replaced by a call to grb2_inq.
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.
Ah, it looks like you found another use that I had forgotten about. Thanks for pointing that out. I should able to replace that relatively quickly.
The chgres cube regression tests all passed on the WCOSS Cray as well. |
…ated use of terrain_from_input_grid_land to fill in missing target grid terrain height values at land ice points in the search/replace routine.
@GeorgeGayno-NOAA I just merged in @LarissaReames-NOAA's latest code fixes for the terrain and wgrib2 change. |
There is something wrong with your latest commit (3393229). I see two 'mpif.h' in model_grid.F90. I thought they were all removed. Why are they reappearing? Also, an error message removed by Larissa has reappeared. We need to stop these fork-to-fork merges. The only merges should be from/to 'develop' of the authoritative repo. |
…k-NOAA/UFS_UTILS into feature/regional_release
I'll now be merging straight to Jeff's branch. Those overwritten changes have now been put back in. |
@@ -0,0 +1,132 @@ | |||
#!/bin/bash | |||
|
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 version 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.
@GeorgeGayno-NOAA Just removed that script.
…feature/regional_release
…Reames-NOAA/UFS_UTILS into feature/regional_release
@GeorgeGayno-NOAA, the develop branch of UFS_UTILS fails all regression tests on Orion with very small (1e-10 to 1e-17 differences), likely related to the baseline files either having been created on a different platform, or by using different compilers. However, I used the develop branch regression test results as the baseline for a test with the new code, and when I do that, the regression tests pass on Orion for this PR. Therefore, all platforms (Orion, Hera, Jet, WCOSS Dell and Cray) now pass the regression tests for this PR. |
Ok. I will do the merge today. |
This PR represents required UFS_UTILS changes for the SRW App release:
Both build and run time changes have been tested successfully on Hera and Jet (so far) using the SRW App community workflow. The build has also been tested on Cheyenne, Stampede2, and Odin, with full end-to-end workflow tests to commence shortly. Testing on WCOSS is pending.
The following developers have provided code for this PR: @LarissaReames-NOAA, @JeffBeck-NOAA, @ywangwof, @gsketefian, @JulieSchramm, and @mkavulich