Skip to content

[develop] Ensure arbitrary restart_interval numbers in model_configure#503

Merged
MichaelLueken merged 2 commits into
ufs-community:developfrom
chan-hoo:feature/arb_restart
Dec 5, 2022
Merged

[develop] Ensure arbitrary restart_interval numbers in model_configure#503
MichaelLueken merged 2 commits into
ufs-community:developfrom
chan-hoo:feature/arb_restart

Conversation

@chan-hoo
Copy link
Copy Markdown
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
Copy Markdown
Collaborator Author

chan-hoo commented Dec 1, 2022

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

Copy link
Copy Markdown
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).

Comment thread tests/WE2E/test_configs/wflow_features/config.specify_RESTART_INTERVAL.yaml Outdated
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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