Skip to content
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

ctsm5.3.019: Stop running 0th time step #2084

Merged
merged 32 commits into from
Jan 14, 2025

Update ChangeLog

71e56b9
Select commit
Loading
Failed to load commit list.
Merged

ctsm5.3.019: Stop running 0th time step #2084

Update ChangeLog
71e56b9
Select commit
Loading
Failed to load commit list.
Task list completed / task-list-completed succeeded Jan 14, 2025 in 0s

3 / 3 tasks completed

All tasks have been completed

Details

Required Tasks

Task Status
Generating new baselines and rerunning the mosart/rtm test suites, because he expects no diffs from the code changes to those components. Incomplete
A F2000 simulation as confirmation that all this works correctly in coupled mode now that CAM has eliminated the 0th time step, as well:
Updated my cesm to cesm3_0_alpha05a
./create_newcase --compset 2000_CAM70_CLM60%SP_CICE%PRES_DOCN%DOM_MOSART_SGLC_SWAV --res ne30pg3_t232 --case /glade/u/home/slevis/cases_LMWG_dev/f2000.ne30_t232.SP --run-unsupported
./case.build currently returns this error (even after the Forums suggested conda activate ctsm_pylib):
ERROR: Cannot modify case, read_only. Case must be opened with read_only=False and can only be modified within a context manager
Creating the case from Cecile's checkout of cesm3_0_beta04 WORKED, so now I need to use the case's /SourceMods.
BUT it complicates things that this tag points to ctsm5.3.007, mosart1.1.02, rtm1_0_80.
I now cloned my own checkout of cesm3.0-alphabranch, which points to the same component versions as Cecile's. Incomplete
2-day versus 1+1-day simulations are b4b. Incomplete
branch? Incomplete
Answers changed from the baseline due to removal of 0th time step (expected) Incomplete
Answers also changed in restart (or branch) but just for one field described as constant Incomplete
Answers in the next PR, which includes the code from this PR, didn't change and restart passed Incomplete
So I'm inclined to ignore the answer change in restart in this PR for now Incomplete
In the ChangeLog, repeat the temporary issues that I reported in ctsm5.3.018 that I will resolve in ctsm5.3.020. Completed
Could you comment on my ChangeLog update or modify it further yourself? Incomplete
Anything else you can think of? Incomplete
As we discussed, we will test this code in an F-case with the latest cesm tag very soon. I am not clear whether that's already available, but let's follow up over the next few days. Incomplete
There are a few things going on: Incomplete
Izumi nag tests that failed to build due to a bug in rtm and mosart. I entered the fix manually in this PR to show that it does not change answers. The fix will go in with the modules update in ctsm5.3.020. Incomplete
We see diffs in mosart and cpl output that appear in two aux_clm tests with cism active and are discussed here:
#2838 (comment)
ESCOMP/MOSART#103 (comment)
These also get fixed with the modules update in ctsm5.3.020. Incomplete
I have added a new comment to the ChangeLog to indicate that the mosart and rtm code changes alone do not change answers here: Incomplete
Could you elaborate regarding the possible need for FATES testing in the google chat that we have going? Incomplete
Regarding your question in our google chat about my recent commit f3a1dc6:
The new rpointer file naming made this change necessary in continue and branch cases. Without the change, the model looked for rpointer files with a 01800 suffix and failed. This would have passed without the change prior to the new rpointer file naming due to the generic names used before. Let me know if you want to discuss this or anything else further. Incomplete
Thanks for the explanation and the change to the log, that makes sense. Incomplete
I don't really know if FATES testing is needed or not, I was just being cautious based on the recent problem encountered with Meier. Are there any FATES tests in aux_clm? If so, maybe that's good enough? Incomplete
Good to know that the change was required due to the implementation of a new feature, thanks. Incomplete
Yes, there are a few Fates tests in aux_clm Incomplete
I will run the fates test-suite before merging ctsm5.3.020 Incomplete
Yes, there are a few Fates tests in aux_clm Incomplete
I will run the fates test-suite before merging ctsm5.3.020 Incomplete
Great, thanks for this change. I agree with this and we can remove the above BACKWARDS_COMPATIBILITY comment. Completed
Generally: As you know, this PR removes references to the 0th time step Incomplete
Specifically: Keith removed the ".and. (nstep /= 0)" part of this if-statement Incomplete
In the latest version, the if-statement became "time_to_reset" and the nstep became "effective_nstep" Incomplete
My question: Is it ok that I removed the effective_nstep part of the line, the same way that Keith originally removed the nstep part? Incomplete
I'm pretty sure this change should be reverted. effective_nstep refers not to the number of timesteps since run start/restart, but rather to the number of timesteps since the accumulator field was initialized/reset. Completed
I assume you mean the aux_clm unit tests rather than the python unit tests, correct? Incomplete
If so, you are suggesting that I run once without reverting the above and let you know what happens, yes? Incomplete