GitHub Issue NOAA-EMC/GSI#282. GSL enhancements to the non-var cloud analysis.

[hu5970]

This PR address the issue #282:

1. GSL enhancements to the non-var cloud analysis.
2. Update the code generating pseudo moisture observations from cloud observations.

[ShunLiu-NOAA]

Ming,

There are a lot of changes in this PR. My comments are here:

1. Since this PR includes many parameter changes, are these change already tested in any parallel experiment?
2. src/gsi/read_Lightning.f90:
   line 424 to 428, is it possible avoid "stop2(50)", it might potentially cause the operation failure.
3. src/gsi/reorg_metar_cloud.f90:
   Could you please remove the line 284 to 288, line 311 and 312?
   Also, is it possible to not use stop2(50) in line 322?

Shun

[hu5970]

@ShunLiu-NOAA

Thanks for reviewing the code:

1. There are four functions are added (listed in issue 282):
   a. more options to decide how to do T/Q adjustment based on the cloud information.
   Tested with retro and cases
   b. check and adjust Qr/qs/qg and Qnr for each vertical profile to reduce the background reflectivity ghost in final analysis
   This is used in the 3DRTMA parallel tests
   c.Add function to build a vertical depended cloud range for METAR cloud observations.
   This is tested with cases. We did not run parallel test for it and will do it for RRFS.
   d. More options to build q innovation based on the cloud information. This is not part of non-var cloud analysis but is closely related.
   This is tested with cases and retros.

2. stop(50) in read_lightning and reorg_metar_cloud will crash the GSI but it is used to give clear crash point and message when GSI fills data out of allocated memory. We have a way to avoid this by counting observations before allocate memory. There are many GSI observation IO use the current way to read the observations and current method does not cause trouble for RAP/HRRR operation.

3. will clean the code

[GangZhao-NOAA]

Hi Ming ( @hu5970 ),

For the algorithm and code, I do not have any comment.
Just do not understand the usage of "cdata_temp" and "cloudlevel_temp".
From line 278 to line 312, the cdata_temp and cloudlevel_temp are modified and updated, but not output or used. So I am kind of confused.
Anyway, as for the algorithm and code, I have no comment.

Thank you!

• Gang

[hu5970]

Gang,

You are right, Line 325 should be:
cdata_all(k,iout) = cdata_temp(k,ista_min)

So the vertical dependent METAR cloud impact radius can be used.

Thanks,
Ming

[GangZhao-NOAA]

Hi Ming (@hu5970) ,

I have no comments and approve this PR. Just one comment for the "src/gsi/reorg_metar_cloud.f90". The new variables "cdata_temp" and "cloudlevel_temp" are updated but not output or used finally. I do not know why. Are they just used for code testing? I also add a single comment in that code.

In summary, I approve this merge.

Thank you!

• Gang

[MichaelLueken]

Hi Ming,

While attempting to compile these changes, CMake failed due to an incorrect number of subscripts for cdata_temp at line 317. Please correct this and please add (i_kind) to the integer declaration.

[hu5970]

@MichaelLueken-NOAA : Mike, I have fixed the bug and combined the commit logs into one following the instructions from PR 285. Thanks, Ming

[MichaelLueken]

I have compiled the changes in debug mode and there was a variable in reorg_metar_cloud.f90 that was found to be unused. Please remove the unused variable (noted in the review). There are also a few questions related to cleaning up commented out lines that were added in the code review. If the commented out lines won't be used in the future, please remove them (these have been noted in the review). Finally, there is a line in gsdcloudanalysis.F90 that exceeds the 12 character limit (due to the comment on the line). Please move the comment to the line before or after the current line to correct this issue (the line has been noted in the review). The debug tests were run and all successfully ran through to completion.

The debug tests were run and all successfully ran through to completion.

While reviewing the changes, I noted that both this PR and PR #285 have the identical change to ush/build.comgsi. Since PR #285 is currently out to the review committee, that will be going in first. It is possible that the same change being in ush/build.comgsi will cause conflicts when that work is merged to the authoritative repository. You might want to remove the ush/build.comgsi change from this PR to ensure that I can submit this work to the review committee once the first PR is merged.

[hu5970]

@MichaelLueken-NOAA Mike, I have revised the code based on your comments. Please check. Thanks,

[MichaelLueken]

@hu5970 After merging PR #285 to the authoritative repo, there is now a conflict in src/gsi/gsimod.F90. Please merge the latest NOAA-EMC/GSI master into your fork's gsl_nonvar_cloudanalysis branch using:

git remote add upstream https://github.com/NOAA-EMC/GSI.git
git remote update
git rebase upstream/master
Correct the comment conflict in src/gsi/gsimod.F90
git add src/gsi/gsimod.F90
git rebase --continue
git push origin gsl_nonvar_cloudanalysis --force

Once this is complete, I should be able to send this work out to the review committee. Please let me know if you have any questions or encounter any issues.

[hu5970]

@MichaelLueken-NOAA I have solved conflict and rebased the branch. Thanks.

[MichaelLueken]

The due date for feedback from the DA review committee has passed. I will now give final approval to these changes and merge them to the authoritative repository.