NOAA-EMC / GSI

Gridpoint Statistical Interpolation
GNU Lesser General Public License v3.0
66 stars 148 forks source link

GNSS-R L2 ocean wind speed assimilation (Spire Global, Inc. and NASA/CYGNSS) #628

Closed KariA-Spire closed 2 months ago

KariA-Spire commented 1 year ago

Description

Type of Change

How Has This Been Tested? Checklist

File Changes

Link to the forked branch:https://github.com/KariA-Spire/GSI/tree/gnssrwnd1.0

Note: Upon request, we can facilitate source code for NCEP-flavor BUFR encoders and JEDI/IODA converters. We can also guide users on observation error and bias correction treatment on an "as-time-permits" basis. Spire Global, Inc. developed this assimilation capability under the NASA CYGNSS ROSES-2020 Grant number 80NSSC21K1120.

RussTreadon-NOAA commented 4 months ago

@KariA-Spire , please note the following information posted on the GSI wiki

The Unified Forecast System (UFS) will use the Joint Effort for Data assimilation Integration (JEDI) for its DA infrastructure. JEDI components, as they mature, will incrementally replace operational GSI-based components.

Given this, the focus of the NOAA-EMC/GSI repository has shifted to operational support during this transition. NOAA-EMC/GSI only supports current and planned operational NWS realizations of the GSI. Changes to NOAA-EMC/GSI must have a clear path to implementation with agreement from operational GSI application leads.

RussTreadon-NOAA commented 4 months ago

@KariA-Spire , your branch, gnssrwnd, is currently 44 commits behind the head of NOAA-EMC/GSI:develop. Please bring your branch up to date with the head of develop.

KariA-Spire commented 4 months ago

@RussTreadon-NOAA, I deleted gnssrwnd and created a new branch https://github.com/KariA-Spire/GSI/tree/gnssrwnd1.0, which is synced with develop.

RussTreadon-NOAA commented 4 months ago

Please review information posted under the GSI Wiki and follow the outlined procedure to move this issue forward. Key pages include Code Management Policy and How to Make Changes.

RussTreadon-NOAA commented 4 months ago

@KariA-Spire : Attempts to compile gnssrwnd1.0 on WCOSS2 (Cactus) fail with numerous errors

/lfs/h2/emc/da/noscrub/russ.treadon/git/gsi/is628/src/gsi/obsmod.F90(486): error #6251: This symbol has multiple PUBLIC statement/attribute declarations which is not allowed.   [VR_DEALISINGOPT]
  public :: vr_dealisingopt, if_vterminal, if_model_dbz, if_vrobs_raw, if_use_w_vr, l2rwthin
------------^

...

/lfs/h2/emc/da/noscrub/russ.treadon/git/gsi/is628/src/gsi/obsmod.F90(613): error #6418: This name has already been assigned a data type.   [IOUT_WSPD10M]
  integer(i_kind) iout_wspd10m,iout_td2m,iout_mxtm,iout_mitm,iout_pmsl,iout_howv
------------------^

Additionally, several routines contain sections which look to be unresolved git conflicts. For example, intjo.f90

<<<<<<< HEAD
  iobOper_rw,         iobOper_dbz,                                                                      &
  iobOper_spd,        iobOper_gnssrspd,     iobOper_oz,         iobOper_o3l,        iobOper_colvk,      &
=======
  iobOper_rw,         iobOper_dbz,        iobOper_fed,                                                  &
                      iobOper_spd,        iobOper_oz,         iobOper_o3l,        iobOper_colvk,        &
>>>>>>> upstream/develop

Which NOAA RDHPCS machine(s) can you access?

RussTreadon-NOAA commented 4 months ago

@KariA-Spire , thank you for updating your branch. I updated my working copy to 5f7f4dde. The WCOSS2 build still failed. The following modifications were needed

If you have access to a NOAA RDHPCS machine I can copy the above modifications to a place where you can see them. Alternatively I can attempt to push to your branch but I'm not sure if this will work.

