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

GitHub Issue NOAA-EMC/GSI#237: merge release/gfsda.v16.x at a723db1 into master #240

Merged
merged 1 commit into from
Nov 3, 2021

Conversation

RussTreadon-NOAA
Copy link
Contributor

Issue #237 documents the merge of the authoritative master at 2f28fbf into a forked copy of the authoritative release/gfsda.v16.x at 3898ab4. The merge was done in @RussTreadon-NOAA 's forked release/gfsda.v16.x and completed at a723db1. From here, the forked release/gfsda.v16.x was squash merged into the forked master. Done at d4645c1

This PR is opened to merge the forked master into the authoritative master.

@RussTreadon-NOAA
Copy link
Contributor Author

The libsrc and fix originally associated with the forked master only contain files and directories needed to build global DA. The authoritative master supports both global and regional DA. The recommended action is to revert libsrc and fix to the authoritative master. gfsda.v16.x updates to fix and libsrc will be brought into the master after this PR is closed.

Toward this end execute the following to revert updated global only libsrc and fix back to the authoritative master:

git clone --recursive git@github.com:RussTreadon-NOAA/GSI.git .
git reset --soft HEAD~1
git reset HEAD fix libsrc
git commit -m "GitHub Issue NOAA-EMC/GSI #237: merge release/gfsda.v16.x at a723db1 into master"
git push origin master --force

Done at 39abaf9.

Copy link
Collaborator

@CatherineThomas-NOAA CatherineThomas-NOAA left a comment

Choose a reason for hiding this comment

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

The changes are as expected. Approved.

Copy link
Contributor

@ADCollard ADCollard left a comment

Choose a reason for hiding this comment

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

Correlated obs changes look correct

Copy link
Contributor

@HaixiaLiu-NOAA HaixiaLiu-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 will check the fix files later

@RussTreadon-NOAA
Copy link
Contributor Author

Haixia's suggestions have been incorporated into this PR. Done at 61e6458.

  • add ompslp_npp to namelist ozobs_enkf in exdgas_enkf_update.sh
  • add iasi_metop-c to namelist satobs_enkf in exgdas_enkf_update.sh

@RussTreadon-NOAA
Copy link
Contributor Author

Test revised exgdas_enkf_update.sh using gfs.v16.1.5 workflow and PR #240 for DA. Haixia provided a copy of global_ozinfo.txt which assimilates ompslp_npp. Ran eobs, ediag, and eupd for 2021101900 using operational input. global_enkf.x aborted when processing the ompslp_npp netcdf diagnostic file with the error

[1]  **   ERROR: The specified variable 'Analysis_Use_Flag' does not exist!
[1]  ** Failed to read NetCDF4.

ncdump -h diag_ompslp_npp_ges.2021101900_ensmean.nc4 confirmed that this variable is not in the diagnostic file. A check of other ozone diagnostic files found Analysis_Use_Flag for other sources of ozone observations.

Examine src/gsi/setupoz.f90. Subroutine setupozlev processes ompslp data. The diagnostic file populated and written from this subroutine differs from what is created and written from subroutine setupozlay. Subroutine setupozlay processes omi, ompsnp, and ompstc8.

Report these findings to Haixia. Agree to remove ompslp_npp entry from namelist ozobs_enkf in exdgas_enkf_update.sh. Rerun eupd after doing this. global_enkf.x ran to completion. Metop-C IASI data was assimilated by global_enkf.x

The removal of ompslp_npp from namelist ozobs_enkf in exdgas_enkf_update.sh was done at 214b60f.

Subroutine setupozlay will be updated to generate a complete netcdf diagnostic file, tested, and submitted via a separate PR.

Copy link
Contributor

@MichaelLueken MichaelLueken left a comment

Choose a reason for hiding this comment

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

I have completed my visual review of the changes in this PR. There are some minor issues that should be addressed:

Two alignment issues in src/gsi/correlated_obsmod.F90

Should three commented out line be kept or removed in src/gsi/correlated_obsmod.F90

Replacing .eq. with == in src/gsi/setupoz.f90

Adding proper precision to a real value

The regression tests were run and the behavior matched the results you documented in issue #237.

The code was compiled in debug mode and no new warning messages were encountered. The debug tests were run and all ran through to completion.

Once these five issues have been addressed, I will submit this work to the DA review committee.

@RussTreadon-NOAA RussTreadon-NOAA force-pushed the master branch 3 times, most recently from a1f356a to 2acd89e Compare October 22, 2021 23:06
@RussTreadon-NOAA
Copy link
Contributor Author

2acd89e adds a two line change to read_nsstbufr.f90.

        else
           cycle read_loop

This block catches the case in which an unexpected bufr subset is found in the nsstbufr file. This blocks acts as a none of the above catch to prevent the recurrence of problems which happened in operations.

@RussTreadon-NOAA
Copy link
Contributor Author

c6b17ce added the following changes

  • scripts/exgdas_enkf_update.sh - add ompsnp_n20 and ompslp_npp to namelist ozobs_enkf
  • src/enkf/innovstats.f90 - replace "sbuv oz" with "all oz" in run time printout
  • src/gsi/setupoz.f90 - add missing data to netcdf diagnostic file for ompslp

Haixia developed and tested the changes in src/gsi/setupoz.f90. This work is in response to the earlier comment in this PR which found that eupd aborted when processing ompslp data. Haixia test her changes for 2021101900. I tested her changes for 2021102412. eupd successfully ran to completion with ompslp data present and processed.

@HaixiaLiu-NOAA
Copy link
Contributor

@RussTreadon-NOAA Thank you for testing the setupoz.f90 changes and confirm the changes are right.

Copy link
Contributor

@MichaelLueken MichaelLueken left a comment

Choose a reason for hiding this comment

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

No new issues were seen while visually reviewing the changes. While attempting to compile the update, however, the EnKF failed to compile with the following error:

/gpfs/dell2/emc/modeling/noscrub/Michael.Lueken/update/src/enkf/innovstats.f90(222): error #7938: Character length argument mismatch. [' all oz']
call printstats(' all oz',sumoz_nh,biasoz_nh,sumoz_spread_nh,sumoz_oberr_nh,nobsoz_nh,&
-------------------^

Please add two more spaces to ensure proper length (character(len=9)) to:

' all oz'

Once this is complete, I will be able to submit this work to the review committee.

@MichaelLueken
Copy link
Contributor

No feedback has been received from the DA review committee, so I will now give final approval to these changes and merge them to the authoritative master. Once merged, I will begin merging the fix file and libsrc modifications to rev2 and master of these submodules, respectively.

@MichaelLueken MichaelLueken changed the title GitHub Issue NOAA-EMC/GSI #237: merge release/gfsda.v16.x at a723db1 … GitHub Issue NOAA-EMC/GSI#237: merge release/gfsda.v16.x at a723db1 into master Nov 3, 2021
@MichaelLueken MichaelLueken merged commit 33d61b1 into NOAA-EMC:master Nov 3, 2021
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.

Merge master into release/gfsda.v16.x
5 participants