Feature/enkf q2 #568
Feature/enkf q2 #568
Conversation
…ve moisture values in the analysis of EnKF for FV3-LAM
…ve moisture values in the analysis of EnKF for FV3-LAM
|
Chunhua, David and Jeff, Could you please review the change of limiting the negative values of sphum (q) in EnKF analysis? Thank you. Shun |
|
In the GFS (GDAS), are negative values of sphum corrected in the code / scripts? Would the change to the EnKF code proposed here affect the GFS (GDAS)? |
|
@daviddowellNOAA In EnKF for GFS, the clipping of negative values for Q is already implemented ( in gridio_gfs.f90). |
|
Ting, thanks for proposing these code changes and testing them. As I understand things, the background specific humidity is clipped (to ensure values > 0) before the computation of virtual temperature, and then the analysis specific humidity is clipped before being output to a file. Clipping makes sense in both instances, and I also see some parallels to how things are done in gridio_gfs.f90. Looking at the code gave me some questions about future work. Meanwhile, I support the changes in the current PR. |
|
The comments below are beyond the scope of the current PR. I'm providing them here for reference in future PRs. (1) The EnKF for the global system (gridio_gfs.f90) appears to do clipping of the input values, before the analysis. Should we consider the same for regional FV3? I can see arguments for and against. (2) Our current RRFS EnKF analyses (with radar-reflectivity DA) can produce negative values of precipitating hydrometeors. Note that the clipping of precipitating hydrometeors in gridio_fv3reg.f90 only happens if l_use_enkf_directZDA is true, but l_use_enkf_directZDA is actually false in our system. In the future, we should discuss if clipping should always happen for precipitating hydrometeors, no matter what the value of l_use_enkf_directZDA is. |
|
@daviddowellNOAA Good points. I will leave 1 for future discussions but I think I can do 2 for this PR, namely, do clipping without dbz analysis being done. |
…ve moisture values in the analysis of EnKF for FV3-LAM also remove the condition of l_use_enkf_directZDA=.true. for clipping of negative values for hydrometeors when cliptracers=.true.
…tion/GSI into feature/enkf_q2
|
@daviddowellNOAA it is done as you commented , namely , changing " if (l_use_enkf_directZDA .and. cliptracers) then " to "if (cliptracers)". |
|
Thank you Jeff.
Shun
…On Fri, May 12, 2023 at 12:41 PM Jeff Whitaker ***@***.***> wrote:
***@***.**** approved this pull request.
—
Reply to this email directly, view it on GitHub
<#568 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACD5GZ4AET4YVGH4EATFPXLXFZR2XANCNFSM6AAAAAAXWHXGGY>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
|
@chunhuazhou Could you have a look at this new change when you have a chance? |
|
Thank you Chunhua.
Ting, could you please make a regression test with your changes?
Shun
…On Fri, May 12, 2023 at 3:54 PM chunhua zhou ***@***.***> wrote:
***@***.**** approved this pull request.
—
Reply to this email directly, view it on GitHub
<#568 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACD5GZ4YCE55JUUV47J6TXTXF2IODANCNFSM6AAAAAAXWHXGGY>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
|
@ShunLiu-NOAA I have just merged the current EMC develop branch to this PR and re-ran the verification for the fv3-lam EnKF. It was as described in the previous verification, because in my case, the hydrometeors are not updated and hence the Yes/No to apply clipping to them doesn't affect the results. @daviddowellNOAA would you consider to run an enkf using dbz and see if the clipping of hydrometeors works as we expect ? Thanks. |
|
@RussTreadon-NOAA removing all extra space before "cliptracer" as you suggested. Thanks. |
|
@TingLei-daprediction and @RussTreadon-NOAA I can do a regression test on WCOSS2. |
|
@ShunLiu-NOAA Thanks a lot! |
|
Thank you, @ShunLiu-NOAA , for running the ctests on WCOSS2. As mentioned before, our current suite of ctests do not exercise the modified code. If we will be exercising the enkf in regional mode in operations, we should add a regional enkf regression test. @TingLei-daprediction , you did not answer my question. On which machine did you run the ctests? Also, a fresh clone of Below is a listing of all There is a double space between It looks like you misunderstood my request. I only wanted the double space on lines 1019, 1038, 2303, and 2396 changed to a single space. The other lines did not need to be changed. We want to see |
99c6286 to
40a6fd0
Compare
|
Thank you Russ. I will coordinate with Ming to update the regional
regression test case. It might take a while to prepare regional ensemble
members.
Shun
…On Fri, Jun 16, 2023 at 10:47 AM RussTreadon-NOAA ***@***.***> wrote:
Thank you, @ShunLiu-NOAA <https://github.com/ShunLiu-NOAA> , for running
the ctests on WCOSS2. As mentioned before, our current suite of ctests do
not exercise the modified code. If we will be exercising the enkf in
regional mode in operations, we should add a regional enkf regression test.
@TingLei-daprediction <https://github.com/TingLei-daprediction> , you did
not answer my question. On which machine did you run the ctests?
Also, a fresh clone of TingLei-daprediction:feature/enkf_q2 at 99c6286
<TingLei-daprediction/GSI@99c6286>
now shows a variety of spacing on the if cliptracers lines in
src/enkf/gridio_fv3reg.f90
Below is a listing of all if lines with cliptracers in gridio_fv3reg.f90.
Orion-login-1:/work2/noaa/da/rtreadon/git/gsi/pr568$ grep -n "cliptracer" src/enkf/gridio_fv3reg.f90 | grep if
857: if (cliptracers ) where (qworkvar3d < clip) qworkvar3d = clip
872: if (cliptracers ) where (qworkvar3d < clip) qworkvar3d = clip
939: if (cliptracers ) where (workvar3d < clip) workvar3d = clip
959: if (cliptracers ) where (workvar3d < clip) workvar3d = clip
979: if (cliptracers ) where (workvar3d < clip) workvar3d = clip
999: if (cliptracers ) where (workvar3d < clip) workvar3d = clip
1019: if ( cliptracers ) where (workvar3d < clip) workvar3d = clip
1038: if ( cliptracers ) where (workvar3d < clip) workvar3d = clip
2162: if (cliptracers ) where (qworkvar3d < clip) qworkvar3d = clip
2176: if (cliptracers ) where (qworkvar3d < clip) qworkvar3d = clip
2272: if (cliptracers ) where (workvar3d < clip) workvar3d = clip
2303: if ( cliptracers ) where (workvar3d < clip) workvar3d = clip
2334: if (cliptracers ) where (workvar3d < clip) workvar3d = clip
2365: if (cliptracers ) where (workvar3d < clip) workvar3d = clip
2396: if ( cliptracers ) where (workvar3d < clip) workvar3d = clip
2426: if (cliptracers ) where (workvar3d < clip) workvar3d = clip
There is a double space between if ( and cliptracers on lines 1019, 1038,
2303, and 2396. The is no spacing between if ( and cliptracers on lines
857, 872, 939, 959, 979, 999, 2162, 2176, 2272, 2334, 2365, and 2426.
It looks like you misunderstood my request. I only wanted the double space
on lines 1019, 1038, 2303, and 2396 changed to a single space. The other
lines did not need to be changed. We want to see if ( cliptracers ).
There is one space before cliptracers and one space after cliptracers.
—
Reply to this email directly, view it on GitHub
<#568 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACD5GZZ2Z5WNZ33RA6RP4N3XLRWX5ANCNFSM6AAAAAAXWHXGGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@RussTreadon-NOAA Please if the current changes do the formatting as you requested. Thanks. |
|
@ShunLiu-NOAA I think a repeated regional enkf using dbz ob (from your operational runs) should work for the regression test purpose ( go through the dbz related change). |
|
@RussTreadon-NOAA Do you think it is acceptable if I run a test case with regional ensembles from realtime parallel without adding a new regression test? At the same time, we will work on prepare regional enkf regression test case for future ctest. |
|
@TingLei-daprediction , a fresh clone of 40a6fd0 shows the desired spacing on the As an aside, the way you update your forked branch |
What you propose is fine. My ctest comment was looking ahead to ensure that future GSI PRs do not break the regional enkf or alter regional enkf output in unexpected ways. We will be running the enkf in the operational RRFS, right? If true, we should have a ctest which tests this future operational functionality. |
@RussTreadon-NOAA not sure why. I used "git push --force " to surpress some complaints from git. I will try to resolve those complaints before using " git push " without "--force option to see if that could avoid the "conflict" complaints on your side. |
|
@TingLei-daprediction , you should not use |
|
@RussTreadon-NOAA Ok. Thanks |
|
@RussTreadon-NOAA Thank you for approving this. I will merge the change after I complete my regional EnKF test. |
|
Jeff's and Chunhua's reviews and approvals along with yours are what really matter. My input is secondary. Thank you @ShunLiu-NOAA for running a regional enkf test. We need to get a regional enkf test into the suite of ctests. Additionally, since HAFSv1 is being implemented next week we should consider dropping the d2 and d3 HWRF tests in lieu of a new HAFS test. |
|
This PR is 11 days past its 6/15/2023 due date. This PR will be closed and returned to the developer tomorrow, 6/27/2023 unless it is finalized and merged into |
|
@russ Treadon - NOAA Federal ***@***.***> TIng and I are
testing EnKF with reflectivity assimilation. However, Ting's test with a
single case works, I still have troubles testing these changes with
realtime parallel. We are working on this.
…On Mon, Jun 26, 2023 at 9:00 AM RussTreadon-NOAA ***@***.***> wrote:
This PR is 11 days past its 6/15/2023 due date. This PR will be closed and
returned to the developer tomorrow, 6/27/2023 unless it is finalized and
merged into develop before then.
—
Reply to this email directly, view it on GitHub
<#568 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACD5GZZFF6GHFBOZXBQ5WR3XNGBV7ANCNFSM6AAAAAAXWHXGGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Thank you @ShunLiu-NOAA for the update. Testing delays are understood. |
|
Sure. If there is other PR merged in develop. We will run regression test again. By the way, we had regression tests on Friday. We also had an issue opened for regional EnKF test. |
|
Yes, I saw GSI issue #586. We should remove obsolete ctests as we add new tests. We want to keep the total number of ctests at a reasonable number. Who will replace the soon to be obsolete HWRF d2 and d3 ctests with a HAFS based ctest? Do we need to update the RTMA test? When can we retire the netcdf_fv3_regional test? |
|
HAFS DA is planning to merge the changes in HAFS v1 back to GSI repo. After
this, we will use HAFS ctest to replace HWRF ctest. Let me get more
information about RTMA test and update you later.
Shun
…On Mon, Jun 26, 2023 at 9:37 AM RussTreadon-NOAA ***@***.***> wrote:
Yes, I saw GSI issue #586 <#586>.
We should remove obsolete ctests as we add new tests. We want to keep the
total number of ctests at a reasonable number.
Who will replace the soon to be obsolete HWRF d2 and d3 ctests with a HAFS
based ctest? Do we need to update the RTMA test? When can we retire the
netcdf_fv3_regional test?
—
Reply to this email directly, view it on GitHub
<#568 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACD5GZ5DU4P3GKDZPYU2LP3XNGF7XANCNFSM6AAAAAAXWHXGGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Resubmit PR for cleaning the unneeded item in the previous PR
In correspondence to the EMC GSI Issue#566, this PR contains a quick adding of the clipping of negative values of sphum (q) in the analysis of FV3-LAM EnKF.
This part of codes are not tested in the current GSI regression tests, which, hence, are not run.
The current codes are verified using local FV3-LAM case. It is found the differences from this changes exist for sphum ( maximum values about 0.003 (units) and 0.3 K for T (sensible T). The latter is because the sphum would be used when the analysis TV is converted to T. All differences are on spontaneous points and values are reasonable as expected.
Hence, the code is regarded as verified.
Fixes #566
DUE DATE for this PR is 6/15/2023. If this PR is not merged into
developby this date, the PR will be closed and returned to the developer.