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

Error in processing VAD winds. #617

Merged
merged 30 commits into from
Sep 1, 2023

Conversation

jderber-NOAA
Copy link
Contributor

@jderber-NOAA jderber-NOAA commented Aug 31, 2023

It was noted that a large number of observations were being eliminated by the setupw routine because there was no observation type. This observation type check was clearly added because of previous problems with this data.

It was found that there was an issue in the read_prepbufr routine because code for vad winds was added in the wrong location. This resulted in many lines containing only zeros to the output file.

The code for processing the vad was moved before the determination of the output line to fix the issue. It was necessary to move a few more lines to make everything work properly. Also one of the checks (there were multiple checks) for a zero observation type was removed from setupw. A check remains, but this condition should not occur unless another error is introduced.

This change results in the removal of many unrealistic observations from the output uv file from prepbufr (when vad winds are used). Because of the existing observation type check analysis results are unchanged.

NOTE: An additional change was made to allow the multiscale ensemble regional test I was using to work (both for the control and modified code). In constants.f90 and gsi_rfv3io_mod.f90 a change was made to allow longer file names to be used for the input ensemble files.

These changes address fixes #615

  • Bug fix (non-breaking change which fixes an issue)

These changes have been tested using the regression tests and a single regional case that is using vad winds. All tests are passed with the only differences being in the output number of wind observation after distribution (the processors containing point 0,0 contained a lot of extra observations from the bug) and the output from the removal of the data by the observation type check.

Note because of the removal of many unrealistic observations in the processing, there appears to be a noticeable reduction in run time when vad winds are used.

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • New and existing tests pass with my changes
  • Any dependent changes have been merged and published

DUE DATE for this PR is 10/12/2023. If this PR is not merged into develop by this date, the PR will be closed and returned to the developer.

@jderber-NOAA jderber-NOAA changed the title Vadbug Error in processing VAD winds. Aug 31, 2023
Copy link
Contributor

@JacobCarley-NOAA JacobCarley-NOAA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm good with the changes pending one very minor comment.

src/gsi/setupw.f90 Show resolved Hide resolved
@RussTreadon-NOAA
Copy link
Contributor

WCOSS2 ctests
Install develop at 9e5aa09 and jderber-NOAA:vadbug at 335c4b2 on Cactus. Run standard suite of 9 ctests with following results

russ.treadon@clogin08:/lfs/h2/emc/da/noscrub/russ.treadon/git/gsi/pr617/build> ctest -j 9
Test project /lfs/h2/emc/da/noscrub/russ.treadon/git/gsi/pr617/build
    Start 1: global_3dvar
    Start 2: global_4dvar
    Start 3: global_4denvar
    Start 4: hwrf_nmm_d2
    Start 5: hwrf_nmm_d3
    Start 6: rtma
    Start 7: rrfs_3denvar_glbens
    Start 8: netcdf_fv3_regional
    Start 9: global_enkf
1/9 Test #8: netcdf_fv3_regional ..............***Failed  484.51 sec
2/9 Test #9: global_enkf ......................   Passed  488.58 sec
3/9 Test #5: hwrf_nmm_d3 ......................   Passed  492.74 sec
4/9 Test #7: rrfs_3denvar_glbens ..............   Passed  605.94 sec
5/9 Test #4: hwrf_nmm_d2 ......................   Passed  607.86 sec
6/9 Test #6: rtma .............................   Passed  1089.33 sec
7/9 Test #3: global_4denvar ...................   Passed  1324.26 sec
8/9 Test #1: global_3dvar .....................   Passed  1442.52 sec
9/9 Test #2: global_4dvar .....................   Passed  1563.17 sec

The netcdf_fv3_regional failure is due to a timing scalability test

The case has Failed the scalability test.
The slope for the update (1.509785 seconds per node) is less than that for the control (1.542529 seconds per node).

The wall times for the updat (jderber-NOAA:vadbug) and contrl (develop) do not show any anomalous behavior

netcdf_fv3_regional_hiproc_contrl/stdout:The total amount of wall time                        = 63.881463
netcdf_fv3_regional_hiproc_updat/stdout:The total amount of wall time                        = 63.013349
netcdf_fv3_regional_loproc_contrl/stdout:The total amount of wall time                        = 65.423992
netcdf_fv3_regional_loproc_updat/stdout:The total amount of wall time                        = 64.221177

This is not a fatal fail.

Check the nkeep value in fort.202. nkeep is the number of wind observations passed out of read_prepbufr.f90 for processing by setup routines. Find that the updat code consistently reduces the number of observation kept by read_prepbufr

contrl updat
global_3dvar 3168472 3147394
global_4denvar 3172060 3150982
netcdf_fv3_regional 323180 321662
rrfs_3denvar_glbens 159146 157394

While the number of kept observations decreases in updat, the number of assimilated observations is the same as contrl. This is consistent with the explanation of the bug and it's fix in this PR.

Copy link
Contributor

@JacobCarley-NOAA JacobCarley-NOAA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

src/gsi/setupw.f90 Show resolved Hide resolved
@jderber-NOAA
Copy link
Contributor Author

Hera ctests

Regression tests were run on Hera earlier. The results appear to be essentially identical to those Russ ran on WCOSS. Even the same test failed for timing reasons. Also, I checked the nkeep values on Hera and they are again identical as would be expected.

Copy link
Contributor

@RussTreadon-NOAA RussTreadon-NOAA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two peer reviewers have looked over changes and approved. WCOSS2 (Cactus) and Hera ctests pass with expected results.

Approve.

@RussTreadon-NOAA RussTreadon-NOAA merged commit be4a3d9 into NOAA-EMC:develop Sep 1, 2023
4 checks passed
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.

Bug in read_prepbufr.f90
4 participants