-
Notifications
You must be signed in to change notification settings - Fork 110
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 tracer interpolation method for missing levels when reading grib2 input #556
Add tracer interpolation method for missing levels when reading grib2 input #556
Conversation
This is related to issue #555 |
Not sure how to solve this test error: /home/runner/work/UFS_UTILS/UFS_UTILS/ufs_utils/sorc/chgres_cube.fd/input_data.F90:6717: error: Member dint2p(PPIN, XXIN, NPIN, PPOUT, XXOUT, NPOUT, LINLOG, XMSG, IER) (function) of module input_data is not documented. (warning treated as error, aborting now) dint2p is a function added for pres to pres interpolation. The compiling and testing are good on Hera though. |
@JiliDong-NOAA That is a doxygen error. All new routines must be documented with doxygen. |
@JiliDong-NOAA We should add a unit test for this new routine. I can help with that. |
Thanks @GeorgeGayno-NOAA . Doxygen documentation is added to dint2p and all checks have passed. |
@GeorgeGayno-NOAA Please let me know how I should go forward with the unit test as I have no idea how to do that. |
I can help with a unit test when I return from lunch. I will create a simple test and check it into your branch. Make sure I can access your fork. |
I added a unit test at c3bf3fb. You will need to add some comments to the source code. Also, you will want to improve and expand on what I did. For example, the routine has different interpolation options - linear, logp, extrapolation. You want to test each option. |
Thanks @GeorgeGayno-NOAA I will work on it. How would I run the unit test? Using a single job card for "srun ftst_dint2p"? |
You can run it on Hera, Jet or Orion. First, to build_all.sh, add 'make test' after the 'make install'. Then, under ./cmake, edit the mpiexec file for your machine for an account and queue you are eligible to use. That will automatically run all the tests. Log files from the tests will be under ./build/Testing/Temporary. |
@JiliDong-NOAA Since you are adding a new option, you will need to update the related documentation: |
dint2p unit test successful after a bug is fixed when using lnP extrapolation (the routine won't use extrapolation option when being called but it is safe to get the big fixed). The unit test consider the four scenarios:
There are several unit tests failed which seem not related to my change: The following tests FAILED: |
@JiliDong-NOAA Don't worry about the tests that failed on Hera. You did not break anything. |
related documentation updated |
I made one minor change to move zeroing out tracer to the end of filling in default value. Otherwise it may zero out pre-assigned missing value of -999 before we use it to determine whether to fill in. @LarissaReames-NOAA could you review it? Other than that, it looks good to me. |
Ah, yep, great catch. That looks good to me. |
Thanks @LarissaReames-NOAA. I am doing consistency tests on Hera but unable to get consistency.log and summary.log. The individual log files (consistency.log[01-16]) seem ok. Anything I need to change to get those two summary log files? |
That's odd. The summary file should be produced so long as all the individual tests complete successfully. I usually only fail to get a summary file when a test fails before it can test for file consistency, so the chgres_cube code fails during execution. I just ran the tests on the most recent code and I get the summary file just fine. You can see the output here: |
@LarissaReames-NOAA Now I see where the problem is. I didn't change HOMEreg to /scratch1/BMC/gsd-fv3/Larissa.Reames/chgres_cube/reg_tests. After changing that, I can get the summary.log. Noticed several reg test failures 08, 09 and 14 though. I guess the reg tests will be updated later. |
All unit and consistency tests pass on Hera and Jet (other than the expected failures for previous tests using GFS grib2 data). @GeorgeGayno-NOAA can you help run tests on Orion and WCOSS? |
The tests that failed - will the baseline data need to be updated? |
Yes, there should be three failures -- all the old tests that used GFS grib2 data. They'll need to be updated, and the change is due to the update of all variable mapping tables to interpolate scattered missing levels. |
I ran the tests on WCOSS-Dell. I got four failures, not three:
|
Ah yes, well that last one will fail if you haven't already created the correct baseline file for the system since it's new in this PR. So the first three will need new baselines, and the last one (test 16) will just need to have whatever file you get from the test itself inserted as the standard baseline result. |
Oh, I forgot you added a new test. Is this the input data for it? |
Yep, that's the one. |
@JiliDong-NOAA I found some bugs in some of the regression test scripts. I will check in the fixes to your branch. |
The consistency tests were run on all Tier 1 machines - Jet, Orion, Hera, WCOSS-Cray and WCOSS-Dell. The baseline data was updated for the three tests that were affected by changes under this PR. And input and baseline data was added for the new consistency test. |
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 have no further comments.
Are we ready to merge? |
I believe so. Thanks for your help with this PR. |
A new method ("intrp") is added for tracers to interpolate to the missing isobaric levels for grib2 input. This pressure to pressure interpolation is mostly to handle the combined GFSv16 grib2 files where specific humidity has missing levels all through the depth and assigning a small constant value (e.g. 1E-7) in the whole column is not always desirable particularly in mid to lower troposphere.
The interpolation is a simple linear one in log(P) coordinate.
To use the option, just replace spfh method from "set_to_fill" to "intrp" in varmap. As this is a new option, it is not expected to change/break any current existing applications if "intrp" is not used.
Fixes #555