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

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

Open
KariA-Spire opened this issue Sep 22, 2023 · 23 comments · May be fixed by #747
Open

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

KariA-Spire opened this issue Sep 22, 2023 · 23 comments · May be fixed by #747
Assignees

Comments

@KariA-Spire
Copy link

KariA-Spire commented Sep 22, 2023

Description

Type of Change

  • New GNSS-R ocean wind speed observation operator and assimilation capability in GSI
  • This source code can directly assimilate GNSS-R L2 ocean wind speed retrievals from commercial Spire Global, Inc. and NASA/CYGNSS satellites
  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?
Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • Any dependent changes have been merged and published

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
Copy link
Contributor

@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
Copy link
Contributor

@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
Copy link
Author

@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
Copy link
Contributor

RussTreadon-NOAA commented May 7, 2024

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
Copy link
Contributor

RussTreadon-NOAA commented May 8, 2024

@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
Copy link
Contributor

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

  • src/gsi/gsi_files.cmake - list the new source files you added to the code base
  • src/gsi/read_gnssrspd.f90 - remove map3grids from the use convthin line. map3grids is not in the code base. read_gnssrspd.f90 does not use map3grids.
  • src/gsi/statsconv.f90 - add a missing end if statement

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
Copy link
Author

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
Copy link
Contributor

@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
Copy link
Contributor

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

  • the name of the new data dump file needs to be added to GSI run script(s)
  • the new data needs to be added to the OBS_INPUT namelist
  • an entry (or entries) need needs to be added to the convinfo file

@KariA-Spire
Copy link
Author

KariA-Spire commented May 9, 2024

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:

  • the name of the new data dump file needs to be added to GSI run script(s)

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.

  • the new data needs to be added to the OBS_INPUT namelist

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

  • an entry (or entries) need needs to be added to the convinfo file
    !otype type sub iuse twindow numgrp ngroup nmiter gross ermax ermin var_b var_pg ithin rmesh pmesh npred pmot ptime ib ip
    gnssrspd 240 0 1 3.0 0 0 0 8.0 6.1 1.4 8.0 0.000000 0 0. 0. 0 0. 0. 0 0

I could also provide BUFR dump files to facilitate testing.

@RussTreadon-NOAA
Copy link
Contributor

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
Copy link
Author

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

@RussTreadon-NOAA
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

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
Copy link
Author

Done!

@RussTreadon-NOAA
Copy link
Contributor

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
Copy link
Contributor

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

@KariA-Spire
Copy link
Author

Thank you @RussTreadon-NOAA for your guidance!

@RussTreadon-NOAA
Copy link
Contributor

@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
Copy link
Contributor

@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 KariA-Spire linked a pull request May 13, 2024 that will close this issue
@KariA-Spire
Copy link
Author

@RussTreadon-NOAA, thanks for the reminder. I created the pull request: #747

@RussTreadon-NOAA
Copy link
Contributor

Thank you @KariA-Spire

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 a pull request may close this issue.

2 participants