Skip to content
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

fix for appararent mis use of variable tpf vs tpf2 in read_nsstbu… #2

Draft
wants to merge 2 commits into
base: feature/fv3reg_parallel_io_upgrade
Choose a base branch
from

Conversation

TingLei-daprediction
Copy link
Owner

By adding extra debug compiler options, when running the regression test : global_4densvar, it was found variables are used while not defined/assigned values as expected in read_nsstbufr.f90
Fix was done but hope for sat da experts to confirm the fix.

TingLei-daprediction pushed a commit that referenced this pull request May 13, 2024
**DUE DATE for merger of this PR into `develop` is 2/19/2024 (six weeks
after PR creation).**
**DUE DATE for this PR is extended to 3/19/2024 because @XuLu-NOAA is on
leave.**
**Description**

Xu Lu (xu.lu@noaa.gov) and Biju Thomas (biju.thomas@noaa.gov) fixed bugs
regarding HAFS GSI debug build and run issues. This is in corresponding
to issue NOAA-EMC#661

Fixes NOAA-EMC#661

1. In read_radar.f90, uninitialized toff is making all the ground-based
radar observations be placed at -3h instead of 0h, which creates wrong
increments for FGAT and 4DEnVar.
2. In read_radar.f90, uninitialized zsges will crash the debug mode.
3. In read_radar.f90, t4dvo should be used instead of t4dv in the
read_radar_l2rw_novadqc subroutine.
4. In radinfo.90, maxscan should be increased to at least 252 to allow
more scans, or it will crash the debug mode.
5. In read_fl_hdob.f90, dlnpsob is replaced with 1000. since the SFMR
does not sample surface pressure, and the uninitialized dlnpsob creates
issues later in setupspd.f90 in the debug mode.
6. In mod_fv3_lola.f90, (i,j+1) should be used instead of (i+1,j) in
searching for V edges.
7. In stpcalc.f90, when tried to find the best stepsize from outpen
around L838-864, the minimum outstp(i) is stored in stp(ii), but the
istp_use is asigned with i instead of ii. Create inconsistency when
assigning stp(istp_use) to stpinout at L872. Should use istp_use=ii
instead.

**Type of change**
- [Yes] Bug fix (non-breaking change which fixes an issue)

**How Has This Been Tested?**
Regression test on Orion:
```
Test project /work/noaa/hwrf/save/xulu/mergeversions/GSI/build
CMake Warning (dev) at CTestTestfile.cmake:9 (subdirs):  Syntax Warning in cmake code at /work/noaa/hwrf/save/xulu/mergeversions/GSI/build/regression/CTestTestfile.cmake:7:10
1/7 Test NOAA-EMC#4: [=[netcdf_fv3_regional]=] ........   Passed  365.11 sec
2/7 Test NOAA-EMC#7: [=[global_enkf]=] ................   Passed  430.29 sec
3/7 Test NOAA-EMC#3: [=[rrfs_3denvar_glbens]=] ........   Passed  605.35 sec
4/7 Test #2: [=[rtma]=] .......................   Passed  969.78 sec
5/7 Test NOAA-EMC#6: [=[hafs_3denvar_hybens]=] ........***Failed  1455.47 sec
6/7 Test #1: [=[global_4denvar]=] .............   Passed  1682.40 sec
7/7 Test NOAA-EMC#5: [=[hafs_4denvar_glbens]=] ........***Failed  1758.90 sec
```

The failed hafs_3denvar and 4denvar are within expectation due to the
fix for toff. As demonstrated in the single observation tests in the
following figure, the uninitialized toff can result in increment
degradations due to wrongly assigned observation times:

![image](https://github.com/NOAA-EMC/GSI/assets/26603014/0de870e1-f8c8-4b6d-8039-57f417b76367)
TingLei-daprediction pushed a commit that referenced this pull request May 13, 2024
…ed 3DRTMA (NOAA-EMC#730)

<!-- PLEASE READ -->
<!--
Before opening a PR, please note these guidelines:

- Each PR should only address ONE topic and have an associated issue
- No hardcoded or paths to personal directories should be present
- No temporary or backup files should be committed
- Any code that was disabled by being commented out should be removed
-->

**Description**

<!-- Please include relevant motivation and context. -->
<!-- Please include a summary of the change and which issue is fixed.
-->
<!-- List any dependencies that are required for this change. -->
To improve the analysis of the near-surface wind gust in 3DRTMA, the
observations of near-surface wind gust would be analyzed directly in GSI
(3DVar and Hybrid 3DEnVar), instead of being a derived product from the
near-surface wind analysis.

Since the core subroutines for direct variational assimilation of wind
gust (e.g., setupgust.f90, intgust.f90, stpgust.f90, etc.) had already
been implemented in GSI for 2DRTMA, so in the work the development in
GSI mainly focus on adding I/O of 2-D wind gust firstguess and analysis
fields for RRFS-based 3DRTMA, and some minor modifications in
observation and background error for wind gust, options to control the
analysis of wind gust, etc.
<!-- Please provide reference to the issue this pull request is
addressing. -->
<!-- For e.g. Fixes #IssueNumber -->
This PR is to address the issue NOAA-EMC#726 : Adding I/O for direct analysis of
near-surface wind gust for RRFS-based 3DRTMA
Fixes NOAA-EMC#726
**Type of change**

Please delete options that are not relevant.

- [x] New feature (non-breaking change which adds functionality)

**How Has This Been Tested?**

<!-- Please describe the tests that you ran to verify your changes and
on the platforms these tests were conducted. -->
<!-- Provide instructions so we can reproduce. -->
<!-- Please also list any relevant details for your test configuration
-->
  
**Checklist**

- [x] My code follows the style guidelines of this project
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] New and existing tests pass with my changes
      tested with a real case - 2024-02-20_12:00Z, 
1. dry-run (using my updated GSI code with wind gust analysis, but
actually no wind gust obs is analyzed, so-called dryrun) is compared to
control-run (original GSI code running without wind gust obs): the
results are identical. This indicates that if without analyzing
wind-gust obs, then the updated code generates the analysis identical to
the analysis of original/control code. Or say, the added code does not
have influence on the other part of code.
2. real case run with updated GSI code to analyze the obs of wind gust:
The following figure shows the used observations of near-surface wind
gust:

![var_obs_2024022012_gust_used_maprll_datll_reg_ncf](https://github.com/NOAA-EMC/GSI/assets/53267411/ecbe479a-03c6-490f-a179-9e0027291468)
the following figure shows the analysis increments:

![GUST_hyb_hwllp90_corptuned_inc_incrintrp_maprll_datrll_reg_grb2](https://github.com/NOAA-EMC/GSI/assets/53267411/a01fca0d-dc1f-438b-b8eb-e624de35a631)
- [x] Any dependent changes have been merged and published
- [x] Regression tests on WCOSS2 (Cactus) and Hera (Rocky-8) : my
updated GSI commit
[#f91f247d](GangZhao-NOAA@f91f247))
vs control/original GSI code (commit
[#6d9ebbb7](NOAA-EMC@6d9ebbb))
Here is the reports of the regression tests on WCOSS2 (Cactus):
~~~
[gang.zhao@clogin02:build] (feature/windgust_in_3dvar_for_3drtma)$ ctest
-j 7
Test project
/lfs/h2/emc/da/save/gang.zhao/WorkDir/ProdGSI_Dev/gsi_dev/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 NOAA-EMC#4: netcdf_fv3_regional ..............   Passed  483.15 sec
2/7 Test NOAA-EMC#3: rrfs_3denvar_glbens ..............   Passed  486.74 sec
3/7 Test NOAA-EMC#7: global_enkf ......................   Passed  850.98 sec
4/7 Test #2: rtma .............................   Passed  970.28 sec
5/7 Test NOAA-EMC#6: hafs_3denvar_hybens ..............   Passed  1152.82 sec
6/7 Test NOAA-EMC#5: hafs_4denvar_glbens ..............   Passed  1213.93 sec
7/7 Test #1: global_4denvar ...................   Passed  1683.16 sec

100% tests passed, 0 tests failed out of 7

Total Test time (real) = 1683.19 sec
~~~
Here is the reports of the regression tests on Hera (Rocky8):
~~~
(base) [Gang.Zhao@hfe11:build] (feature/windgust_in_3dvar_for_3drtma)$
ctest -j 7
Test project /scratch1/NCEPDEV/da/Gang.Zhao/ProdGSI_dev/gsi_dev/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 NOAA-EMC#4: netcdf_fv3_regional ..............   Passed  491.53 sec
2/7 Test NOAA-EMC#3: rrfs_3denvar_glbens ..............***Failed  495.27 sec
3/7 Test #2: rtma .............................   Passed  982.45 sec
4/7 Test NOAA-EMC#6: hafs_3denvar_hybens ..............   Passed  1168.99 sec
5/7 Test NOAA-EMC#7: global_enkf ......................   Passed  1239.77 sec
6/7 Test NOAA-EMC#5: hafs_4denvar_glbens ..............***Failed  1347.87 sec
7/7 Test #1: global_4denvar ...................   Passed  1974.45 sec

71% tests passed, 2 tests failed out of 7

Total Test time (real) = 1974.91 sec

The following tests FAILED:
          3 - rrfs_3denvar_glbens (Failed)
          5 - hafs_4denvar_glbens (Failed)
Errors while running CTest
Output from these tests are in:
/scratch1/NCEPDEV/da/Gang.Zhao/ProdGSI_dev/gsi_dev/build/Testing/Temporary/LastTest.log
Use "--rerun-failed --output-on-failure" to re-run the failed cases
verbosely.
(base) [Gang.Zhao@hfe11:build] (feature/windgust_in_3dvar_for_3drtma)$
ctest -R rrfs_3denvar_glbens
Test project /scratch1/NCEPDEV/da/Gang.Zhao/ProdGSI_dev/gsi_dev/build
    Start 3: rrfs_3denvar_glbens
1/1 Test NOAA-EMC#3: rrfs_3denvar_glbens ..............   Passed  430.52 sec

100% tests passed, 0 tests failed out of 1

Total Test time (real) = 430.55 sec
(base) [Gang.Zhao@hfe11:build] (feature/windgust_in_3dvar_for_3drtma)$
ctest -R hafs_4denvar_glbens
Test project /scratch1/NCEPDEV/da/Gang.Zhao/ProdGSI_dev/gsi_dev/build
    Start 5: hafs_4denvar_glbens
1/1 Test NOAA-EMC#5: hafs_4denvar_glbens ..............   Passed  1225.37 sec

100% tests passed, 0 tests failed out of 1

Total Test time (real) = 1225.39 sec
~~~
**Note**: 
_When I was running the regression tests, GSI code was just updated to
the latest commit
[#b53740a7](GangZhao-NOAA@f91f247).
Considering the frequent update in EMC GSI code recently and saving the
time, after this PR has been reviewed and approved by peer-reviewers, I
will update the code to latest EMC GSI commit, then re-run the
regression tests for final approval.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant