-
Notifications
You must be signed in to change notification settings - Fork 341
this change is needed for proper operation of st_archive #3354
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
this change is needed for proper operation of st_archive #3354
Conversation
|
Thanks, Jim!
↑ Moved to Issue #3359. |
|
In order to add that test the cime and cmeps pr's will need to be merged and the ctsm externals updated to include them. |
|
Ah, thanks for the heads up. That's ESCOMP/CMEPS#576 and ESMCI/cime#4827, right? We're updating our submodules as we speak with PR #3353, so hopefully it will be simple to merge those new additional tags if they're made soon. Do you have a sense of when you or someone else will be able to get those done? I'm also assuming it requires ESCOMP/MOSART#115, right? Or would that not be affected by the test? |
|
That's correct, it also needs the mosart PR. |
slevis-lmwg
left a comment
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.
I'm approving, thanks @jedwards4b!
|
I will rebase to b4b-dev. If that's wrong, we can rebase back to master. |
|
The cime PR has been merged in cime6.1.110 |
… fix_archive_issue
I tested |
|
Can you point me to the test that you ran? |
|
Sure, it's at |
|
It looks like it passed fortuitously. If you look at the directory |
|
Hmm. I can't get it to break with either of those modifications:
Looks like case2 just always uses the first restart, which is always okay. I can't figure out how that's getting set. |
|
I found it - it's done in err.py, and yes it is always choosing the first one. I am trying with a modification in err.py and will make that change if it works (or rather fails). |
|
@samsrabin I have made a change so that ERR_Ld9.f10_f10_mg37.I1850Clm60BgcCrop.derecho_intel.drv-interim_restart restarts on 0001-01-05-00000 instead of 0001-01-03-00000, however it turns out that the |
|
Makes sense, thanks! |
|
@jedwards4b Is it possible there's a race condition in the IRT SystemTest? I had one passing at 8a9c25d but then failing when I tried later. This prompted me to try five more replicates, of which only one passed. Here are the directories and their results:
The failures all have this in |
|
It's not a race condition exactly, it's a faulty method of sorting the restart directories by using mtime. I am experimenting with an alternate method of sorting and will let you know if it's more consistent. By the way I noticed that the ERR test was doing this too, but wrote it off to human error until you said something - thanks. |
|
Awesome, thanks! |
|
I'm going to merge this PR for now and will open a new one once there's a CIME tag with a fix. |
Description of changes
I submitted this before and it was reverted without explaination. Now submitting again to address #3351
Specific notes
Contributors other than yourself, if any:
CTSM Issues Fixed:
Are answers expected to change (and if so in what way)? No
Any User Interface Changes (namelist or namelist defaults changes)? No
Does this create a need to change or add documentation? Did you do so? No
Testing performed, if any:
ERR.f09_t232.I1850Clm60BgcCrop.derecho_intel.drv-interim_restart