upcoming conflict with dev/ufs-weather-model branch, IC4 method=8 #1187
Comments
|
@DeniseWorthen we've spot checked a few PRs from the develop branch with the dev/ufs-waether-model branch, but we do not at this time consider it a requirement to run the ufs-weather-model PRs for the develop branch, just the standalone WW3 regression tests. There is certainly an added complexity with not all updates form dev/ufs-weather-model being included in develop, which will unfortunately persist for a while moving forward. It is planned to merge develop to dev/ufs-weather-model soon, although I don't have an exact timing at this moment. I'll look at the conflicts with IC4 M8 and see how we can resolve those conflicts. |
|
The IC4 method 8 in dev/ufs-weather-model will be moved to method 10 when develop is merged into dev/ufs-weather-model. I am resolving an issue I get when running some tests in the new dev/ufs-weather-model branch so likely have a merge issue to investigate. Hopeful timeline is mid-next week to open a PR. |
|
Just as a heads up to everyone. I've isolated a commit where we change answers in the dev/ufs-weather-model b/c of the wave model changes. It's unexpected and not something we saw in standalone tests so I'm still tracking this down. I will not be able to work on this next week, but will hope to have a PR as soon as possible after that. |
|
@JessicaMeixner-NOAA I've been able to do some testing on this issue. I believe the B4B changes occur when merging in develop hash 3952826 (#1124). In the ATM-WAV test, the field which changes is the export of z0 to the atmosphere. The first change is at a single point and it is round-off level (O~10-12). The answers then diverge. For the coupled model, the first difference is in the export of one of the stokes drift components. Given then code changes that were made in src4 at 3952826, is it a reasonable result that values of A used in the calculation of stokes drift and Z0 would change? In my tests, a baseline created using the merged #1124 hash is B4B w/ updating to the top of develop in dev/ufs-weather-model. |
|
@DeniseWorthen I had also found that. My standalone tests that should mimic what is going on the in the coupled model as best we can with the standalone do not change with that commit - including if we have the same output to the ocean. I'm not really comfortable going forward when I don't know why the z0 or Stokes parameters are changing as from what I can tell I don't think they should for the way that we're using the code. I have a few things I'm planning to try out to see if I can narrow down where the code changes are. We can as a back-up accept the changes to move forward, but honestly I'm not quite ready to do that. |
|
@DeniseWorthen I've confirmed with the developer of that PR that these answer changes are indeed unexpected so I'll continue to investigate and hopefully determine why. |
|
@JessicaMeixner-NOAA Thanks. I'm a bit confused though, since in the original PR I see Matt makes the comment about expected differences etc. It seems this #1124 didn't go into the develop branch w/o B4B changes. |
|
@DeniseWorthen there originally were some answer changes that were worked through and resolved. The last set of logs is here: #1124 (comment) and show that we do not get unexpected diffs (this list is the expected non-b4b tests). Moreover, we do have a few regtests that test the coupled configurations in standalone mode. I added extra output there and I still get no answer changes. I'm continuing to work on this as fast as I can. I know progress is slower than we all would like. |
|
I think I don't understand the comment "b4b with itself". I thought that referred to being able to reproduce it's own baseline? |
|
@DeniseWorthen great catch. That was making sure moving forward things were as expected. I thought it looked strange that we didn't have more. The actual last set of logs are here: #1124 (comment) apologies for the mistake on my part. We do get answer changes in mod_defs, but not in the actual answers for the ufs1.1 tests which mimic some of the coupled tests. |
|
I believe EPIC wants to work on ufs-community/ufs-weather-model#2072 sometime this week. I wanted to check in to see if this issue was something that might be resolved soon, as that PR depends on this issue getting resolved. If not, it would be helpful to get a general idea on a timeframe to revisit that PR. Thanks for your time! |
|
@BrianCurtis-NOAA The PR here: #1211 anticipates the conflict coming when develop is going to be merged in. The merge of develop to dev/ufs-waether-model is not ready as I'm still unsure what the answer changes are but I do not believe that it should hold back ufs-community/ufs-weather-model#2072 Others should definitely chime in if they feel differently. |
Thanks @JessicaMeixner-NOAA . If there's no hold back anticipated then that's great. Let's see how 1211 discussions go. |
Describe the bug
The current dev/ufs-weather-model branch has an existing IC4 method=8; at the next merge from develop this will now need to be resolved with the new M8 and M9 added in PR #1176
There is an open UWM PR (ufs-community/ufs-weather-model#2072) to add a CICE6-WW3 coupling RT, which first required the fix for the divide by zero (PR #1163). To move forward UWM #2072, we now require an update from develop. We do not want to commit an RT using IC4 M8 only to have it break when the new M8 and M9 are brought to the mesh cap.
When is the next update to dev/ufs-weather-model from develop? The last update from develop was in October. Since there have been there have been multiple PRs merged at develop since then, they will all come to dev/ufs-weather-model at once. What is the plan for resolving the conflict w/ IC4 M8? Do we know that none of the intervening PRs break UWM? That is, were any UWM RTs run for any of these PRs?
The text was updated successfully, but these errors were encountered: