ufs-community / UFS_UTILS

Utilities for the NCEP models.
Other
21 stars 105 forks source link

chgres_cube: Erroneous soil temperatures at isolated locations when using GEFS grib2 data #447

Closed LarissaReames-NOAA closed 3 years ago

LarissaReames-NOAA commented 3 years ago

GEFS.v12 grib2 data had a known bug where soil temperature at isolated locations (I counted 176 points on the global grid in some sample data) over land had missing values (9E20) as if they were sea points. These values on the GEFS grid resulted in similarly erroneous temperatures on the output grid (I found 2 such points on a 3km North American target domain) , which caused weather model failure. These incorrect values of soil temperature have been fixed in GFSv16 and a patch has been submitted to NCO for future GEFS v12.1.1 implementation. However, these erroneous data need to be accounted for to allow users to use data prior to the patch.

The proposed code fix is to replace all levels of soil temperature at these locations (i.e., soil temperature > 350.0 K at landmask = land) with skin temperature. This will be performed on the input grid in input_data.F90 in subroutine read_input_sfc_grib2_file. @JacobCarley-NOAA has approved this solution as a fix to make the code backwards compatible with GEFS.v12 grib2 files.

Contributors: @JamesAbeles-NOAA @christinaholtNOAA

GeorgeGayno-NOAA commented 3 years ago

Do you plan to add this fix to the input routine? That is what I would recommend.

Another option is to check the soil temperature at the eight neighboring points and pick a valid one. If none are valid, use the skin temperature.

LarissaReames-NOAA commented 3 years ago

Do you plan to add this fix to the input routine? That is what I would recommend.

Another option is to check the soil temperature at the eight neighboring points and pick a valid one. If none are valid, use the skin temperature.

See this part of the issue description

This will be performed on the input grid in input_data.F90 in subroutine read_input_sfc_grib2_file

edwardhartnett commented 3 years ago

How shall this change be tested?

How big is the input file? We can put it on the FTP site and fetch it for tests...

LarissaReames-NOAA commented 3 years ago

@edwardhartnett The test to see if it works should be a simple one: if you process GEFS.v12 grib2 files (with "GFS" as the input to external_model) on to a North American (or global) domain and no grid points exist where soil temperature is very large (> 350K) at a land point, then the fix has succeeded. It would also successfully initialize a weather model forecast, whereas files with these erroneous soil temperatures would cause model failure. Is this what you're looking for?

GeorgeGayno-NOAA commented 3 years ago

Do you plan to add this fix to the input routine? That is what I would recommend. Another option is to check the soil temperature at the eight neighboring points and pick a valid one. If none are valid, use the skin temperature.

See this part of the issue description

This will be performed on the input grid in input_data.F90 in subroutine read_input_sfc_grib2_file

Sorry. That is what happens when I read too fast.

edwardhartnett commented 3 years ago

@LarissaReames-NOAA thanks for answering. No, these are not what I am looking for. ;-)

When I say "test" I mean a test program that tests input_file without running the entire system. These tests are separate fortran codes that do nothing else except test the input routine. In other words, unit tests.

Unit tests are required for all new contributions or changes to chgres_cube, so plan on writing the test code as you make the changes in the input_file test.

@GeorgeGayno-NOAA perhaps a basic input_file test would be the logical next step for testing. Then @LarissaReames-NOAA would have a test to modify, instead of having to add a new one.

GeorgeGayno-NOAA commented 3 years ago

@LarissaReames-NOAA If you add a small routine or function (called from read_input_sfc_grib2_file) that fixes the issue, I can help you write the required test.

LarissaReames-NOAA commented 3 years ago

@GeorgeGayno-NOAA Thanks, that would be really helpful. I have a single line added to that subroutine that appears to solve the issue in my fork. @JamesAbeles-NOAA was able to run a successful forecast using the newly-generated surface file yesterday.

LarissaReames-NOAA commented 3 years ago

@LarissaReames-NOAA If you add a small routine or function (called from read_input_sfc_grib2_file) that fixes the issue, I can help you write the required test.

@GeorgeGayno-NOAA I've now moved that whole section of code to its own subroutine, and the results are the same as they were before I moved the code. So I guess now I'll need to figure out how to write a unit test for it.

GeorgeGayno-NOAA commented 3 years ago

@LarissaReames-NOAA If you add a small routine or function (called from read_input_sfc_grib2_file) that fixes the issue, I can help you write the required test.

@GeorgeGayno-NOAA I've now moved that whole section of code to its own subroutine, and the results are the same as they were before I moved the code. So I guess now I'll need to figure out how to write a unit test for it.

I can help you get started. Will you allow me to update your branch?

LarissaReames-NOAA commented 3 years ago

@GeorgeGayno-NOAA Sure, I've added you as a collaborator on my fork.

GeorgeGayno-NOAA commented 3 years ago

Please merge in the latest updates from 'develop', which will make testing easier.

LarissaReames-NOAA commented 3 years ago

@GeorgeGayno-NOAA Done. I also added in my first attempt at a test for the subroutine. I tried to base it off your test for the search utility. It compiles on Hera, but I haven't tried running it yet.

GeorgeGayno-NOAA commented 3 years ago

@GeorgeGayno-NOAA Done. I also added in my first attempt at a test for the subroutine. I tried to base it off your test for the search utility. It compiles on Hera, but I haven't tried running it yet.

Ok. I have my test working on Hera. If you want to see what I did, then go to /scratch1/NCEPDEV/da/George.Gayno/ufs_utils.git/UFS_UTILS.larissa and do a 'git diff'.

GeorgeGayno-NOAA commented 3 years ago

Also, in your check_soilt routine, the soilt array is declared as (k,i,j), but the routine loops over (i,j,k).

LarissaReames-NOAA commented 3 years ago

@GeorgeGayno-NOAA Yes I had just noticed that before you posted. Thanks for finding that. The main substantive difference between your routine and mine is mine checks the whole content of the final array for correctness instead of individual points. I also included a point over land with an acceptable soil temperature to check that it wasn't being replaced, and wrote in some text output in the even that the test fails. My changes are pushed to my fork and my version of the test (in ftst_sfc_input_data) now passes. I went ahead and named the file appropriately for the coming change to splitting input_data.F90 in to three parts.