KariA-Spire commented 4 months ago

Hi @RussTreadon-NOAA , Thank you for compiling the source code on WCOSS2. Unfortunately, I don't have access to NOAA RDHPCS machines. I updated the branch with the modifications I think you did. Hopefully I got them all correctly. If not, please let me know. Thank you again and best regards.

RussTreadon-NOAA commented 4 months ago

@KariA-Spire , I updated my working copy of gnssrwnd1.0. While your branch at 1aa6743 compiles, I don't think the all if-end if blocks in src/gsi/statsconv.f90 are correct.

An end if is needed on line 680 to close the if(mype==mype_wspd10m) then block which begins on line 642.

The end if added on line 757 at 1aa6743 is incorrect. The end if on line 756 closes the if(mype==mype_td2m) then block which begins on line 720.

Please check your working copy of statsconv.f90 at 1aa6743 to see if you agree.

RussTreadon-NOAA commented 4 months ago

The initial attempt to run the new code through ctests on WCOSS2 was unsuccessful. All ctests running gsi.x aborted with the traceback

forrtl: severe (153): allocatable array or pointer is not allocated
Image              PC                Routine            Line        Source
gsi.x              00000000022D2644  Unknown               Unknown  Unknown
gsi.x              00000000006DBEB1  m_rhs_mp_rhs_deal         243  m_rhs.F90
gsi.x              0000000000B27A76  setuprhsall_              639  setuprhsall.f90
gsi.x              00000000010A9FAD  glbsoi_                   323  glbsoi.f90
gsi.x              000000000065B807  gsisub_                   200  gsisub.F90
gsi.x              000000000041367D  gsimod_mp_gsimain        2431  gsimod.F90
gsi.x              00000000004135B7  MAIN__                    634  gsimain.f90

Line 243 of m_rhs.F90 is

deallocate(rhs_split_gps)

A search for rhs_split_gps found the following occurrences

./m_rhs.F90:  public:: rhs_split_gps
./m_rhs.F90:  integer(i_kind),allocatable,dimension(:    ),save:: rhs_split_gps
./m_rhs.F90:  deallocate(rhs_split_gps)

The array is declared and deallocated but it is never allocated or initialized. More importantly, rhs_split_gps is not referenced outside of m_rhs.F90. As such, I removed rhs_split_gps from m_rhs.F90, recompiled, and reran the ctests. This time all ctests passed.

Test project /lfs/h2/emc/da/noscrub/russ.treadon/git/gsi/is628/build
    Start 4: netcdf_fv3_regional
    Start 1: global_4denvar
    Start 5: hafs_4denvar_glbens
    Start 6: hafs_3denvar_hybens
    Start 2: rtma
    Start 7: global_enkf
    Start 3: rrfs_3denvar_glbens
1/7 Test #4: netcdf_fv3_regional ..............   Passed  483.18 sec
2/7 Test #3: rrfs_3denvar_glbens ..............   Passed  485.61 sec
3/7 Test #7: global_enkf ......................   Passed  851.66 sec
4/7 Test #2: rtma .............................   Passed  968.98 sec
5/7 Test #6: hafs_3denvar_hybens ..............   Passed  1154.17 sec
6/7 Test #5: hafs_4denvar_glbens ..............   Passed  1213.48 sec
7/7 Test #1: global_4denvar ...................   Passed  1682.81 sec

100% tests passed, 0 tests failed out of 7

Total Test time (real) = 1682.97 sec

The above was repeated on NOAA RDHPCS Hera with all tests passing on Hera.

Test project /scratch1/NCEPDEV/da/Russ.Treadon/git/gsi/is628/build
    Start 1: global_4denvar
    Start 5: hafs_4denvar_glbens
    Start 2: rtma
    Start 6: hafs_3denvar_hybens
    Start 7: global_enkf
    Start 4: netcdf_fv3_regional
    Start 3: rrfs_3denvar_glbens
