-
Notifications
You must be signed in to change notification settings - Fork 258
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
Fix wave init #1547
Fix wave init #1547
Conversation
@pjpegion I'm not sure I understand by "not wave in the coupler" Do you mean that the wave fields are not in the coupler restart file? |
yes, it should have said waves are not in the mediator/coupler restart files. |
Thanks for the clarification. You said you tested w/ cpld_control and cpld_restart. But if the issue is that you don't have the wave ICs from the replay, wouldn't the test have been to run w/o waves for the control case and then try to restart with that but with waves included? |
@DeniseWorthen I will do that test to confirm it works in that situation |
@DeniseWorthen I did the following tests which all worked
|
I've brought in the new PR template and copied over the appropriate text. Please fix what I may have missed. This is still waiting on WW3 review of that code, and then you will need to run the full RT suite with an up-to-date (as possible) branch before this makes it into the commit queue. |
Also I wasn't sure if this needed new baselines, but assumed it did. Please remove that label if I am incorrect. |
@pjpegion I believe the fix I had proposed for adding a time-offset is cleaner than your proposed change. It leaves intact the logic of initial/continue and adds a time-offset for fhrot in the same way the driver does. |
@DeniseWorthen I remember you suggesting the time offset. Did you code that up? |
@pjpegion yes, I did code it up. I added a few log messages regarding the clock, but those aren't really required. You can see the change in wav_comp_nuopc in this branch. For some reason I can't find your fork to compare to. Note this branch also fixed CI for the dev/ufs-weather-model branch and updated to the latest develop so additional changes are present. |
Still waiting on Intel/GNU logs from Hera or Cheyenne showing the anticipated RT's are the only ones failing. |
Hi, @pjpegion. In anticipation of working to test this PR next, would you be able to resolve the conflicts and provide Intel/GNU logs for Hera or Cheyenne? |
@pjpegion is on leave this week so I'm helping him w/resolving the conflicts and can provide the intel/gnu for hera. |
@JessicaMeixner-NOAA Ok, understood. Thank you! |
Conflicts: WW3 tests/RegressionTests_hera.intel.log
Okay the WW3 branch for this PR has been updated. And I have a PR with the updated and merge conflicts resolved and is here: pjpegion#14 I'll post gnu/intel hera logs as soon as i have them. |
@JessicaMeixner-NOAA @pjpegion the pr still needs work. Meanwhile, we like to check next pr #1642 as well. FYI @zach1221 @BrianCurtis-NOAA |
Merge in develop of UFS and point to updated WW3
@JessicaMeixner-NOAA cleaned up my mess. |
on-behalf-of @ufs-community <[email protected]>
on-behalf-of @ufs-community <[email protected]>
Automated RT Failure Notification |
on-behalf-of @ufs-community <[email protected]>
I re-ran the 2 failed jobs on gaea this morning and both passed: /lustre/f2/scratch/ncep/Jessica.Meixner/ufs-weather-model/tests/RegressionTests_gaea.intel.log |
@JessicaMeixner-NOAA thank you. I'll kill my tests running and use your logs. |
@jkbk2004 let me know when you're ready for me to merge in the WW3 update. |
@JessicaMeixner-NOAA sure! go ahead to merge ww3 pr. |
The WW3 PR has been merged. The hash from the sqush commit for WW3 is: af544dca5bf49bf3c3ad8d47b786e8211dcf4bba I dont know if @pjpegion is out for the day yet or not, but if he is maybe someone w/maintainer privelages can push the revert to his branch? |
I'm here, and will do the update to my branch. |
revert ww3 submodule
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.
Hash looks good.
Description
Fixes the logic for a restart when there are not wave in the coupler and start_type needs to be set to initial. This is needed for EP4 which will use warm starts from the replay, but the wave initial conditions are not part of the replay, so they are not in the mediator.
Top of commit queue on: TBD
Input data additions/changes
Anticipated changes to regression tests:
cpld_control_p8 and cpld_restart_p8
Subcomponents involved:
Combined with PR's (If Applicable):
Commit Queue Checklist:
Linked PR's and Issues:
Testing Day Checklist:
Testing Log (for CM's):