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

Use nceplibs-ncio for enkf_chgres_recenter_nc utility #839

Merged
merged 1 commit into from
Jun 3, 2022

Conversation

aerorahul
Copy link
Contributor

Description

This PR:

  • removes dependency on GSI built fv3gfs_ncio and uses nceplibs-ncio.
  • Only updated for WCOSS2

This fix is needed for gfsv16.3.0

Type of change

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

How Has This Been Tested?

  • Clone and Build tested the utility enkf_chgres_recenter_nc on WCOSS2

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • My changes generate no new warnings

Copy link

@MichaelLueken MichaelLueken left a comment

Choose a reason for hiding this comment

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

These changes look good. They will allow the enkf_chgres_recenter_nc utility to build on WCOSS2 following the removal of fv3gfs_ncio from the GSI repository.

@aerorahul aerorahul requested a review from lgannoaa June 3, 2022 12:04
Copy link
Member

@KateFriedman-NOAA KateFriedman-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, thanks @aerorahul !

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.

Changes look good. Has anyone built and run enkf_chgres_recenter_nc.x from feature/update_ncio to ensure that it generates b4b identical results with the control (WCOSS2 operations)? It should, but have we confirmed this?

@aerorahul
Copy link
Contributor Author

Changes look good. Has anyone built and run enkf_chgres_recenter_nc.x from feature/update_ncio to ensure that it generates b4b identical results with the control (WCOSS2 operations)? It should, but have we confirmed this?

No @RussTreadon-NOAA.
My only test has been to build the executable with the library that is in nceplibs and not in gsi.
As there are no standalone tests for this utility, a cycled experiment will need to be set up and executed, or a standalone test will have to be propped up.
I am not sure which is faster.

@RussTreadon-NOAA
Copy link
Contributor

@aerorahul , valid point. "Run a test to confirm b4b identical results" is easy to say, not so easy to do. One could stage data in a production look-alike directory structure to run enkfgdas_ecen for a 3, 6, or 9 hour case via ecflow, rocoto, or manual submission of a j-job driver. Not hard, but certainly tedious.

@aerorahul aerorahul mentioned this pull request Jun 3, 2022
7 tasks
@aerorahul
Copy link
Contributor Author

Is it ok to merge this or are we waiting for a test confirmation?

@RussTreadon-NOAA
Copy link
Contributor

Testing is tedious. Perhaps an after-the-fact test may be run after this PR is closed. When the GFS v16.3.0 parallel begins take a cycle and run echgres (or gdas_atmos_enkf_chgres) twice. The first run uses enkf_chgres_recenter_nc.x built from this PR. The second uses NCO's executable. Switching executables can be done by setting CHGRESNCEXEC in config.echgres.

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.

enkf_chgres_recenter_nc.x is built with the nceplibs-ncio.

@aerorahul aerorahul merged commit 169766f into release/gfs.v16.3.0 Jun 3, 2022
@aerorahul aerorahul deleted the feature/update_ncio branch June 3, 2022 18:59
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.

5 participants