1/7 Test #4: netcdf_fv3_regional ..............   Passed  548.59 sec
2/7 Test #3: rrfs_3denvar_glbens ..............   Passed  554.27 sec
3/7 Test #7: global_enkf ......................   Passed  941.55 sec
4/7 Test #2: rtma .............................   Passed  1032.06 sec
5/7 Test #6: hafs_3denvar_hybens ..............   Passed  1115.26 sec
6/7 Test #5: hafs_4denvar_glbens ..............   Passed  1472.35 sec
7/7 Test #1: global_4denvar ...................   Passed  1987.88 sec

100% tests passed, 0 tests failed out of 7

Total Test time (real) = 1987.93 sec

None of the existing ctests include GNSS-R ocean wind speed observations. As such, the above ctests do not address the question as to whether or not the new code actually works as intended.

Even if a bufr dump file containing the new data were available additional changes are needed

KariA-Spire commented 4 months ago

As for the additional changes, if you point me to NOAA-specific runscript, OBS_INPUT namelist, and convinfo files I'd be happy to update them.

Meanwhile, the following lines need to be added to the specified scripts:

Files like "gfs.t12z.spirewnd.tm00.bufr_d" or "gfs.t12z.cygnswnd.tm00.bufr_d need to be linked to gnssrwndbufr in a runscript for GSI processing.

OBS_INPUT:: ! dfile dtype dplat dsis dval dthin dsfcalc gnssrwndbufr gnssrspd null gnssrspd 0.0 0 0

I could also provide BUFR dump files to facilitate testing.

RussTreadon-NOAA commented 4 months ago

GSI scripts and fix files are managed in other repositories. It is sufficient at this time if you can confirm from your side that the code in gnssrwnd1.0 behaves as expected.

KariA-Spire commented 4 months ago

I can confirm that the code in gnssrwnd1.0 behaves as expected in Spire's DA system.

RussTreadon-NOAA commented 4 months ago

What do you think about rhs_split_gps in src/gsi/m_rhs.F90?

Execution on WCOSS2 and Hera fails because array rhs_split_gps is deallocated before it is allocated. I removed rhs_split_gps from m_rhs.F90. If this is the correct change, please commit this change to m_rhs.F90.

KariA-Spire commented 4 months ago

The rhs_split_gps array is unrelated to the gnssrwnd1.0 development. I can commit the change to m_rhs.90 if you think that's best.

RussTreadon-NOAA commented 4 months ago

If we leave rhs_split_gps in m_rhs.F90 as-is, gsi.x aborts with a segmentation fault. This is not acceptable. Allocating and initializing rhs_split_gps in m_rhs.F90 is another option. I opted to not do this since rhs_split_gps is not used elsewhere in the code.

KariA-Spire commented 4 months ago

Done!

RussTreadon-NOAA commented 4 months ago

Problems remain with statsconv.f90. Please see comments here. I see your thumbs up to the comment, but I do not see any changes to statsconv.f90 in gnssrwnd1.0

RussTreadon-NOAA commented 4 months ago

Thank you @KariA-Spire for updating statsconv.f90at 869c61f

KariA-Spire commented 4 months ago

Thank you @RussTreadon-NOAA for your guidance!

RussTreadon-NOAA commented 4 months ago

@KariA-Spire , the next step is to open a GSI Pull Request. This is step 4 under GSI wiki page GSI:-How-to-Make-Changes

RussTreadon-NOAA commented 4 months ago

@KariA-Spire, please open a GSI pull request if you would like the changes in gnssrwnd1.0 to be considered for merger into the authoritative GSI develop.

KariA-Spire commented 4 months ago

@RussTreadon-NOAA, thanks for the reminder. I created the pull request: https://github.com/NOAA-EMC/GSI/pull/747

RussTreadon-NOAA commented 4 months ago

Thank you @KariA-Spire