GitHub Issue NOAA-EMC/GSI#658 Issues of undefined values and thread safety in intrad.f90

[shoyokota]

Description

This PR fixes the following issues of undefined values and thread safety found in intrad.f90.

• tdir(1:nsigradjac) is undefined for variables except for (tsen,q,oz,cw,ql,qi,qh,qg,qr,qs,u,v,sst). (e.g., w: vertical velocity)
• val_quad and mm that should be private variables of OpenMP are shared variables.

Fixes #658

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How Has This Been Tested?

EnVar tests with the RRFS setting (Radar reflectivity is simultaneously assimilated with the other observations using SDL/VDL) were done on Orion. This fix didn't change the result.

Checklist

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• New and existing tests pass with my changes
• Any dependent changes have been merged and published

[RussTreadon-NOAA]

@shoyokota , thank you for catching this bug. Who would you like to be peer reviewers on this PR?

[RussTreadon-NOAA]

I added myself as the Handling Reviewer. We still need two peer reviewers.

[JacobCarley-NOAA]

@RussTreadon-NOAA @shoyokota I am pretty familiar with the bug/issue and am happy to review.

[RussTreadon-NOAA]

@RussTreadon-NOAA @shoyokota I am pretty familiar with the bug/issue and am happy to review.

Thank you, @JacobCarley-NOAA . I'll add you as a reviewer.

[JacobCarley-NOAA]

Changes look good. OMP directive changes appear as they will only take effect when use_corr_obs=.true. which I don't think is the case for RRFS, hence no impact on RRFS test results.

[shoyokota]

@JacobCarley-NOAA Thank you for reviewing it!

@RussTreadon-NOAA I think @TingLei-NOAA and @ShunLiu-NOAA are suited for the reviewer because they also investigated this bug.

[RussTreadon-NOAA]

WCOSS2 ctests
Build shoyokota:feature/PR_NOAA-EMC_EnVar-intrad on Cactus. Run ctests with following results

Test project /lfs/h2/emc/da/noscrub/russ.treadon/git/gsi/pr659/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: netcdf_fv3_regional ..............   Passed  482.48 sec
2/7 Test #3: rrfs_3denvar_glbens ..............   Passed  484.59 sec
3/7 Test #7: global_enkf ......................   Passed  607.95 sec
4/7 Test #2: rtma .............................   Passed  967.81 sec
5/7 Test #6: hafs_3denvar_hybens ..............   Passed  1210.18 sec
6/7 Test #5: hafs_4denvar_glbens ..............   Passed  1210.42 sec
7/7 Test #1: global_4denvar ...................   Passed  1321.75 sec

100% tests passed, 0 tests failed out of 7

Total Test time (real) = 1321.75 sec

Hera ctests
Build shoyokota:feature/PR_NOAA-EMC_EnVar-intrad on Hera. Run ctests with follow results

Test project /scratch1/NCEPDEV/da/Russ.Treadon/git/gsi/pr659/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 #3: rrfs_3denvar_glbens ..............   Passed  487.64 sec
2/7 Test #4: netcdf_fv3_regional ..............   Passed  545.67 sec
3/7 Test #7: global_enkf ......................   Passed  912.11 sec
4/7 Test #2: rtma .............................   Passed  971.42 sec
5/7 Test #6: hafs_3denvar_hybens ..............   Passed  1220.51 sec
6/7 Test #5: hafs_4denvar_glbens ..............   Passed  1280.82 sec
7/7 Test #1: global_4denvar ...................   Passed  1612.60 sec

100% tests passed, 0 tests failed out of 7

Total Test time (real) = 1612.61 sec

@shoyokota , would you please run the ctests on Orion and include the results in this PR?

[RussTreadon-NOAA]

@JacobCarley-NOAA Thank you for reviewing it!

@RussTreadon-NOAA I think @TingLei-NOAA and @ShunLiu-NOAA are suited for the reviewer because they also investigated this bug.

Thank you, @shoyokota , for identifying peer reviewers. @TingLei-NOAA has been added as a reviewer.

[RussTreadon-NOAA]

Changes look correct. Approve.

[ShunLiu-NOAA]

The changes looks good and the bug need to be fixed.

[shoyokota]

I ran the ctests on Orion. Three tests are failed by exceeding maximum allowable threshold time, but all results are identical, so I think it is no problem.

Orion ctests

Test project /home/syokota/da/gsi/ctest/PR_NOAA-EMC_EnVar-intrad/build
    Start 1: global_4denvar
1/7 Test #1: global_4denvar ...................   Passed  3124.34 sec
    Start 2: rtma
2/7 Test #2: rtma .............................***Failed  1519.64 sec
    Start 3: rrfs_3denvar_glbens
3/7 Test #3: rrfs_3denvar_glbens ..............   Passed  610.19 sec
    Start 4: netcdf_fv3_regional
4/7 Test #4: netcdf_fv3_regional ..............   Passed  244.04 sec
    Start 5: hafs_4denvar_glbens
5/7 Test #5: hafs_4denvar_glbens ..............***Failed  2003.41 sec
    Start 6: hafs_3denvar_hybens
6/7 Test #6: hafs_3denvar_hybens ..............   Passed  2182.31 sec
    Start 7: global_enkf
7/7 Test #7: global_enkf ......................***Failed  1721.10 sec

57% tests passed, 3 tests failed out of 7

Total Test time (real) = 11405.07 sec

[RussTreadon-NOAA]

Thank you @shoyokota . Pending Ting's review we can schedule this PR for merger into develop.

[RussTreadon-NOAA]

Thank you @TingLei-NOAA . I'll contact the GSI Handling Review team to schedule merger of this PR into develop.