-
Notifications
You must be signed in to change notification settings - Fork 251
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
Update WW3 with fb_coupling_fields; add cice fix for zlvs #573
Update WW3 with fb_coupling_fields; add cice fix for zlvs #573
Conversation
coupling fields and requires the ww3_multi.inp files to change
wave tests now pass on hera.intel
as PR was committed
which means we can remove the if statement in the CMakeLists.txt logic for wcoss2
@JessicaMeixner-NOAA Is this PR ready for commit? Have you run test with with PR? |
@junwang-noaa yes, it should be ready to be committed. When the PR was created, I created a new baseline and tested against it, and all wave tests passed (only wave tests were run). I have not recreated those baselines for the subsequent merges with develop branch. |
@JessicaMeixner-NOAA we made two big PRs recently, it would be good you can merge with your branch, and then at least test on one platform, your PR will be the next in the commit queue, adequate testing can help to have a smooth code commit. Thanks |
Conflicts: tests/tests/fv3_wrtGauss_nemsio_c768
develop has been merged in and a few extra files have been changed based on the recent updates. In addition, the 35 day tests for the coupled model will need new ICs. They should all be ready on hera (just waiting for hera to come back up). I'm in the process of creating a baseline to then check against for the wave tests. |
The ATMW test passed after creating a new baseline. I'm creating the S2SW baseline to run against now. In addition, I'm testing the 35 day tests with the new ICs I made (not the full 35 day runs, but just making sure they all start okay). @junwang-noaa @DeniseWorthen or others, please let me know if there is any other additional pre-testing that would be helpful to ensure the commit process can go as smoothly as possible. |
Thanks for doing the testing, Jessica. Since CICE6 update PR610 is ready, we'd like to commit that so that we can create P7b branch. Your PR will be combined with PR#584 and is planned to be committed on Monday 6/7. |
@JessicaMeixner-NOAA I removed the comparison of the W3 restarts from the comparison list for the v16_wave test when I needed to adjust the number of forecast hours. Will the changes you made #544 with the template automatically create restarts at the right forecast hour? |
@DeniseWorthen it will not automatically do that but I can check for that and re-add it back if that's the goal. |
@DeniseWorthen if we change WW3RSTDTHR = FHMAX that would write the restart at the end of the run, is that what you want? Currently it defaults to every 12 hours. Alternatively we could change WW3RSTDTHR to the dt of restarts needed in hours. (It gets converted to seconds in the edit_inputs.sh script. Let me know which test and what the value is and I can add it to this PR. |
@JessicaMeixner-NOAA I think we do want that it writes at the end of the run, whatever FHMAX is. |
@junwang-noaa @DeniseWorthen @DusanJovic-NOAA These will vary more slowly than the MOD_DEFs so I lean towards: but am open to whatever makes the most sense to others. |
Great, thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code changes look good to me, needs to confirm all RTs are done.
All RTs completed and CI has passed. CICE has been merged so once the submodule has been reverted we're ready for commit. |
PR Checklist
Ths PR is up-to-date with the top of all sub-component repositories except for those sub-components which are the subject of this PR. Please consult the ufs-weather-model wiki if you are unsure how to do this.
This PR has been tested using a branch which is up-to-date with the top of all sub-component repositories except for those sub-components which are the subject of this PR
An Issue describing the work contained in this PR has been created either in the subcomponent(s) or in the ufs-weather-model. The Issue should be created in the repository that is most relevant to the changes in contained in the PR. The Issue and the dependent sub-component PR
are specified below.
If new or updated input data is required by this PR, it is clearly stated in the text of the PR.
Instructions: All subsequent sections of text should be filled in as appropriate.
Description
This PR updates WW3 to a version that adds the option of 2 more input fields. Therefore this requires modifications to the input file ww3_multi.inp which is updated here.
Baselines will change because an extra field is added into the WW3 restart file and therefore the WW3 binary restart file is changed. New mod_def files are not needed.
Issue(s) addressed
Testing
New baselines for the wave tests were created on hera.intel and the wave related tests were then run and passed that new baseline.
Dependencies
WW3 branch is here: https://github.com/ukmo-waves/WW3/tree/fb_coupling_fields