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

Restart overwrite protection added #1306

Open
wants to merge 2 commits into
base: dev/ufs-weather-model
Choose a base branch
from

Conversation

sbanihash
Copy link
Collaborator

@sbanihash sbanihash commented Oct 2, 2024

Pull Request Summary

restart overwrite protection, regtest module update

Description

This PR intends to make configurable to not have time0 and timeN be over-written by forecast hour 0/N
similar to what has been done manually here

Commit Message

Configurable restart overwrite

Check list

Testing

Runs were completed in both Hera and Orion. This change is not expected to change any results in the regtests, the module update will add the capability to run regression tests for this branch on the RDHPCs and the configurable restart overwrite needs an attribute that has been added to a companion change in the ufs-weather-model and can be tested in the ufs-wm rt's.

I have attached a sample of running matrix.comp for running matrix12 (0 diffs). Note that you can not simply run this matrix with the PR branch and the compare with the current dev/ufs-weather-model branch run since the modules needed to be updated. So some changes were also needed in the matrix_cmake scripts in the NOAA-EMC:dev/ufs-weather-model as well to be able to run it.

matrixCompSummary.txt
matrixCompFull.txt

sbanihash and others added 2 commits September 13, 2024 11:12
* adding attribute for time0/timeN oevrwrite protection

* adding time0/timeN overwrite protection

* initialize cpl_scalars when created (NOAA-EMC#1274)

---------

Co-authored-by: Denise Worthen <[email protected]>
@MatthewMasarik-NOAA
Copy link
Collaborator

This change is not expected to change any results in the regtests, the module update will add the capability to run regression tests for this branch on the RDHPCs

Hi @sbanihash, we don't require running the regtests for the dev/ufs-weather-model branch, though sometimes this is done on our end for a sanity check. If this add the capability for running the regtests on RDHPCS and the results are not expected to change, can you attach the matrix output?

@sbanihash
Copy link
Collaborator Author

sbanihash commented Oct 2, 2024

@MatthewMasarik-NOAA here is a path on Orion:
/work/noaa/marine/Saeideh.Banihashemi/GlobalDev/GEFS/WAVE_INFRST/WW3
Since the comps in dev/ufs-weather-model were not described here this list was not helpful in identifying if the diff's were expected. But I manually did a diff between the binary outputs and also checked that all runs were successful with the module updates for all the 14 matrix runs.

@MatthewMasarik-NOAA
Copy link
Collaborator

Since the comps in dev/ufs-weather-model were not described here this list was not helpful in identifying if the diff's were expected.

@sbanihash this means there were unexpected differences, if I understand correctly. At any rate, attaching the 3 matrix comparison files here will give all that detail, and is useful for posterity.

@sbanihash
Copy link
Collaborator Author

sbanihash commented Oct 2, 2024

@MatthewMasarik-NOAA sure, here it is on Hera:
/scratch1/NCEPDEV/stmp2/Saeideh.Banihashemi/GlobalDev/WW3/myWW3/regtests
Let me know if you have access issues.

@MatthewMasarik-NOAA
Copy link
Collaborator

@sbanihash, not everyone has access to hera. The PR header should have given instruction to posting the non-b4b's from the matrixCompSummary.txt, as well as attaching the 3 full files. You can click on the bar beneath edit box, or drag and drop to attach the text files.

@sbanihash sbanihash changed the title Restart overwrite protection added + Updates to modules and cmake for regtests Restart overwrite protection added Oct 10, 2024
@DeniseWorthen
Copy link
Contributor

This change is not expected to change any results in the regtests, the module update will add the capability to run regression tests for this branch on the RDHPCs

Hi @sbanihash, we don't require running the regtests for the dev/ufs-weather-model branch, though sometimes this is done on our end for a sanity check. If this add the capability for running the regtests on RDHPCS and the results are not expected to change, can you attach the matrix output?

The code changes in this PR are in a module (wav_comp_nuopc) which isn't even present in the develop branch, so in this case I don't understand why anything other than the UWM RTs are being run.

Also, I believe the initial reason this change was required in the HAFS production branch was to control restart frequency. Unless HAFS intends not to make use of the new restartnc capability, the flexibility I believe they need will be provided by the update to allow WW3 (as well as the other components) to have "flexible restart times". See related PR ufs-community/ufs-weather-model#2419.

Once the same change is made in WW3 that is being made in the other components, it will enable WW3 to read the same configuration variable in model_configure that is used for the ATM and produce netcdf restarts at any output frequency. Note this will only be possible with the restartnc capability, because it requires use of the rstwr logical. @BinLiu-NOAA

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.

3 participants