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.

Sign up for GitHub

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

Jump to bottom

Address debug build and run issues for HAFS GSI #661

Closed
BijuThomas-NOAA opened this issue Nov 29, 2023 · 8 comments · Fixed by #679
Closed

Address debug build and run issues for HAFS GSI #661

BijuThomas-NOAA opened this issue Nov 29, 2023 · 8 comments · Fixed by #679
Assignees
@BijuThomas-NOAA

Comments

@BijuThomas-NOAA
Copy link

The HAFS application regression tests failed at the GSI level on the Jet platform and upon debugging, it appears that the variables toff and t4dv in the subroutine read_radar_l2rw_novadqc(read_radar.f90) were uninitialized and get assigned NaN values. Not totally sure that these are the reasons for HAFS RT failures on
Jet, but it would be helpful if these variables could be properly initialized.
@BijuThomas-NOAA BijuThomas-NOAA mentioned this issue Nov 29, 2023
Merged
9 tasks
@RussTreadon-NOAA
Copy link
Contributor

@BijuThomas-NOAA , may this issue be closed?

@BijuThomas-NOAA
Copy link
Author

@RussTreadon-NOAA Would like to see this issue be fixed.

@RussTreadon-NOAA
Copy link
Contributor

@BijuThomas-NOAA , it is fine to leave the issue open. We no longer have a GSI code manager. All changes originate from developers. Therefore, please have someone on your team our yourself open a PR to fix the problem identified in this issue.

@RussTreadon-NOAA
Copy link
Contributor

This issue will be closed in six weeks (January 23, 2024) if it remains inactive at that time.

@BijuThomas-NOAA
Copy link
Author

@RussTreadon-NOAA Someone in our HAFS team will look into this issue. Thanks

@RussTreadon-NOAA
Copy link
Contributor

Great, thank you @BijuThomas-NOAA , for the update.

@XuLu-NOAA
Copy link
Contributor

Hi, @RussTreadon-NOAA and @BijuThomas-NOAA ,

  1. The toff is the time offset based on the start of the DA time window. In the current code, toff is not properly assigned and will result in all the ground radar observations to be considered at -3h when using FGAT or 4DEnVar. This is an error and will affect the DA results for those radar obs. The right fix is to use time_offset from obsmod.f90 to correct the time labels for each ob.
  2. The t4dv is a variable used in the read_radar subroutine, not in read_radar_l2rw_novadqc. In the read_radar_l2rw_novadqc subroutine, the right fix is to use t4dv0 instead of t4dv.
  3. I also found other uninitialized variables that will fail the GSI in debug mode: zsges in read_radar_l2rw_novadqc (read_radar.f90) should be calculated using the deter_zsfc_model subroutine, and dlnpsob in read_fl_hdob.f90 should be assigned with a value of 1000 since the SFMR observations do not provide any surface pressure sampling.

@RussTreadon-NOAA
Copy link
Contributor

Thank you @XuLu-NOAA for looking into this issue, identifying the reason for existing and new problems, and proposing solutions. To move forward, can someone fork off the current head of GSI develop, commit the proposed fixes to the fork, and open a PR to merge the fork into develop?

@XuLu-NOAA XuLu-NOAA mentioned this issue Jan 9, 2024
Merged
@jderber-NOAA jderber-NOAA mentioned this issue Jan 18, 2024
Merged
@BinLiu-NOAA BinLiu-NOAA changed the title uninitialized variables (develop) Address debug build and run issues for HAFS GSI Jan 26, 2024
ShunLiu-NOAA pushed a commit that referenced this issue Mar 27, 2024
@XuLu-NOAA
Fix HAFS GSI debug build and run issues (#679)
b53740a 
**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 #661

Fixes #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 #4: [=[netcdf_fv3_regional]=] ........   Passed  365.11 sec
2/7 Test #7: [=[global_enkf]=] ................   Passed  430.29 sec
3/7 Test #3: [=[rrfs_3denvar_glbens]=] ........   Passed  605.35 sec
4/7 Test #2: [=[rtma]=] .......................   Passed  969.78 sec
5/7 Test #6: [=[hafs_3denvar_hybens]=] ........***Failed  1455.47 sec
6/7 Test #1: [=[global_4denvar]=] .............   Passed  1682.40 sec
7/7 Test #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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@BijuThomas-NOAA BijuThomas-NOAA

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix HAFS GSI debug build and run issues hafs-community/GSI
3 participants
@BijuThomas-NOAA @XuLu-NOAA @RussTreadon-NOAA