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

Add logic to switch between JEDI and GSI ice increment names #43

Closed

Conversation

DavidNew-NOAA
Copy link
Contributor

This PR is fixes the following problem NOAA-EMC/GDASApp#1112 (comment) . It creates logic so that if JEDI is the DA system, then "ice_wat_inc" is used as the ice increment variable name, and if GSI is the DA system, then "icmr_inc" is used. A "jedi", logical namelist parameter for src/netcdf_io/interp_inc.fd/driver.F90 is added which defaults to .false. (GSI; else JEDI). Once this PR is merged, a single line can be added to ush/calc_analysis.py, building on what was done in GW #2420 via GSI Utils #35, to specify "jedi" in the namelist.

@DavidNew-NOAA
Copy link
Contributor Author

Closing temporarily, because I did this too fast and I'm missing a bunch of logic

@DavidNew-NOAA DavidNew-NOAA reopened this May 21, 2024
@DavidNew-NOAA
Copy link
Contributor Author

@RussTreadon-NOAA Do you understand what the failed "GCC Linux Build / gsi-monitor" test is about? I'm looking at the log but having trouble understanding what went wrong.

@RussTreadon-NOAA
Copy link
Contributor

@DavidNew-NOAA , I am seeing the similar failures with the GSI. I reached out to EIB. They are investigating.

@RussTreadon-NOAA
Copy link
Contributor

Install DavidNew-NOAA:feature/ice_wat_inc on Orion in a working copy of asdfsd. Also update g-w ush/calcanl_gfs.py as shown in 26fd24d. Rewind and reboot 2024022400 gfsanalcalc.

  • Good news: interp_inc.x successfully ran to completion
  • Bad news: calc_analysis.x aborted with the message
 Adding Increment to icmr   icmr
[Orion-12-39:214519:0:214519] Caught signal 11 (Segmentation fault: Sent by the kernel at address (nil))
 rank of data array != variable ndims (or ndims-1)
[Orion-12-40:53668:0:53668] Caught signal 11 (Segmentation fault: Sent by the kernel at address (nil))
99
 rank of data array != variable ndims (or ndims-1)
==== backtrace (tid:  53671) ====
 0 0x000000000049b1d4 module_ncio_mp_read_vardata_3d_r4_()  /work/noaa/epic/role-epic/spack-stack/orion/spack-stack-1.6.0/cache/build_stage/spack-stage-ncio-1.1.2-ocwpickzbfg6lozblsd2q7auqk2smpa3/spack-src/src/read_vardata_code_3d.f90:50
 1 0x0000000000411484 inc2anl_mp_add_increment_()  /work2/noaa/da/rtreadon/git/global-workflow/submodules/sorc/gsi_utils.fd/src/netcdf_io/calc_analysis.fd/inc2anl.f90:202
 2 0x000000000040f109 inc2anl_mp_gen_anl_()  /work2/noaa/da/rtreadon/git/global-workflow/submodules/sorc/gsi_utils.fd/src/netcdf_io/calc_analysis.fd/inc2anl.f90:67
 3 0x000000000041837d MAIN__()  /work2/noaa/da/rtreadon/git/global-workflow/submodules/sorc/gsi_utils.fd/src/netcdf_io/calc_analysis.fd/main.f90:29
 4 0x000000000040ead2 main()  ???:0
 5 0x0000000000022495 __libc_start_main()  ???:0
 6 0x000000000040e9e9 _start()  ???:0

A check of gsi_utils.fd/src/netcdf_io/calc_analysis.fd/inc2anl.f90 finds icmr being used. I checked today's operational gdas.t00z.atmanl.nc and gdas.t00z.atmanl.ensres.nc. Both use icmr. Changing this to ice_wat could have downstream impacts.

It looks like inc2anl.f90 assumes the increment is simply the field name with the suffix _inc added. This is no longer valid.

Copy link
Contributor

@CoryMartin-NOAA CoryMartin-NOAA left a comment

Choose a reason for hiding this comment

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

Revoking my approval given Russ's findings

@DavidNew-NOAA
Copy link
Contributor Author

Ok, the fun continues. Will take a look

@RussTreadon-NOAA
Copy link
Contributor

Naive question: What's the reason for replacing icmr with ice_wat? When we run g-w CI for C96C48_hybatmDA (GSI-based DA), we find the following in gfs atmfXXX.nc files

        float clwmr(time, pfull, grid_yt, grid_xt) ;
        float delz(time, pfull, grid_yt, grid_xt) ;
        float dpres(time, pfull, grid_yt, grid_xt) ;
        float dzdt(time, pfull, grid_yt, grid_xt) ;
        float grle(time, pfull, grid_yt, grid_xt) ;
        float hgtsfc(time, grid_yt, grid_xt) ;
        float icmr(time, pfull, grid_yt, grid_xt) ;
        float nccice(time, pfull, grid_yt, grid_xt) ;
        float nconrd(time, pfull, grid_yt, grid_xt) ;
        float o3mr(time, pfull, grid_yt, grid_xt) ;
        float pressfc(time, grid_yt, grid_xt) ;
        float rwmr(time, pfull, grid_yt, grid_xt) ;
        float snmr(time, pfull, grid_yt, grid_xt) ;
        float spfh(time, pfull, grid_yt, grid_xt) ;
        float tmp(time, pfull, grid_yt, grid_xt) ;
        float ugrd(time, pfull, grid_yt, grid_xt) ;
        float vgrd(time, pfull, grid_yt, grid_xt) ;

icmr is in the atmfXXX.nc file.

@DavidNew-NOAA
Copy link
Contributor Author

DavidNew-NOAA commented May 22, 2024

@RussTreadon-NOAA No, it's a good question. @CoryMartin-NOAA were just sorting this out right now. I think the naivety was actually on my part, and I will revert GDASApp #1112 for now. Here's the explanation:

With IAU, UFS is reading the increment names as trim(tracer_names(l))//'_inc'. See here https://github.com/DavidNew-NOAA/GFDL_atmos_cubed_sphere/blob/cc8afe64b9420dd7379e47cee5e3f30f4ac44b58/tools/fv_iau_mod.F90#L485

The internal name for ice water mixing ratio is "ice_wat", but it's written in atmfXXX.nc as "icmr". See here https://github.com/NOAA-EMC/global-workflow/blob/7d2c539f45194cd4e5b21bfd4b83a9480189cd0f/parm/ufs/fv3/diag_table_da#L20

Without IAU, the tracer increment names and reads are hardcoded. In fact, the ice water increment isn't even read, and liquid water increment name is hardcoded as "liq_wat_inc" even though liquid water mixing ratio is written in atmfXXX.nc as "clwmr" in atmfXXX.nc. See here: https://github.com/DavidNew-NOAA/GFDL_atmos_cubed_sphere/blob/dev/emc/tools/fv_treat_da_inc.F90#L288

When I was implementing cubed sphere increments in GFDL_atmos_cubed_sphere #342, I was trying to eliminate some of this hardcoding, but it was not reading the ice increment, because it was looking for "ice_wat_inc" rather than "icmr_inc". I assumed this was a mistake in the DA system.

Long term this is an issue when we start using IAU, but for now, all hydrometeor increments are set to zero anyway in operations and development. Thus I'll revert for now, and then make an issue. But better to be deliberate and break as few things as possible.

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.

3 participants