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

Recover the capability of handling model fields from operation gfs.v16.3 #785

Merged
merged 3 commits into from
Aug 22, 2024

Conversation

emilyhcliu
Copy link
Contributor

@emilyhcliu emilyhcliu commented Aug 20, 2024

Description
Make sure GSI can read the background fields from the operational gfs.v16.3
Please see details in issue #782
Resolves #782

I fixed the indentations. GitHub messes up (confuses) the code indentation change.
So, to clarify, my code changes are:

  • fix indentation (no code change, just space change)
  • add a if...end condition to an existing code block

So, the real code change is just two lines (see the following link). The rest of the changes are related to indentation fixes.
3404ddf?diff=split&w=1

Type of change

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

How Has This Been Tested?

The branch was tested on ORION using two sets of ICs on 2024021900:

  • background fields from v17 (/work2/noaa/da/eliu/ICs/UFO_eval/data/para/output_ufo_eval_feb2024/)
  • background fields from operational gfs.v16.3 (/work2/noaa/da/eliu/ICs/GDAS_ops/)

The updated code can handle background fields from both gfs.v16.3 and v17.
The updated code does not change the results of regression tests.

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

@emilyhcliu
Copy link
Contributor Author

Working on regression tests. Will post results here soon.

@emilyhcliu
Copy link
Contributor Author

Results of Regression Test on WCOSS-2:

    Start 1: global_4denvar

1/6 Test #1: global_4denvar ...................   Passed  1685.96 sec
    Start 2: rtma
2/6 Test #2: rtma .............................   Passed  970.86 sec
    Start 3: rrfs_3denvar_rdasens
3/6 Test #3: rrfs_3denvar_rdasens .............   Passed  729.70 sec
    Start 4: hafs_4denvar_glbens
4/6 Test #4: hafs_4denvar_glbens ..............   Passed  1334.85 sec
    Start 5: hafs_3denvar_hybens
5/6 Test #5: hafs_3denvar_hybens ..............   Passed  1335.36 sec
    Start 6: global_enkf
6/6 Test #6: global_enkf ......................   Passed  853.77 sec

@emilyhcliu
Copy link
Contributor Author

@RussTreadon-NOAA Please add @ADCollard and @xincjin-NOAA to review this PR. Thank you.

@emilyhcliu
Copy link
Contributor Author

@RussTreadon-NOAA Please add @ADCollard and @xincjin-NOAA to review this PR. Thank you.

@RussTreadon-NOAA Never mind. I just found out that I can assign reviewers myself. I will do that.

Copy link

@XuanliLi-NOAA XuanliLi-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 to me.
BTW, imp_physics=8 is for Thompson scheme, what value is it for the current GFDL microphysics?

@emilyhcliu
Copy link
Contributor Author

Changes look good to me. BTW, imp_physics=8 is for Thompson scheme, what value is it for the current GFDL microphysics?

The current imp_physics is 11 for GFDL
Thanks for reviewing this PR.

Copy link
Contributor

@xincjin-NOAA xincjin-NOAA left a comment

Choose a reason for hiding this comment

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

looks good to me.

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.

Looks good. Confirm ctest results on Cactus. Also run stand-alone GSI script using operational backgrounds and imp_physics=11. gsi.x ran to completion. Repeat test with imp_physics=8. gsi.x hangs with

rank of data array != variable ndims (or ndims-1)

Something to consider: It is preferable for gsi.x to

  • print an informative error message explaining why the code is hung (e.g., background inconsistent with imp_physics)
  • abort execution

instead of hanging when imp_physics does not match what is in the background atmfXXX.nc file.

As things currently stand, it is not clear from the printout

nid001009.cactus.wcoss2.ncep.noaa.gov 14:  rank of data array != variable ndims (or ndims-1)
nid001009.cactus.wcoss2.ncep.noaa.gov 14: 99
nid001098.cactus.wcoss2.ncep.noaa.gov 44:  rank of data array != variable ndims (or ndims-1)
nid001109.cactus.wcoss2.ncep.noaa.gov 74:  rank of data array != variable ndims (or ndims-1)

why the code is hung.

Not sure if NCO's IT testing will catch this or not.

src/gsi/general_read_gfsatm.f90 Show resolved Hide resolved
src/gsi/general_read_gfsatm.f90 Show resolved Hide resolved
@emilyhcliu
Copy link
Contributor Author

Looks good. Confirm ctest results on Cactus. Also run stand-alone GSI script using operational backgrounds and imp_physics=11. gsi.x ran to completion. Repeat test with imp_physics=8. gsi.x hangs with

rank of data array != variable ndims (or ndims-1)

Something to consider: It is preferable for gsi.x to

  • print an informative error message explaining why the code is hung (e.g., background inconsistent with imp_physics)
  • abort execution

instead of hanging when imp_physics does not match what is in the background atmfXXX.nc file.

As things currently stand, it is not clear from the printout

nid001009.cactus.wcoss2.ncep.noaa.gov 14:  rank of data array != variable ndims (or ndims-1)
nid001009.cactus.wcoss2.ncep.noaa.gov 14: 99
nid001098.cactus.wcoss2.ncep.noaa.gov 44:  rank of data array != variable ndims (or ndims-1)
nid001109.cactus.wcoss2.ncep.noaa.gov 74:  rank of data array != variable ndims (or ndims-1)

why the code is hung.

Not sure if NCO's IT testing will catch this or not.

This is a good suggestion. I will improve the error handling. Thanks for looking into this.

@RussTreadon-NOAA
Copy link
Contributor

@emilyhcliu , it may be easier said than done when it comes to adding code to cleanly abort when imp_physics does not align with the background atmfXXX.nc files. The rank of data array != variable ndims (or ndims-1) error message comes from ncio. The read_vardata calls for nccice and nconrd occur inside subcommunicator blocks. Not all tasks will see the error message. Thus, mpi_abort for mpi_comm_world will hang.

If you see an easy way to obtain the desire behavior, fine ... add it. If not, let's move on.

@emilyhcliu
Copy link
Contributor Author

@emilyhcliu , it may be easier said than done when it comes to adding code to cleanly abort when imp_physics does not align with the background atmfXXX.nc files. The rank of data array != variable ndims (or ndims-1) error message comes from ncio. The read_vardata calls for nccice and nconrd occur inside subcommunicator blocks. Not all tasks will see the error message. Thus, mpi_abort for mpi_comm_world will hang.

If you see an easy way to obtain the desire behavior, fine ... add it. If not, let's move on.

I see. @RussTreadon-NOAA Thanks for the heads up. I will try and let you know.

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.

Looks good to me.

Approve.

@RussTreadon-NOAA RussTreadon-NOAA merged commit 8a1c0e7 into NOAA-EMC:develop Aug 22, 2024
4 checks passed
DavidHuber-NOAA added a commit to DavidHuber-NOAA/GSI that referenced this pull request Sep 6, 2024
* origin/develop:
  Move to contrib spack-stack on Jet (NOAA-EMC#787)
  a quick workaround for increasing the mpi task numbers on orion for ctest :: rrfs_3denvar_rdasens  (NOAA-EMC#788)
  Recover the capability of handling model fields from operation gfs.v16.3 (NOAA-EMC#785)
  fix a bug in deter_sfc_gmi (NOAA-EMC#781)
  add safeguard to thompson_reff (NOAA-EMC#779)
  Fix incorrect usage of real(i_kind) in mg_input.f90  (NOAA-EMC#760)
  Transition to Thompson Microphysics for Microwave All-sky Assimilation (NOAA-EMC#743)
  Format changes for EUMETSAT metop-sg and CADS debug fix (NOAA-EMC#773)
  Update global_4denvar and global_enkf ctests to reflect GFS v17 (NOAA-EMC#774)
  fix for cris-fsr memory corruption (NOAA-EMC#767)
  Gnssrwnd1.0 (NOAA-EMC#747)
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.

Make sure the GSI develop can handle model background fields from current operational system (gfs.v16.3)
4 participants