-
Notifications
You must be signed in to change notification settings - Fork 156
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
Adding the dry-bulb air temperature and tv flag in the observation diagnostic file of conventional Q obs (for 3D RTMA run only) #688
Adding the dry-bulb air temperature and tv flag in the observation diagnostic file of conventional Q obs (for 3D RTMA run only) #688
Conversation
@MatthewMorris-NOAA @xyzemc and @guoqing-noaa Could you review this PR? Thanks. |
Hi, @MatthewMorris-NOAA @ManuelPondeca-NOAA @xyzemc @guoqing-noaa @ShunLiu-NOAA,
Now you may start to review the updated code. |
sure |
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.
The code changes look good to me.
remote-tracking branch 'upstream/develop' into feature/tdry_in_qob_diag_rtma
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 good Gang!
Hi @AnnetteGibbs-NOAA @MatthewMorris-NOAA, Thank you! |
Thank you, @AnnetteGibbs-NOAA ! |
with my latest updated code (for this PR#688), new ctest (regression tests) running Hera:
|
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 good!
into feature/tdry_in_qob_diag_rtma
This morining EMC GSI was updated with the latest commit #a898668. So I updated my fork of GSI and all the branches, including working branch "feature/tdry_in_qob_diag_rtma" to the current branch head of develop. As for now, my working branch for this PR #688 is 5 commits ahead of develop branch of EMC GSI. Since the changes of commit#a898668 were related to the build files of GSI, there is No any changes to source files. So I believe after I updated to current head of develop branch, my code should be good enough, there is no need for further peer review and ctest. But if the ctests are still necessary, please let me know. Thank you! |
ctest (7 regression tests) were done today on wcoss2 and hera with the latest updates (after merging latest EMC GSI commit#a898668 into working branch "feature/tdry_in_qob_diag_rtma" (commit #b11bedd) of my personal fork). All the regression tests were passed successfully.
on hera:
|
@GangZhao-NOAA Could you sync this PR with develop and do regression tests. I think this PR is ready for merging. Thanks, |
into feature/tdry_in_qob_diag_rtma
@hu5970 Hi, Ming, Following your suggestion, I merged the latest EMC GSI develop branch (commit #bae0342f) into my working branch "feature/tdry_in_qob_diag_rtma" (the current commit #26a5401). So now it is updated to the current head of branch develop. Then I ran the ctest (regression tests) on Hera and WCOSS2 (Dogwood). All the 7 tasks passed successfully on both machines. Here are the test results: on Hera:
On WCOSS2 (Dogwood):
|
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.
The code looks good.
Two reviewers approved this PR. All regression test cases passed.
Two minor comments @GangZhao-NOAA . You don't need to change code in this PR. My comments are for future PRs. Question: Have you or others confirmed that the additional information written to the binary and netcdf diagnostic files is does not cause any problems for downstream codes, specifically observation monitoring codes. Related question: have we confirmed that the additional information written to the diagnostic files is correct? I ask these questions since none of our ctests check or use the diagnostic file created by the updated |
@RussTreadon-NOAA , Hi, Russ, Thank you! -Gang |
Good to get confirmation that the 2DRTMA diagnostic file is correct. Hopefully the same remark applies to a 3DRTMA run using the changes in this PR. |
|
DUE DATE for merger of this PR into
develop
is 3/1/2024 (six weeks after PR creation).Description
Motivation:
In the Auto-QC utility of 3D-RTMA, the dry-bulb air temperature (hereafter as T_dry) which is accompanied with moisture observation is required in the procedure of the quality control for conventional moisture observations (Q obs). Since this auto-qc of 3DRTMA retrieves many information from the observation diagnostic files (output of GSI run), it should be convenient for the sake of auto-qc of Q obs, if the required T_dry could be output into the observation diagnostics file of Q obs in the GSI run.
Modifications to the code:
read_prepbufr.f90:
for qob and rtma run, add lines to use tpc to identify the sensible temp and virtual temp, and mark with tvflag, then get tdry with the temp obs in different ways w.r.t the associated tvflag;
setupq.f90:
add line to use l_rtma3d from module rapidrefresh_cldsurf_mod
variable nreal (the length of obs diag information record for each obs) needs to be increased by 2 (for tdry and tvflag, when running GSI for 3DRTMA only)
in subroutine contents_binary_diag_, add line to put tdry into the array rdiagbuff (for binary format obsdaig file)
in subroutine contents_netcdf_diag_, add line to put tdry into metadata (for netcdf format obsdiag file)
This PR is to address the issue #666 : Adding the (calculated) dry-bulb temperature in the observation diagnostic file for conventional Q obs (only for 3D RTMA)
Fixes #666
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Checklist
[*] My code follows the style guidelines of this project
[*] I have performed a self-review of my own code
[*] I have commented my code, particularly in hard-to-understand areas
[*] New and existing tests pass with my changes
[*] Any dependent changes have been merged and published
[*] Regression Test of GSI
In the observation diagnostic file for q-obs generated by GSI, T_dry and tvflag were added for each record of Q obs in binary format diagnostic file; and in netcdf-4 format diagnostic file, new metadata variable "Observation_Tdry" and "Setup_QC_Mark" (used for tvflag) were added in the metadata group. The other variables in obsdiag file are still as same as in the obsdiag file from original GSI code. It indicates that the modification does not affect the other variables in the obsdiag files. And in the test run with l_rtma3d=.false. (this is the GSI run not for 3D RTMA), then in the obsdiag file for Q obs, there is no Tdry and tvflag neither in binary format nor in netcdf-4 format, this is also as expected that this modifications should have no influence for other GSI applications. And the most impratant thing is, the analysis, minimisation information, obs-fitting stats from the control run (using original code without adding tdry) and testing run with new code adding tdry are identical. So the modified code have no influences to the analysis.