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

Unify gfs, cfsr, and gefs datm datamodes #63

Merged
merged 4 commits into from
Aug 27, 2024

Conversation

NickSzapiro-NOAA
Copy link
Collaborator

@NickSzapiro-NOAA NickSzapiro-NOAA commented Aug 7, 2024

Description of changes

Code for gfs, cfsr, and gefs datm datamodes is very similar (just with different function names). This PR removes the duplicates. As gefs datm datamode is already in ESCOMP/CDEPS, gefs is retained here. Note that GFS, GEFS, CFSR, etc. input datasets are still handled via streams in the same way, just with datamode == 'GEFS'. Sunset gfs_hafs datamode no longer used.

Specific notes

Contributors other than yourself, if any:

CDEPS Issues Fixed (include github issue #): #64

Are there dependencies on other component PRs (if so list):

Are changes expected to change answers (bfb, different to roundoff, more substantial): bfb

Any User Interface Changes (namelist or namelist defaults changes): Remove gfs and cfsr datm datamodes. Use gefs instead

Testing performed (e.g. aux_cdeps, CESM prealpha, etc): UFS regression test suite

Hashes used for testing: See ufs-community/ufs-weather-model#2389

@NickSzapiro-NOAA
Copy link
Collaborator Author

If datm_datamode_gfs_hafs_mod has been replaced with CDEPS inline functionality and not to be pushed to ESCOMP/CDEPS, maybe this would be a good place to sunset it @binli2337

@binli2337
Copy link
Collaborator

@NickSzapiro-NOAA Yes, it is time to remove the gfs_hafs data mode. Thanks!

Copy link
Collaborator

@binli2337 binli2337 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. I will approve this PR after the related ufs-weather-model PR is approved. Thanks!

@NickSzapiro-NOAA
Copy link
Collaborator Author

Hi @binli2337 , Jun reviewed and approved the ufs-weather-model PR

@zach1221
Copy link

@binli2337 following up here. Are you able to review this PR?

@zach1221
Copy link

@binli2337 this PR is ready for final review.

@jkbk2004
Copy link
Collaborator

looks like I don't have permission to merge. @DeniseWorthen @binli2337 can you merge this pr?

@NickSzapiro-NOAA NickSzapiro-NOAA merged commit 1f9eaaa into NOAA-EMC:develop Aug 27, 2024
@NickSzapiro-NOAA
Copy link
Collaborator Author

@jkbk2004 merged into cdeps

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.

4 participants