ufs-community / ufs-weather-model

UFS Weather Model
Other
129 stars 238 forks source link

update MOM6 to its main repo. 20240401 commit #2241

Closed jiandewang closed 3 weeks ago

jiandewang commented 4 weeks ago

Commit Queue Requirements:

Commit Message:

update MOM6 to its main repo. 20240401 commit (NCAR-candidate-20240319)

* UFSWM - 
  * MOM6 - 

Priority:

Git Tracking

UFSWM:

Sub component Pull Requests:

MOM6 PR: https://github.com/NOAA-EMC/MOM6/pull/132

UFSWM Blocking Dependencies:


Changes

Regression Test Changes (Please commit test_changes.list):

Input data Changes:

Library Changes/Upgrades:


Testing Log:

jiandewang commented 4 weeks ago

@jkbk2004 can you add this PR to the commit queue ? It is a relative simple one with no BL change so can be combined with other PR

FernandoAndrade-NOAA commented 3 weeks ago

@jiandewang #2173 has been merged in and we'd like to get started with testing for this PR, please go ahead and re-sync your branch, thanks!

jiandewang commented 3 weeks ago

@BrianCurtis-NOAA I saw you merged some of your fixing in this PR, are you plan to process this PR today ?

BrianCurtis-NOAA commented 3 weeks ago

@BrianCurtis-NOAA I saw you merged some of your fixing in this PR, are you plan to process this PR today ?

Yes, we are processing it right now. @zach1221 is running hera to make sure we get any early indication of failures, and i'm running on WCOSS2 to get it in before the prod switch tomorrow morning.

jiandewang commented 3 weeks ago

@BrianCurtis-NOAA I saw you merged some of your fixing in this PR, are you plan to process this PR today ?

Yes, we are processing it right now. @zach1221 is running hera to make sure we get any early indication of failures, and i'm running on WCOSS2 to get it in before the prod switch tomorrow morning.

thanks for the heads up. It is great that you are running this PR now as a new GFDL MOM6 candidate PR will come late this week. In ths way I can do clean test for the upcoming new code.

FernandoAndrade-NOAA commented 3 weeks ago

FYI added small fix to move directory checks a little lower after the mkdir commands in rt.sh , otherwise clean runs would result in errors due to those directories not yet being created.

zach1221 commented 3 weeks ago

Hera seems to be running through all the tests fine, but it's just taking a while. I'll go ahead and get the rest started.

zach1221 commented 3 weeks ago

I had to reset the jenkins runner, so I'm going to give the pipeline another try, but I've got the tests going manually in the background just in case.

jkbk2004 commented 3 weeks ago

I had to reset the jenkins runner, so I'm going to give the pipeline another try, but I've got the tests going manually in the background just in case.

Orion/Hercules went thru unexpected power outage. They are restored but maintenance is scheduled today.

zach1221 commented 3 weeks ago

Testing is complete. I have followed up on the mom6 sub-pr to begin the merging process.

jiandewang commented 3 weeks ago

just merged MOM6 (hash # 0730606b), is it the time for me to do the submodule revert ?

jiandewang commented 3 weeks ago

revert MOM6 submodule done

DusanJovic-NOAA commented 2 weeks ago

Why are these two new log files tests/logs/RT-run-Derecho.log and tests/logs/RT-run-Hercules.log committed to the repository?

This PR did not change the baselines yet tests/test_changes.list file indicates that 14 tests had changes, why? Shouldn't this file be empty if there are no baseline changes?

DeniseWorthen commented 2 weeks ago

Did @jiandewang ever commit a test change list? It seems the current list is from #2173. So does that mean that EPIC regenerated baselines those tests, even if that was not required?

DusanJovic-NOAA commented 2 weeks ago

The "bl_date.conf" has not be updated in this PR, which means no baseline changes. In that case "test_changes.list" must be empty, by definition. Conversely, whenever "bl_date.conf" is updated the "test_changes.list" file must not be empty, also by definition.

DeniseWorthen commented 2 weeks ago

Agreed. Does there need to be a logic check in the RT scripts to verify that the two conditions are met? Or do we rely on CMs checking that the two conditions agree?

In this case, I don't see that an updated test_changes.list was ever committed.

BrianCurtis-NOAA commented 2 weeks ago

I did approve it, so I missed those as well. Thanks for catching them. @jkbk2004 can we get those files removed from the repo in Ufuk's PR ?

jkbk2004 commented 2 weeks ago

I did approve it, so I missed those as well. Thanks for catching them. @jkbk2004 can we get those files removed from the repo in Ufuk's PR ?

@BrianCurtis-NOAA sure! @zach1221 @FernandoAndrade-NOAA FYI

jiandewang commented 2 weeks ago

Did @jiandewang ever commit a test change list? It seems the current list is from #2173. So does that mean that EPIC regenerated baselines those tests, even if that was not required?

I did committed an empty test_changes.list at the time when I generated this PR. Just re-cloned my original PR (created on Apr 15, hash # dc550f05). test_changes.list is an empty file

jiandewang commented 2 weeks ago

@BrianCurtis-NOAA I saw you merged some of your fixing in this PR, are you plan to process this PR today ?

I suspect the original empty test_changes.list was overwritten by this step, this is why @DeniseWorthen think it is from https://github.com/ufs-community/ufs-weather-model/pull/2173

DeniseWorthen commented 2 weeks ago

@jiandewang That is probably what happened, because as someone updates their (or someone else's) PR branch as develop moves along, they need to remember to always "git checkout --ours test_changes.list".