-
Notifications
You must be signed in to change notification settings - Fork 155
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
Dual-resolution EnVar capability for HAFS ensemble #652
Dual-resolution EnVar capability for HAFS ensemble #652
Conversation
… ensemble. From Xu Lu, OU ([email protected]) and POC: [email protected]
-- Xu Lu & Xuguang Wang from OU. POC: [email protected]
@yonghuiweng and @TingLei-daprediction Could you please review this PR? Thanks. |
@XL-OU Thanks to you and collaborators for adding this function to FV3-LAM GSI . In addition to existing regressing test and your own verification, have you done a verification using the existing single resolution option for the dual resolution option added in this PR? Namely, to repeat a single resolution case but enforce the dual resolution option turned on. If the latter is implemented correctly, it should produce the identical results (except for round-off errors). I would take such a successful test as a good verification of this PR. |
Hi, Ting,
I remembered doing similar tests before when we initially started the
implementation.
But I did not try with this new version of code since there is a lib/module
related issue on Orion that limits my ability to test the capabilities
since last weekend.
I'm not sure how long this problem will last, but I will try as soon as I
can.
Best,
Xu
…On Tue, Nov 7, 2023 at 7:16 AM TingLei-NOAA ***@***.***> wrote:
@XL-OU <https://github.com/XL-OU> Thanks to you and collaborators for
adding this function to FV3-LAM GSI . In addition to existing regressing
test and your own verification, have you done a verification using the
existing single resolution option for the dual resolution option added in
this PR? Namely, to repeat a single resolution case but enforce the dual
resolution option turned on. If the latter is implemented correctly, it
should produce the identical results (except for round-off errors). I would
take such a successful test as a good verification of this PR.
—
Reply to this email directly, view it on GitHub
<#652 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGK64BUWLCBEINZYHOVGBQLYDIYC3AVCNFSM6AAAAAA646XX4KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOJYGQ4DMMZRGU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Hi, Ting,
I'm not sure what kind of test you want me to do. Do you want me to clean
the if statement and to check if the grid_type_fv3_regional variable can be
passed down from the control grid?
Yes, in the current configuration, the ensemble and control should be the
same orientation. But do we want to keep the possibility in the future that
it can read other dual_res ensembles from the other FV3 systems, e.g.
global FV3?
Regards,
Xu
…On Tue, Nov 7, 2023 at 7:31 AM TingLei-NOAA ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/gsi/gsi_rfv3io_mod.f90
<#652 (comment)>:
> + enddo
+ endif
+ if(mype==0)write(6,*),'nxens,nyens=',nxens,nyens
+ if(mype==0)write(6,*),'ny_layout_lenens=',ny_layout_lenens
+ if(mype==0)write(6,*),'ny_layout_bens=',ny_layout_bens
+ if(mype==0)write(6,*),'ny_layout_eens=',ny_layout_eens
+
+!
+! need to decide the grid orientation of the FV regional model
+!
+! grid_type_fv3_regional = 0 : decide grid orientation based on
+! grid_lat/grid_lon
+! 1 : input is E-W N-S grid
+! 2 : input is W-E S-N grid
+!
+ if(grid_type_fv3_regional == 0) then
Since dual_res is to deal with ensemble forecast being different from the
control forecast, will it be necessary to consider if they are also
different in the grid orientation? Or, at least, do a test to make sure
they are are same ( only one grid_type_fv3_regional parameter is needed).
—
Reply to this email directly, view it on GitHub
<#652 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGK64BSNTUYLMWRBSYBIDZDYDIZZ5AVCNFSM6AAAAAA646XX4KVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTOMJXGY4TGNRXGI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Hi, Ting,
nxcase and nycase is assigned, but the ny_layout_len and ny_layout_lenens
is different when allocating countloc and uu2d_layout if this is your
question.
Best,
Xu
…On Tue, Nov 7, 2023 at 7:39 AM TingLei-NOAA ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/gsi/gsi_rfv3io_mod.f90
<#652 (comment)>:
> @@ -2376,11 +2562,20 @@ subroutine gsi_fv3ncdf_read(grd_ionouv,cstate_nouv,filenamein,fv3filenamegin)
if(fv3_io_layout_y > 1) then
do nio=0,fv3_io_layout_y-1
- countloc=(/nxcase,ny_layout_len(nio),1/)
- allocate(uu2d_layout(nxcase,ny_layout_len(nio)))
+ if (ensgrid) then
if nxcase (nycase) is already assigned corresponding values at line 2474
to 2480, do we still need this if branch here?
—
Reply to this email directly, view it on GitHub
<#652 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGK64BX7E2E5RS4KCGVLLTDYDI2ZBAVCNFSM6AAAAAA646XX4KVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTOMJXG4YDQNRQGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Yes, Ting,
This line should be removed. Since there is a new max_filename_length being
used, we have removed the changes to the max_varname_length.
Should I change and push it directly to this branch or how to proceed for
this?
Thanks,
Xu
…On Tue, Nov 7, 2023 at 12:55 PM TingLei-NOAA ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/gsi/constants.f90
<#652 (comment)>:
> @@ -31,6 +31,7 @@ module constants
! 2016-02-15 Johnson, Y. Wang, X. Wang - define additional constant values for
! radar DA, POC: ***@***.***
! 2019-09-25 X.Su - put stndrd_atmos_ps constant values
+! 2022-03-01 X.Lu & X.Wang - increased max_varname_length for HAFS dual ens. POC: ***@***.***
where is the corresponding change? From this PR review interface, there is
no changes in this file except for this comment line.
—
Reply to this email directly, view it on GitHub
<#652 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGK64BTXRISKIMXHNBSLAKDYDJ7ZTAVCNFSM6AAAAAA646XX4KVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTOMJYGQ4DSNZXGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@XL-OU yes, make your change and commit back . I think you could use "commit --amend" and then "git push --force". but simpler, you could just do regular commit and push since current GSI managing team will use git sqush in final merging of this PR to the repository and be satisfied with created log of changes. |
-- by Xu Lu and Xuguang Wang from OU. POC: [email protected]
Yes. since now, you are creating grids and conversion between native grids and "analysis" grids for ensemble seperately, I think an extra check on the grid orientation is needed. From my undstanding, in your current version, ensemble forecast and the control control could be of different orientation. But, for safety (before any tests/verification) and simplicity, now, we can require they are the same. Namely, you can add the code to get the orientation as it was done for the control grid. And if they are not the same, print out "error message' and abort. |
Yes, please do a verification for this PR when you have a chance. |
Hi, Ting,
Good catch! I think it is somehow duplicating L1391-1393 during the merge.
I'll just remove them.
Thanks,
Xu
…On Tue, Nov 7, 2023 at 3:53 PM TingLei-NOAA ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/gsi/hybrid_ensemble_isotropic.F90
<#652 (comment)>:
> @@ -1393,6 +1393,10 @@ subroutine load_ensemble
endif
! regional_ensemble_option = 5: ensembles are fv3 regional.
+ if (l_both_fv3sar_gfs_ens) then ! first read in gfs ensembles for regional
for line 1394 to 1399, why do we need to change to it? though the
essential change is of the comment.
I believe the "cltthink .." question is not needed now.
—
Reply to this email directly, view it on GitHub
<#652 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGK64BUIG5PVJS7Q6NADOL3YDKUUHAVCNFSM6AAAAAA646XX4KVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTOMJYHA2DQMZVGM>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Sure, I will do that.
Xu
…On Tue, Nov 7, 2023 at 3:55 PM TingLei-NOAA ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/gsi/hybrid_ensemble_isotropic.F90
<#652 (comment)>:
> @@ -4035,6 +4042,8 @@ subroutine hybens_grid_setup
logical,allocatable::vector(:)
real(r_kind) eps,r_e
real(r_kind) rlon_a(nlon),rlat_a(nlat),rlon_e(nlon),rlat_e(nlat)
+ character(:),allocatable:: fv3_spec_grid_filename
to conform to the style of your changes in this PR , I suggest to use name
as "fv3_ens_spec_grid_filename"
—
Reply to this email directly, view it on GitHub
<#652 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGK64BTFEEA52D7BXGWZMSTYDKUZ7AVCNFSM6AAAAAA646XX4KVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTOMJYHA2TANBWHA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
…tropic.F90 -- Xu Lu & Xuguang Wang from OU. POC: [email protected]
Sure, I can try with that. But again, the missing modules/libs on Orion are
failing my compilations for GSI. So I'll still have to wait until it gets
back to normal.
Regards,
Xu
On Tue, Nov 7, 2023 at 4:02 PM TingLei-NOAA ***@***.***>
wrote:
… Hi, Ting, I'm not sure what kind of test you want me to do. Do you want me
to clean the if statement and to check if the grid_type_fv3_regional
variable can be passed down from the control grid? Yes, in the current
configuration, the ensemble and control should be the same orientation. But
do we want to keep the possibility in the future that it can read other
dual_res ensembles from the other FV3 systems, e.g. global FV3? Regards, Xu
… <#m_-730429452887118020_>
On Tue, Nov 7, 2023 at 7:31 AM TingLei-NOAA *@*.*> wrote: @.** commented
on this pull request. ------------------------------ In
src/gsi/gsi_rfv3io_mod.f90 <#652 (comment)
<#652 (comment)>>: > +
enddo + endif + if(mype==0)write(6,*),'nxens,nyens=',nxens,nyens +
if(mype==0)write(6,*),'ny_layout_lenens=',ny_layout_lenens +
if(mype==0)write(6,*),'ny_layout_bens=',ny_layout_bens +
if(mype==0)write(6,*),'ny_layout_eens=',ny_layout_eens + +! +! need to
decide the grid orientation of the FV regional model +! +!
grid_type_fv3_regional = 0 : decide grid orientation based on +!
grid_lat/grid_lon +! 1 : input is E-W N-S grid +! 2 : input is W-E S-N grid
+! + if(grid_type_fv3_regional == 0) then Since dual_res is to deal with
ensemble forecast being different from the control forecast, will it be
necessary to consider if they are also different in the grid orientation?
Or, at least, do a test to make sure they are are same ( only one
grid_type_fv3_regional parameter is needed). — Reply to this email
directly, view it on GitHub <#652 (review)
<#652 (review)>>,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AGK64BSNTUYLMWRBSYBIDZDYDIZZ5AVCNFSM6AAAAAA646XX4KVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTOMJXGY4TGNRXGI
. You are receiving this because you were mentioned.Message ID: *@*.***>
Yes. since now, you are creating grids and conversion between native grids
and "analysis" grids for ensemble seperately, I think an extra check on the
grid orientation is needed. From my undstanding, in your current version,
ensemble forecast and the control control could be of different
orientation. But, for safety (before any tests/verification) and
simplicity, now, we can require they are the same. Namely, you can add the
code to get the orientation as it was done for the control grid. And if
they are not the same, print out "error message' and abort.
—
Reply to this email directly, view it on GitHub
<#652 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGK64BVXID53MAHL3AYBOQDYDKVXVAVCNFSM6AAAAAA646XX4KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBQGI2TIMJQGU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@XL-OU I had a successful building of your branch on orion: /work/noaa/fv3-cam/tlei/dr-gsi-pr/GSI. Just ran : ush/build.sh . Please see if you could use that setup to avoid the "missing lib" issue for you . |
Hi, Ting!
Thanks for your suggestion!
Though I can not access your dir due to permission issues, I did try the
build.sh on my end and it works now.
Can you
check /work/noaa/aoml-hafsda/xlu/newtrunk/sync_202311/GSI/src/gsi/gsi_rfv3io_mod.f90
between L638-652 to see if they are consistent with what you want before I
push the commit?
I did a simple test and the orientations are consistent between
the ensemble and the control.
Best,
Xu
On Tue, Nov 7, 2023 at 9:51 PM TingLei-NOAA ***@***.***>
wrote:
… Sure, I can try with that. But again, the missing modules/libs on Orion
are failing my compilations for GSI. So I'll still have to wait until it
gets back to normal. Regards, Xu On Tue, Nov 7, 2023 at 4:02 PM
TingLei-NOAA *@*.
*> wrote: … <#m_2253337994894212727_> Hi, Ting, I'm not sure what kind of
test you want me to do. Do you want me to clean the if statement and to
check if the grid_type_fv3_regional variable can be passed down from the
control grid? Yes, in the current configuration, the ensemble and control
should be the same orientation. But do we want to keep the possibility in
the future that it can read other dual_res ensembles from the other FV3
systems, e.g. global FV3? Regards, Xu … <#m_-730429452887118020_> On Tue,
Nov 7, 2023 at 7:31 AM TingLei-NOAA @.> wrote: @.* commented on this pull
request. ------------------------------ In src/gsi/gsi_rfv3io_mod.f90 <
#652 <#652> (comment) <#652 (comment)
<#652 (comment)>>>: > +
enddo + endif + if(mype==0)write(6,*),'nxens,nyens=',nxens,nyens +
if(mype==0)write(6,*),'ny_layout_lenens=',ny_layout_lenens +
if(mype==0)write(6,*),'ny_layout_bens=',ny_layout_bens +
if(mype==0)write(6,*),'ny_layout_eens=',ny_layout_eens + +! +! need to
decide the grid orientation of the FV regional model +! +!
grid_type_fv3_regional = 0 : decide grid orientation based on +!
grid_lat/grid_lon +! 1 : input is E-W N-S grid +! 2 : input is W-E S-N grid
+! + if(grid_type_fv3_regional == 0) then Since dual_res is to deal with
ensemble forecast being different from the control forecast, will it be
necessary to consider if they are also different in the grid orientation?
Or, at least, do a test to make sure they are are same ( only one
grid_type_fv3_regional parameter is needed). — Reply to this email
directly, view it on GitHub <#652
<#652> (review) <#652 (review)
<#652 (review)>>>,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AGK64BSNTUYLMWRBSYBIDZDYDIZZ5AVCNFSM6AAAAAA646XX4KVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTOMJXGY4TGNRXGI
. You are receiving this because you were mentioned.Message ID: *@*.*>
Yes. since now, you are creating grids and conversion between native grids
and "analysis" grids for ensemble seperately, I think an extra check on the
grid orientation is needed. From my undstanding, in your current version,
ensemble forecast and the control control could be of different
orientation. But, for safety (before any tests/verification) and
simplicity, now, we can require they are the same. Namely, you can add the
code to get the orientation as it was done for the control grid. And if
they are not the same, print out "error message' and abort. — Reply to this
email directly, view it on GitHub <#652 (comment)
<#652 (comment)>>, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AGK64BVXID53MAHL3AYBOQDYDKVXVAVCNFSM6AAAAAA646XX4KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBQGI2TIMJQGU
<https://github.com/notifications/unsubscribe-auth/AGK64BVXID53MAHL3AYBOQDYDKVXVAVCNFSM6AAAAAA646XX4KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBQGI2TIMJQGU>
. You are receiving this because you were mentioned.Message ID: @.*>
@XL-OU <https://github.com/XL-OU> I had a successful building of your
branch on orion: /work/noaa/fv3-cam/tlei/dr-gsi-pr/GSI. Just ran :
ush/build.sh . Please see if you could use that setup to avoid the "missing
lib" issue for you .
—
Reply to this email directly, view it on GitHub
<#652 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGK64BX5ZYHMSYY4QFDLXPDYDL6TTAVCNFSM6AAAAAA646XX4KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBQHE3DSNBXGI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@XL-OU That is great you added that check in the code. Just a minor point: could the name :grid_type_fv3_regionalens be changed to , say, grid_ens_type_fv3_regional? (also, in the following comment, "same " to "the same"). |
…ientation as the control. -- Xu Lu & Xuguang Wang from OU. POC: [email protected]
Sure, Ting,
Here are the single_res and fake-dual_res runs using both 6-km control and
ensemble as suggested. Barely a difference by visual check.
The log files show the max increment differences are about 1% of the
innovation.
/work/noaa/aoml-hafsda/xlu/testrun/sync_test/2020082512/13L/testlow/stdout
/work/noaa/aoml-hafsda/xlu/testrun/sync_test/2020082512/13L/
testlow_dual/stdout
Hope this satisfies the verification purpose.
Best,
Xu
…On Thu, Nov 9, 2023 at 1:14 PM TingLei-NOAA ***@***.***> wrote:
@XL-OU <https://github.com/XL-OU> Thanks. I will take a closer look at
your figures. For verification, what I am thinking about is an experiment
as :
1. from your 6 km ensemble, choose any one to create a 6 km control
background.
so, now, you have a single resolution run on 6 km,
2. starting from this 6 km single resolution run, enforce
dual_res=.true., you would have a dual resolution while the ensembles are
on the same 6 km resolution. If this dual resolution run produces an
identical result as the single resolution run, that will be a verification
of the codes.
—
Reply to this email directly, view it on GitHub
<#652 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGK64BUDILESBYXE2V2DJILYDUTQVAVCNFSM6AAAAAA646XX4KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBUGQ2TANBWHA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@XL-OU Thanks for those verifications results. For the
@XL-OU Thanks for this comparison experiments. For me, the difference is still too large. It is always recognized difference in the initial gradients. First, could you run such comparison experiments with real data rather than one ob test? Second, does this PR repeat the results from your dual resolution GSI version? Except for the two questions, this PR looks good me and approval only depends on if it would pass the regression tests. |
Hi, Ting,
Please see the increments using all prepbufr data, and still there are no
differences in the increment patterns despite the stdout and maximum
differences.
Logs are here:
/work/noaa/aoml-hafsda/xlu/testrun/sync_test/2020082512/13L/dual_prep/stdout
/work/noaa/aoml-hafsda/xlu/testrun/sync_test/2020082512/13L/low_prep/stdout
And the results are almost comparable between this version and my old
versions.
If you compare the one ob results:
/work/noaa/aoml-hafsda/xlu/testrun/sync_test/2020082512/13L/dual_old/stdout
/work/noaa/aoml-hafsda/xlu/testrun/sync_test/2020082512/13L/test/stdout
The differences only start in the cost function after minimization and look
to be round-off errors with an order of < 10e-10.
Best,
Xu
On Tue, Nov 14, 2023 at 11:03 AM TingLei-NOAA ***@***.***>
wrote:
… @XL-OU <https://github.com/XL-OU> Thanks for those verifications results.
For the
Sure, Ting, Here are the single_res and fake-dual_res runs using both 6-km
control and ensemble as suggested. Barely a difference by visual check. The
log files show the max increment differences are about 1% of the
innovation.
/work/noaa/aoml-hafsda/xlu/testrun/sync_test/2020082512/13L/testlow/stdout
/work/noaa/aoml-hafsda/xlu/testrun/sync_test/2020082512/13L/
testlow_dual/stdout Hope this satisfies the verification purpose. Best, Xu
… <#m_7550226161775171842_>
On Thu, Nov 9, 2023 at 1:14 PM TingLei-NOAA *@*.*> wrote: @XL-OU
<https://github.com/XL-OU> https://github.com/XL-OU
<https://github.com/XL-OU> Thanks. I will take a closer look at your
figures. For verification, what I am thinking about is an experiment as :
1. from your 6 km ensemble, choose any one to create a 6 km control
background. so, now, you have a single resolution run on 6 km, 2. starting
from this 6 km single resolution run, enforce dual_res=.true., you would
have a dual resolution while the ensembles are on the same 6 km resolution.
If this dual resolution run produces an identical result as the single
resolution run, that will be a verification of the codes. — Reply to this
email directly, view it on GitHub <#652 (comment)
<#652 (comment)>>, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AGK64BUDILESBYXE2V2DJILYDUTQVAVCNFSM6AAAAAA646XX4KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBUGQ2TANBWHA
<https://github.com/notifications/unsubscribe-auth/AGK64BUDILESBYXE2V2DJILYDUTQVAVCNFSM6AAAAAA646XX4KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBUGQ2TANBWHA>
. You are receiving this because you were mentioned.Message ID: @.*>
@XL-OU <https://github.com/XL-OU> Thanks for this comparison experiments.
For me, the difference is still too large. It is always recognized
difference in the initial gradients. First, could you run such comparison
experiments with real data rather than one ob test? Second, does this PR
repeat the results from your dual resolution GSI version? Except for the
two questions, this PR looks good me and approval only depends on if it
would pass the regression tests.
—
Reply to this email directly, view it on GitHub
<#652 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGK64BV2FLHCMEM5VX3FN33YEOP4HAVCNFSM6AAAAAA646XX4KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJQG4YDCNZRHA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@XL-OU as shown in your real data experiments, their initial cost and gradients are as Initial cost function = 6.994250892733360524E+04 |
@ShunLiu-NOAA This pr looks good to me now . The only thing left is the regression tests. Thanks. |
Here are my ctest results:
The hafs_3denvar_hybens failed because the control branch does not have the capability to read in the dual_res HAFS ensemble properly. |
The difference of control exp and update exp is expected for the hafs_3denvar_hybens case for this PR because of the dual-res HAFS ensemble member. The other HAFS related test case hafs_4denvar_glbens should pass with no error, which is also expected. |
@JingCheng-NOAA or @yonghuiweng could you please start a regression test on WCOSS2? |
I still haven't got an account on WCOSS2 yet. Yonghui, could you please
help with this?
…On Thu, Nov 16, 2023 at 10:00 AM ShunLiu-NOAA ***@***.***> wrote:
@JingCheng-NOAA <https://github.com/JingCheng-NOAA> or @yonghuiweng
<https://github.com/yonghuiweng> could you please start a regression test
on WCOSS2?
—
Reply to this email directly, view it on GitHub
<#652 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/BAHEWILSZUM7TJBJLKBSQQLYEYTARAVCNFSM6AAAAAA646XX4KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJUGYZDIOJUGM>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Here is the test on wcoss2: 86% tests passed, 1 tests failed out of 7 Total Test time (real) = 1613.87 sec The following tests FAILED: |
Thank you Yonghui. The result is consistent with Xu's RT result on Orion.
…On Thu, Nov 16, 2023 at 3:34 PM Yonghui Weng ***@***.***> wrote:
Here is the test on wcoss2:
Test project
/lfs/h2/emc/hur/noscrub/yonghui.weng/regression/hafs_dualres_envar/build
Start 1: global_4denvar
Start 2: rtma
Start 3: rrfs_3denvar_glbens
Start 4: netcdf_fv3_regional
Start 5: hafs_4denvar_glbens
Start 6: hafs_3denvar_hybens
Start 7: global_enkf
1/7 Test #4 <#4>: netcdf_fv3_regional
.............. Passed 485.81 sec
2/7 Test #3 <#3>:
rrfs_3denvar_glbens .............. Passed 728.40 sec
3/7 Test #7 <#7>: global_enkf
...................... Passed 1094.43 sec
4/7 Test #2 <#2>: rtma
............................. Passed 1217.68 sec
5/7 Test #6 <#6>: hafs_3denvar_hybens
..............***Failed 1278.39 sec
6/7 Test #5 <#5>: hafs_4denvar_glbens
.............. Passed 1454.72 sec
7/7 Test #1 <#1>: global_4denvar
................... Passed 1613.84 sec
86% tests passed, 1 tests failed out of 7
Total Test time (real) = 1613.87 sec
The following tests FAILED:
6 - hafs_3denvar_hybens (Failed)
Errors while running CTest
—
Reply to this email directly, view it on GitHub
<#652 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/BAHEWINFFQLGXZ6XAN4N2ITYEZ2GTAVCNFSM6AAAAAA646XX4KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJVGI3TCNZWHE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@JingCheng-NOAA @yonghuiweng @XL-OU do we need further address the reason of the failure of hafs_3denvar_hybens? |
The control branch does not have the dual_res capability for the HAFS ensemble. It does not produce error info but erroneously treats the dual_res HAFS ensemble as the same resolution as the control. So when the correct dual_res capability is in the update branch, it will create a difference and fail. |
@JingCheng-NOAA @yonghuiweng @XL-OU @TingLei-NOAA After careful discussion with all of you, it appears that we need to correct the issue in the control run first, and then CTEST in this PR may work as intended to ensure that the modifications in this PR do not disrupt the existing functions in GSI. |
The hafs_3denvar_hybens dataset contains different resolutions of hafs
members. The current DEV branch code doesn't have the capability to handle
dual res data. In this case, I will suggest to remove hafs_3denvar_hybens
from ctest before this PR, because it was designed to test the dual_res
capability.
…On Fri, Nov 17, 2023 at 10:08 AM ShunLiu-NOAA ***@***.***> wrote:
@JingCheng-NOAA <https://github.com/JingCheng-NOAA> @yonghuiweng
<https://github.com/yonghuiweng> @XL-OU <https://github.com/XL-OU>
@TingLei-NOAA <https://github.com/TingLei-NOAA> After careful discussion
with all of you, it appears that we need to correct the issue in the
control run first, and then CTEST in this PR may work as intended to ensure
that the modifications in this PR do not disrupt the existing functions in
GSI.
—
Reply to this email directly, view it on GitHub
<#652 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/BAHEWIKWLLLKWHC23K3DHMLYE54XRAVCNFSM6AAAAAA646XX4KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJWGU4TMOBYG4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@ShunLiu-NOAA, @RussTreadon-NOAA, based on the discussions/communications, my understanding is that the regression test of "hafs_3denvar_hybens" is expected to not get the same results for this PR. However, for future PR RTs after this PR being merged, the hafs_3denvar_hybens will work normally as expected. With that, wondering if there is a plan and timeline to move forward to merge this PR. Or, otherwise, please feel free to instruct us on how to move forward. Much appreciated! |
@BinLiu-NOAA @JingCheng-NOAA @yonghuiweng @XL-OU As discussed with PR handling group, we suggest:
If the only failure is from "hafs_3denvar_hybens", this PR will be good to merge into develop.
|
Thanks, @ShunLiu-NOAA! And @JingCheng-NOAA, @yonghuiweng, @XL-OU, please go ahead to rerun the RTs on WCOSS2, HERA and ORION as suggested. Thanks! |
Hi, All,
|
Here are the RTs on HERA: Total Test time (real) = 1626.58 sec The following tests FAILED: ` |
Test on wcoss2 - catcus 86% tests passed, 1 tests failed out of 7 Total Test time (real) = 1489.28 sec The following tests FAILED: |
Thank you all for regression test. |
@TingLei-NOAA Thank you for reviewing this PR. As agreed upon with the PR handling team, we will merge this PR into "develop" since, as the HAFS DA team has explained, doing so will fix the issue in "hafs_3denvar_hybens". We recognized your concern as well. If we find any possible issues in the future, we will ask the HAFS DA team to address them. |
@XL-OU Your branch is one commit behind "develop". Could you sync your branch with develop? |
…hafs_dualres_envar
@ShunLiu-NOAA, since @XL-OU is on travel, I have just synced this PR branch with the latest develop branch. Thanks! Meanwhile, if possible, when merging this PR, please use the "Squash and Merge" option. Thanks! |
Description
Add Dual-resolution EnVar capability for HAFS ensemble
By Xu Lu and Xuguang Wang from OU. POC: [email protected]
Fixes #603
Type of change
How Has This Been Tested?
Did a single observation test to ensure the capability works and the increments look reasonable compared to the single-resolution increment on Orion.
Checklist
DUE DATE for this PR is 12/15/2023. If this PR is not merged into
develop
by this date, the PR will be closed and returned to the developer.