Bugfix in EnKF assim of conventional q obs

[ClaraDraper-NOAA]

Description

Added code to divide hx_modens by qsat, for consistency with treatment of q as q/qsat.

Fixes issue #544

Type of change

Please delete options that are not relevant.

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

How Has This Been Tested?

I created a test case, in which I assimilated a single sonde flight. Without this bugfix, assimilating conventional q obs from the sonde with the EnKF results in insignificant q increments. With the fix, the q increments match what I'd expect (based on approximating the increments offline, from the diag file and ensemble background). I also ran the enkf assimilation of all standard obs (sats and conventional), with and without the bugfix to test the magnitude of it's impact. The impact is substantial over land (where conventional obs are present). Details:

https://drive.google.com/file/d/19TRrygaR5g2iJ2xMmthfHzxrCCf61TeR/view?usp=sharing

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
• New and existing tests pass with my changes
  The tests will not pass, since the output from assimilating conventional q has been changed.
• [ x] Any dependent changes have been merged and published

[RussTreadon-NOAA]

@ClaraDraper-NOAA , who do you recommend as reviewers for this PR?

[ClaraDraper-NOAA]

@ClaraDraper-NOAA , who do you recommend as reviewers for this PR?

perhaps @jswhit and @CoryMartin-NOAA?

[CoryMartin-NOAA]

@RussTreadon-NOAA I'll take the lead on this one

[RussTreadon-NOAA]

Thank you @CoryMartin-NOAA .

[jswhit]

This looks good to me - it's an important bug fix that should be merged ASAP

[RussTreadon-NOAA]

While I'm not supposed to act as a peer reviewer, it's important to get this bug fix into develop. John's work on minimization problems has identified the moisture channels of hyperspectral IR sensors (iasi & cris) as problematic. He addressed this by adding a weighting factor, optconv, to downweight these channels in the minimization. See issue #375 for details. It would be interesting to see how cycling with the bug fix in this PR impacts gsi.x minimization.

[RussTreadon-NOAA]

Change is straightforward and fixes a bug.

[ClaraDraper-NOAA]

Thanks all. If you run any tests on the impact of this bug, can you please share the results with me? I've also noticed an inconsistency in the way that the EnKF and Var are treating updates to q from T obs. I'll look into whether this has much impact soon.

[CoryMartin-NOAA]

I plan to setup regression tests soon to see if the results change there.

[CoryMartin-NOAA]

The results are in:

    Start 1: [=[global_3dvar]=]
1/9 Test #1: [=[global_3dvar]=] ...............   Passed  1914.45 sec
    Start 2: [=[global_4dvar]=]
2/9 Test #2: [=[global_4dvar]=] ...............   Passed  2130.24 sec
    Start 3: [=[global_4denvar]=]
3/9 Test #3: [=[global_4denvar]=] .............   Passed  2632.53 sec
    Start 4: [=[hwrf_nmm_d2]=]
4/9 Test #4: [=[hwrf_nmm_d2]=] ................   Passed  3192.03 sec
    Start 5: [=[hwrf_nmm_d3]=]
5/9 Test #5: [=[hwrf_nmm_d3]=] ................   Passed  1398.73 sec
    Start 6: [=[rtma]=]
6/9 Test #6: [=[rtma]=] .......................   Passed  2067.66 sec
    Start 7: [=[rrfs_3denvar_glbens]=]
7/9 Test #7: [=[rrfs_3denvar_glbens]=] ........   Passed  529.33 sec
    Start 8: [=[netcdf_fv3_regional]=]
8/9 Test #8: [=[netcdf_fv3_regional]=] ........   Passed  2722.26 sec
    Start 9: [=[global_enkf]=]
9/9 Test #9: [=[global_enkf]=] ................***Failed  971.32 sec

89% tests passed, 1 tests failed out of 9

Unsurprisingly, this alters the results of the EnKF regression test. @RussTreadon-NOAA what is our process for updating this? Do I need to first make sure the changes are expected and then pass the updated files to you somehow?

[RussTreadon-NOAA]

@CoryMartin-NOAA , GSI ctests currently do not have reference tests results against which PR results are compared.

Instead, we are still using the old GSI ctest approach of running a control (usually develop) and an update (the PR). The global_enkf ctest fails if checks in regression/regression_test_enkf.sh indicate a fail. Some of the checks in regression_test_enkf.sh and regression_test.sh need revisiting. A fail isn't always a fatal fail.

I checked /scratch1/NCEPDEV/stmp2/Cory.R.Martin/pr545/tmpreg_global_enkf/tmpreg_global_enkf/global_enkf_loproc_updat_vs_gl/global_enkf_regression_results.txt. The ctest failure is due to

The results between the two runs are nonreproducible,
thus the regression test has Failed mean anal for global_enkf_loproc_updat and global_enkf_loproc_contrl analyses with 36 lines different.

This is an accurate message. compare_ncfile.py of incr_2022110900_fhr03_mem001, a sample member increment file, for the update and control show differences in several fields. This is expected.

The change to src/enkf/readconvobs.f90 will definitely alter enkf.x results when processing obtype == ' q'.

Comparison of the update and control stdout show the initial difference to be in the q observation stats (< is update, > is control)

959,961c959,961
< NH     all q  16647  0.199E-01  0.137E+00  0.196E+00  0.367E-01  0.193E+00
< TR     all q   4919  0.297E-01  0.150E+00  0.204E+00  0.410E-01  0.200E+00
< SH     all q   1053  0.542E-02  0.155E+00  0.205E+00  0.447E-01  0.200E+00
---
> NH     all q  16647  0.199E-01  0.137E+00  0.237E+01  0.236E+01  0.193E+00
> TR     all q   4919  0.297E-01  0.150E+00  0.237E+01  0.236E+01  0.200E+00
> SH     all q   1053  0.542E-02  0.155E+00  0.221E+01  0.220E+01  0.200E+00

Based on what I see the differences look acceptable.

[CoryMartin-NOAA]

Given the results of the regression tests have acceptable and expected differences, I am approving this bugfix. If no objections from the other code managers, we can merge this in soon.

[RussTreadon-NOAA]

I'm fine with merging. I don't think this one routine change will cause many, if any, conflicts with existing PRs.