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

[develop] Ensure arbitrary restart_interval numbers in model_configure #503

Merged
merged 2 commits into from
Dec 5, 2022

Conversation

chan-hoo
Copy link
Collaborator

@chan-hoo chan-hoo commented Dec 1, 2022

DESCRIPTION OF CHANGES:

  • To ensure arbitrary numbers for restart_interval, the unnecessary parameter value -1 is removed from the template parm/model_configure.
  • The WE2E test is updated with restart_interval: 1 2 5, which creates the restart files at fhr=1, 2, and 5.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

TESTS CONDUCTED:

  • WE2E test: specify_RESTART_INTERVAL with the following three cases:
  1. RESTART_INTERVAL: 1 2 5
  2. RESTART_INTERVAL: 1 -1
  3. RESTART_INTERVAL: 0
  • hera.intel
  • orion.intel
  • cheyenne.intel
  • cheyenne.gnu
  • gaea.intel
  • jet.intel
  • wcoss2.intel
  • NOAA Cloud (indicate which platform)
  • Jenkins
  • fundamental test suite
  • comprehensive tests (specify which if a subset was used)

ISSUE:

Fixes issue mentioned in #502

CHECKLIST

  • My code follows the style guidelines in the Contributor's Guide
  • I have performed a self-review of my own code using the Code Reviewer's Guide
  • I have commented my code, particularly in hard-to-understand areas
  • My changes need updates to the documentation. I have made corresponding changes to the documentation
  • My changes do not require updates to the documentation (explain).
  • My changes generate no new warnings
  • New and existing tests pass with my changes
  • Any dependent changes have been merged and published

@MichaelLueken MichaelLueken linked an issue Dec 1, 2022 that may be closed by this pull request
@MichaelLueken MichaelLueken changed the title Ensure arbitrary restart_interval numbers in model_configure [develop] Ensure arbitrary restart_interval numbers in model_configure Dec 1, 2022
@chan-hoo
Copy link
Collaborator Author

chan-hoo commented Dec 1, 2022

@MichaelLueken, Thank you! I forgot to add [develop] to the title.

Copy link
Collaborator

@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.

@chan-hoo These changes look good to me. However, the specify_RESTART_INTERVAL WE2E test was ran on Cheyenne. This test failed due to running over the walltime limit. Please increase WTIME_RUN_FCST: to allow the test to run on this machine (01:30:00 or 02:00:00 should allow this test to run to completion).

Copy link
Collaborator

@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.

Thank you very much, @chan-hoo, for addressing my concern! A rerun of the specify_RESTART_INTERVAL test on Cheyenne is now successfully passing. Approving this work and launching the Jenkins tests.

@MichaelLueken MichaelLueken added the run_we2e_coverage_tests Run the coverage set of SRW end-to-end tests label Dec 2, 2022
Copy link
Collaborator

@danielabdi-noaa danielabdi-noaa left a comment

Choose a reason for hiding this comment

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

I have run the test case modified in this PR on Hera, and it does produce restart files for forecast hours 1, 2 and 5 so approving.

@MichaelLueken
Copy link
Collaborator

The Jenkins tests have passed, two approvals have been given, and there are no outstanding comments, so I will now move forward with merging this work.

@MichaelLueken MichaelLueken merged commit b414442 into ufs-community:develop Dec 5, 2022
@chan-hoo chan-hoo deleted the feature/arb_restart branch January 12, 2023 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run_we2e_coverage_tests Run the coverage set of SRW end-to-end tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Arbitrary restart interval numbers are not allowed
3 participants