Skip to content

Add data model support to HAFS workflow#91

Merged
evankalina merged 86 commits into
developfrom
feature/hafs_couplehycom_cdeps
Sep 9, 2021
Merged

Add data model support to HAFS workflow#91
evankalina merged 86 commits into
developfrom
feature/hafs_couplehycom_cdeps

Conversation

@evankalina
Copy link
Copy Markdown
Contributor

@evankalina evankalina commented Aug 16, 2021

Description of changes

This PR allows developers to perform HAFS runs with the Community Data models for Earth Prediction Systems (CDEPS). Developers can use the new workflow capabilities to one-way couple the HYCOM ocean model to a data atmosphere created from the ERA5 reanalysis. Or they can one-way couple the UFS weather model to a data ocean created from the OISST or GHRSST analyses. The workflow is capable of running all of the necessary steps in an automated fashion, including pre-processing the input data, generating the data model grids, and running the forecast. Scripts are provided to download ERA5, OISST, and GHRSST data in an offline fashion. This PR is not expected to have any impact on HAFS runs that do not use data models.

Dependencies

There are no submodule dependencies, as the necessary updates have already been made to the support/HAFS branch of CDEPS in a previous commit (PR # 4).

A user guide that describes how to run CDEPS in the workflow, along with suggestions for how to use custom data sources to generate the data models, is available here. It would be good to include this information in a new chapter of the HAFS Developers guide.

Contributors

Along with the author, @SamuelTrahanNOAA, @uturuncoglu, and @danrosen25 worked to add CDEPS support to the HAFS workflow. We would also like to thank @rsdunlapiv, @mvertens, and @ligiabernardet for helping guide the design decisions.

Tests conducted

On Orion, the full workflow was tested, with 126-h forecasts of the ERA5 data atmosphere and both the OISST and GHRSST data oceans for several cycles of Hurricanes Dorian and Zeta. On Jet and Hera, the entire workflow was tested except for the forecast and post-processing steps. The forecast jobs have been sitting in the Jet/Hera queues for weeks without running. We anticipate that Orion will be the primary machine used for data model runs in HAFS, and we can always submit a new PR if any changes are needed to fix bugs on Jet and Hera. We anticipate only needing to make small changes, if any, on those platforms.

Application-level regression test status

Running the HAFS application-level regression tests is currently performed by code reviewers after the developer creates the initial PR. As regression tests are conducted, the testers should use the checklist below to indicate successful regression tests. You may add other tests as needed. If a test fails, do not check the box. Instead, describe the failure in the PR comments, noting the platform where the test failed.

  • Jet
  • Hera
  • Orion
  • WCOSS Dell P3
  • WCOSS Cray

Let's start the HAFS application/workflow level regression tests as you helped previously:
@evankalina, @panll, @mrinalbiswas (Hera and Orion)
@JunghoonShin-NOAA (Jet, we probably will skip Jet since it is occupied by the HFIP real-time parallel experiments under reservation)
@LinZhu-NOAA (wcoss_dell_p3)
@BijuThomas-NOAA (wcoss_cray)
Also, @JohnSteffen-NOAA please test the three HAFS CDEPS tests on wcoss_dell_p3.

danrosen25 and others added 30 commits October 27, 2020 16:11
…oupling,

even when CMEPS is disabled, according to the instructions from Dan R.
variable is already used locally by multiple scripts in the workflow.
The new CDEPS input data directory is called DATMdir and is specific
to the CDEPS DATM.
grid corners, HYCOM_merged branch, ability to set NEMS connector
attributes through config, and CMEPS datm mask value fix.
…m_cmeps'. Update submodule for hafs_forecast.fd. CMEPS has been synchronized with ESCOMP/CMEPS and NEMS updated to include PIO initialization.
@LinZhu-NOAA
Copy link
Copy Markdown
Contributor

@BinLiu-NOAA In rocoto/hafs_multistorm_workflow.xml.in, the "@** endif" after the if condiction for RUN_OCEAN==YES at line 578 needs to move up to line 573 after if condition for RUN_GSI_VR==YES. Otherwise, it could not generate correct xml for globalnest tests.

@BinLiu-NOAA
Copy link
Copy Markdown
Collaborator

@BinLiu-NOAA In rocoto/hafs_multistorm_workflow.xml.in, the "@** endif" after the if condiction for RUN_OCEAN==YES at line 578 needs to move up to line 573 after if condition for RUN_GSI_VR==YES. Otherwise, it could not generate correct xml for globalnest tests.

Thanks, @LinZhu-NOAA! @evankalina or @SamuelTrahanNOAA, Could you please check and commit this change back into the rocoto/hafs_workflow.xml.in file of the feature/hafs_couplehycom_cdeps branch? Thanks!

* ocn_prep should be a dependency for the forecast job
  regardless of whether we are running the global nesting
  or regional HAFS. From Lin Zhu (IMSG at NOAA/EMC).
@evankalina
Copy link
Copy Markdown
Contributor Author

@BinLiu-NOAA In rocoto/hafs_multistorm_workflow.xml.in, the "@** endif" after the if condiction for RUN_OCEAN==YES at line 578 needs to move up to line 573 after if condition for RUN_GSI_VR==YES. Otherwise, it could not generate correct xml for globalnest tests.

Thanks, @LinZhu-NOAA! @evankalina or @SamuelTrahanNOAA, Could you please check and commit this change back into the rocoto/hafs_workflow.xml.in file of the feature/hafs_couplehycom_cdeps branch? Thanks!

Thanks @LinZhu-NOAA and @BinLiu-NOAA for noting this error! It has been fixed.

@panll
Copy link
Copy Markdown
Contributor

panll commented Sep 2, 2021

The regression tests finished smoothly with the latest commit on Hera. @evankalina @BinLiu-NOAA

panll
panll previously approved these changes Sep 2, 2021
@LinZhu-NOAA
Copy link
Copy Markdown
Contributor

The regression tests completed successfully on WCOSS-DELL.

LinZhu-NOAA
LinZhu-NOAA previously approved these changes Sep 2, 2021
@mrinalbiswas
Copy link
Copy Markdown
Contributor

@BinLiu-NOAA @evankalina RTs passed on Orion

@BijuThomas-NOAA
Copy link
Copy Markdown
Contributor

@BinLiu-NOAA @evankalina,
RTs were successful on WCOSS-Cray

BijuThomas-NOAA
BijuThomas-NOAA previously approved these changes Sep 3, 2021
@BinLiu-NOAA
Copy link
Copy Markdown
Collaborator

Thanks for conducting the HAFS application level regression tests on hera, orion, wcoss_cray and wcoss_dell_p3!

@JohnSteffen-NOAA, have you get a chance to test the three HAFS CDEPS coupling test cases on wcoss_dell_p3 yet?

@evankalina, depending upon the status of @JohnSteffen-NOAA's HAFS CDEPS coupling tests on wcoss_dell_p3, we may decide to wait for his test results or to just go ahead merging this PR. In the meantime, in terms of how to merge this PR, not sure if you want choose the "squash and merge" approach or the regular merge approach. But in any case, it would be nice to include a comprehensive/detailed commit log message. Thanks!

@JohnSteffen-NOAA
Copy link
Copy Markdown
Contributor

@BinLiu-NOAA I have been working on the HAFS CDEPS tests for wcoss_dell_p3 as of yesterday and will continue today. I will keep you updated.

@BinLiu-NOAA
Copy link
Copy Markdown
Collaborator

@BinLiu-NOAA I have been working on the HAFS CDEPS tests for wcoss_dell_p3 as of yesterday and will continue today. I will keep you updated.

@JohnSteffen-NOAA, Thanks for keeping us updated with your HAFS CDEPS coupling tests on wcoss_dell_p3!

@evankalina
Copy link
Copy Markdown
Contributor Author

Thanks @BijuThomas-NOAA, @LinZhu-NOAA, @panll, and @mrinalbiswas for taking the time to conduct these regression tests, and to @JohnSteffen-NOAA for testing HAFS CDEPS on wcoss_dell_p3!

@BinLiu-NOAA I think this PR is a good candidate for the "squash and merge" approach. We made so many individual commits (85!), so a regular merge will make the commit history look noisy. I can write a clean commit message summarizing all of the changes instead when I squash merge it. I will wait a bit to hear from @JohnSteffen-NOAA first.

@BinLiu-NOAA
Copy link
Copy Markdown
Collaborator

Thanks @BijuThomas-NOAA, @LinZhu-NOAA, @panll, and @mrinalbiswas for taking the time to conduct these regression tests, and to @JohnSteffen-NOAA for testing HAFS CDEPS on wcoss_dell_p3!

@BinLiu-NOAA I think this PR is a good candidate for the "squash and merge" approach. We made so many individual commits (85!), so a regular merge will make the commit history look noisy. I can write a clean commit message summarizing all of the changes instead when I squash merge it. I will wait a bit to hear from @JohnSteffen-NOAA first.

Agreed. The squash and merge method sounds perfectly good to me as well. Thanks @evankalina!

…s_dell_p3 and adds CDO module load to modulefile
Copy link
Copy Markdown
Collaborator

@BinLiu-NOAA BinLiu-NOAA left a comment

Choose a reason for hiding this comment

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

HAFS application regression tests passed. And HAFS CDEPS coupling tests also ran through on Hera/Orion/Wcoss_dell_p3.

@evankalina evankalina merged commit ff56ac2 into develop Sep 9, 2021
@BinLiu-NOAA BinLiu-NOAA deleted the feature/hafs_couplehycom_cdeps branch September 14, 2021 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request Ready for Review The PR is considered complete by the author(s) Ready for RTs The PR is ready to conduct Regression Tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.