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

Iasi ng bufr change #818

Merged
merged 3 commits into from
Dec 18, 2024
Merged

Conversation

wx20jjung
Copy link
Contributor

@wx20jjung wx20jjung commented Dec 16, 2024

Description

The mnemonics to read the IASI-NG data and the corresponding MetImage data were changed by NESDIS. This PR is to change the read_iasing subroutine to use the new mnemonics.
Resolves #815
There are no dependencies with this change.

Two links in the /src/enkf directory were also removed, as they are not necessary.
observer_nmmb.f90 -> observer_reg.f90
observer_wrf.f90 -> observer_reg.f90

Type of change

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

How Has This Been Tested?

NESDIS provided ~6 hours of proxy data containing these new mnemonics. The OBSPROC group (Steve Stegall) generated the two dump files consistent with the global workflow. We tested the our changes on these data.

In order for others to test these changes, modifications to scripts in the global-workflow and channel entries in the satinfo file will be required. The global-workflow script changes are in a separate (global-workflow) pull request. Changes to the satinfo and scaninfo will be in a separate pull request (gsi/fix) once the script changes to the global-workflow are incorporated.

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

wx20jjung and others added 2 commits December 10, 2024 17:03
…These are expected to be identical for the operational stream (from EUMETSAT via NESDIS) and all DBNet data.
@wx20jjung
Copy link
Contributor Author

@ADCollard @DavidHuber-NOAA @emilyhcliu would you review these changes?

@wx20jjung
Copy link
Contributor Author

My ctests on Hera completed as expected.

Test project /scratch1/NCEPDEV/jcsda/Jim.Jung/save/ctests/update/build
Start 1: global_4denvar
Start 2: rtma
Start 3: rrfs_3denvar_rdasens
Start 4: hafs_4denvar_glbens
Start 5: hafs_3denvar_hybens
Start 6: global_enkf
1/6 Test #3: rrfs_3denvar_rdasens ............. Passed 497.71 sec
2/6 Test #2: rtma ............................. Passed 981.02 sec
3/6 Test #5: hafs_3denvar_hybens .............. Passed 1105.12 sec
4/6 Test #6: global_enkf ...................... Passed 1149.28 sec
5/6 Test #4: hafs_4denvar_glbens .............. Passed 1289.76 sec
6/6 Test #1: global_4denvar ................... Passed 1965.64 sec

100% tests passed, 0 tests failed out of 6

Total Test time (real) = 1965.66 sec

One of my ctests on wcoss (global_4denvar) failed due to not having permission to use restricted data (RSTPROD).

@DavidHuber-NOAA DavidHuber-NOAA self-requested a review December 16, 2024 18:19
Copy link
Collaborator

@DavidHuber-NOAA DavidHuber-NOAA left a comment

Choose a reason for hiding this comment

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

Changes look good. Just one request that you also remove the two commented-out lines in src/enkf/enkf_files.cmake that reference observer_nmmb.f90 and observer_wrf.f90.

@RussTreadon-NOAA
Copy link
Contributor

WCOSS2 ctests
Install develop at 0921251 and wx20jjung:IASI-NG_BUFR_change at 57d3e98 on Dogwood. Run ctests with the following results

Test project /lfs/h2/emc/da/noscrub/russ.treadon/git/gsi/pr818/build
    Start 1: global_4denvar
    Start 2: rtma
    Start 3: rrfs_3denvar_rdasens
    Start 4: hafs_4denvar_glbens
    Start 5: hafs_3denvar_hybens
    Start 6: global_enkf
1/6 Test #3: rrfs_3denvar_rdasens .............   Passed  788.28 sec
2/6 Test #6: global_enkf ......................   Passed  913.43 sec
3/6 Test #5: hafs_3denvar_hybens ..............   Passed  1213.72 sec
4/6 Test #4: hafs_4denvar_glbens ..............   Passed  1214.60 sec
5/6 Test #2: rtma .............................   Passed  2530.94 sec
6/6 Test #1: global_4denvar ...................   Passed  2882.98 sec

100% tests passed, 0 tests failed out of 6

Total Test time (real) = 2883.00 sec

@RussTreadon-NOAA
Copy link
Contributor

Orion ctests

Install develop at 092151 and wx20jjung:IASI-NG_BUFR_change at 57d3398 on Orion. Run ctests with the following results

