-
Notifications
You must be signed in to change notification settings - Fork 341
ctsm5.3.062: Put instantaneous and non-inst. fields on separate history files #2445
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.062: Put instantaneous and non-inst. fields on separate history files #2445
Conversation
|
This far I have tried one test: |
|
Conceptually, this seems like a good idea. Practically, this seems like a heavy lift. |
|
I think that consistency in the component output of CESM3 is quite important from a user's standpoint. CTSM would be an outlier amongst the 4 major components if these changes were not implemented. Looking at the project page, these changes have already been put into CAM and CICE. MOM came with these settings out of the box. @slevis-lmwg has a couple of pull requests with these changes for MOSART and RTM. I am a bit unclear where CISM is on implementing this. The only component that I know of that will likely not be implementing this is WW. |
|
I started to write a reply but @phillips-ad beat me to it. The critical piece of this is fixing time values for time-averaged fields so that the time is in the middle of the averaging period instead of at the end. As @phillips-ad says, it is likely to be particularly confusing if CTSM (and the river components) differs from other components in this respect. The separation of instantaneous from time-averaged fields is not a critical piece of this, but seemed like the best way to support having correct time values for both time-averaged fields and instantaneous fields. |
@billsacks I find the distinction in priority between the "middle of averaging period" task and the "separating instantaneous and non" task very helpful, because the former will take me a few days at most, while the latter will take me a few weeks at least. |
Thanks, that's good to know, @slevis-lmwg . I'm curious what the plan is, then, for instantaneous fields? Will you just let the time be incorrect for those fields for now? @phillips-ad do you agree that getting time correct for the time-averaged fields is the high priority thing here even if instantaneous fields aren't handled ideally for now? |
|
Yes I 100% agree, getting the time set to the middle of the averaging period for time-averaged fields is the higher priority. |
|
Just wanted to add: Separating instantaneous and time-averaged fields will probably be an issue for #2061, which adds a namelist option to enable all our history fields and put them all on the h0 file. Depending on which PR comes in first, we'll have to think about how we want to handle that. |
Thank you for pointing this out @samsrabin |
Yes, we will let the instantaneous fields be incorrect to start with. And then a later more involved change will fix that. I like the way that @samsrabin framed this when we discussed this. Right now "I" fields are correct, but "A" fields are incorrect. But, we care more about "A" fields (there's more of them output and they are the standard for most things) so getting them corrected at the expense of "I" fields is an improvement. |
Reduce outputs from matrixcnOn tests slevis resolved conflicts: src/main/histFileMod.F90
This comment was marked as resolved.
This comment was marked as resolved.
|
As part of work on #2838, I've added:
As part of this PR, the python code can be simplified to remove the non-instantaneous tape handling (where there's an option for both). Those new SystemTests can also be removed. |
Bring in several fixes for testing in the previous cesm3_0_beta03/04 tags Fix a list of issues mostly for testing that came in with the cesm3_0_beta03 and cesm3_0_beta04 tags for the science "chill" deadline bringing in the baseline science capabilities needed for the cesm3_0 release. List of things: - New polarcap grid - Fix use_init_interp for several resolutions that had problems - Remove clm5_1 physics option - Newly needed surface datasets added to auto build in Makefile for mksurfdata_esmf - Fix several individual tests that were failing - Update to submodules to ones roughly based on cesm3_0_beta04 - Add graceful error checking to the FATES parameter file tests that will get it working in some cases
|
Try Sam R.'s comparison tool.
Only one result to investigate and seems like a false DIFF: |
|
@samsrabin this commit earlier today fixed some RXCROP tests but it fails in these two: These two generate this error UPDATE I'm reverting above-mentioned commit and submitting the tests again, in case I'm missing something: |
|
Update: |
|
@phillips-ad just a heads up that I will merge this PR this week (as early as today). Let me know if you have a documentation file that you would like me to update accordingly. |
Thanks again for doing all of this @slevis-lmwg! The time change documentation is currently here; you should have editing permissions. This text will eventually be posted on the CESM3 webpage to document the output changes for every component. |
I'm seeing more issues than that with that file: |
|
Currently doing |
I see what you mean. I saw these and took them to be expected, but I now see that mosart (and presumably rtm) hXi files end up with the same metadata as hXa files. I will open issues in mosart (and rtm after I've checked rtm, too). |
|
Log is at
(Note that the output in that log file is for the Also, these expected failures: And these expected no-outputs: |
|
I think it's actually expected that those two tests should fail here. From the expected failures file: |
|
This PR took a while due to many competing priorities, but also due to the need for significant and somewhat subtle code changes. Now that it's done, I'd like to say thank you to various people who helped along the way and I hope I'm not forgetting anyone: |
|
Thank YOU very much, @slevis-lmwg ! I haven't followed all of the challenges and work needed to get this working, but I really appreciate all of your effort on this!! 🚀🎉 |
Description of changes
I started from issue #1059 and PR #2019 and am restarting here. From closed PR #2019 I preserved a few things:
In this PR we also intend to split all history tapes into instantaneous and non-instantaneous. The CAM group accomplished this in PR ESCOMP/CAM#903 and we plan to maintain consistency in the appearance of CLM and CAM history files. Possibly helpful in CAM's PRs:
The work will need to be repeated in
rtm1_0_87: Put instantaneous and non-inst. fields on separate history files RTM#32 and
Separate instantaneous from non-inst. history tapes MOSART#52
f19_g17.I*Clm60Bgcneed finidat with c13/c14 to PASS #3311history_tape_in_usefunctionality RTM#62 and MOSART does not have thehistory_tape_in_usefunctionality MOSART#119UPDATE
Due to higher priority, I completed the task of changing "time" to equal the middle of time_bounds in
#2838
ESCOMP/MOSART#106
ESCOMP/RTM#39
Specific notes
Contributors other than yourself, if any:
@ekluzek @billsacks
CTSM Issues Fixed (include github issue #):
Fixes #1059
Resolves ESCOMP/RTM#32
Resolves ESCOMP/MOSART#52
Are answers expected to change (and if so in what way)? No
Any User Interface Changes (namelist or namelist defaults changes)?
We intend to split all history tapes into instantaneous and non-instantaneous.