Test project /work2/noaa/da/rtreadon/git/gsi/pr818/build
    Start 1: global_4denvar
    Start 2: rtma
    Start 3: rrfs_3denvar_rdasens
    Start 4: hafs_4denvar_glbens
    Start 5: hafs_3denvar_hybens
    Start 6: global_enkf
1/6 Test #6: global_enkf ......................   Passed  850.53 sec
2/6 Test #3: rrfs_3denvar_rdasens .............   Passed  966.83 sec
3/6 Test #2: rtma .............................   Passed  1696.53 sec
4/6 Test #5: hafs_3denvar_hybens ..............   Passed  2968.41 sec
5/6 Test #4: hafs_4denvar_glbens ..............   Passed  3089.64 sec
6/6 Test #1: global_4denvar ...................   Passed  3603.42 sec

100% tests passed, 0 tests failed out of 6

Total Test time (real) = 3603.90 sec

@RussTreadon-NOAA
Copy link
Contributor

Hercules ctests

Install develop at 0921251 and wx20jjung:IASI-NG_BUFR_change at 57d3e98 on Hercules. Run ctests with the following results:

Test project /work/noaa/da/rtreadon/git/gsi/pr818/build
    Start 1: global_4denvar
    Start 2: rtma
    Start 3: rrfs_3denvar_rdasens
    Start 4: hafs_4denvar_glbens
    Start 5: hafs_3denvar_hybens
    Start 6: global_enkf
1/6 Test #3: rrfs_3denvar_rdasens .............   Passed  30555.97 sec
2/6 Test #6: global_enkf ......................   Passed  30910.00 sec
3/6 Test #5: hafs_3denvar_hybens ..............   Passed  31576.54 sec
4/6 Test #2: rtma .............................   Passed  31868.43 sec
5/6 Test #4: hafs_4denvar_glbens ..............   Passed  31943.67 sec
6/6 Test #1: global_4denvar ...................   Passed  32584.85 sec

100% tests passed, 0 tests failed out of 6

Total Test time (real) = 32584.86 sec

Note the extremely long ctest time - about 9 hours. This is due to extremely long queue times on Hercules.

@RussTreadon-NOAA
Copy link
Contributor

@wx20jjung : None of the ctests exercise the code changed in this PR. You mention above that you tested these changes with NESDIS provided data. I assume the test was successful. It would be good to state this.

You hint at

It is good to explicitly mention these as I did above so that reviewers and code managers better understand what's needed for this capability to be fully exercised.

@RussTreadon-NOAA RussTreadon-NOAA self-requested a review December 17, 2024 12:53
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.

Agree with @DavidHuber-NOAA . Please remove the commented out lines

#observer_nmmb.f90 -- This is soft-linked to observer_reg.f90
#observer_wrf.f90  -- This is soft-linked to observer_reg.f90

at the end of src/enkf/enkf_files.cmake.

The change to src/gsi/read_iasing.f90 is straightforward but I have no way to confirm that the new mnemonic is correct.

Is there EUMETSAT or NESDIS document we can link to this PR or the parent issue #815 to confirm that the RPS mnemonics are correct?

@wx20jjung
Copy link
Contributor Author

@RussTreadon-NOAA I use this page for most of my IASI info.
https://user.eumetsat.int/resources/user-guides/metop-sg-iasi-ng-l1c-and-l1d-data-guide

These are no longer relevant.
@wx20jjung
Copy link
Contributor Author

The two comment lines in src/enkf/enkf_files.cmake
#observer_nmmb.f90 -- This is soft-linked to observer_reg.f90
#observer_wrf.f90 -- This is soft-linked to observer_reg.f90

are removed. Changes are pushed up to github in @wx20jjung branch IASI-NG_BUFR_change

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.

Approve

Copy link
Collaborator

@DavidHuber-NOAA DavidHuber-NOAA left a comment

Choose a reason for hiding this comment

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

LGTM

@RussTreadon-NOAA
Copy link
Contributor

@wx20jjung : This PR can be scheduled for merger into develop after we get one more peer review and approval.

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.

Changes look good

@RussTreadon-NOAA
Copy link
Contributor

Thank you @ADCollard and @DavidHuber-NOAA for your reviews and approvals. I'll work with the GSI Handling Review team to get this PR merged into develop.

@RussTreadon-NOAA RussTreadon-NOAA merged commit 737c6b8 into NOAA-EMC:develop Dec 18, 2024
2 of 4 checks passed
@wx20jjung wx20jjung deleted the IASI-NG_BUFR_change branch December 18, 2024 16:18
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.

IASI-NG BUFR mnemonics changes
4 participants