cam6_3_140: Separate history tapes into hXi and hXa

[peverwhee]

Overview

Per #159 - this PR changes the "time" variable in a history file with averaged (and/or min/max/stdev quantities) to be the midpoint of the averaging period (basically, the average of the time_bounds variable).

Side affects of the above/related changes in this PR (all implemented):

• Each history tape can now result in TWO files - hXa and hXi. The 'i' file is ALWAYS created and will, at minimum, contain time-dependent variables (like solar forcing data, etc). If only the instantaneous file is created, it will still carry the 'i' flag in the filename
• Changed 'time_bnds' variable name to 'time_bounds'
• Ensured the 'cell_methods' metadata field always includes "time: XXXXX" (where XXXXX refers to the avgflag) for history fields so that it's always clear what operation is being done on/for that field
• Any user-specified history file name or template will also have "a" or "i" appended to it and may result in two files
• time, date, datesec are all calculated based on the midpoint time for the accumulated file

Implementation

• amending the active_entry DDT "File" property to be an array of files called "Files" and adding a num_files attribute to determine if we have one or two files
• iterating over Files array and adding all necessary metadata and variables to both "i" and "a" files when necessary

Testing

• All regression tests run successfully (except for known failures)
• Wrote a script to compare the results vs baselines; confirmed no fields were lost in the transition and no answers changed (except the expected midpoint time variables on the accumulated file)

Related to: ESCOMP/CTSM#2019

closes #159
closes #554
Supersedes #869

[cacraigucar]

Asking a basic question to aid my understanding during the review process

[cacraigucar]

My brain is officially fried and I'm just doing the review!! I am impressed with the changes you've made!

First batch of review comments - more may be coming tomorrow

[cacraigucar]

This is a little fragile - assuming that the first file is accumulated and the second is instantaneous. With this current implementation, that is the case, but if someone flipped them down at line 5650, then it would no longer work. Since this history is nearing end-of-life and the likelihood of it getting broken is low, I won't require this be fixed, but if it is easy to determine which is which, I'd suggest that be used instead.

[gold2718]

Maybe assign a parameter to the instantaneous file?

[peverwhee]

I agree @cacraigucar that this is fragile. Good point.

@gold2718 what do you mean by "assign a parameter to the instantaneous file"?

My initial thought is to add "accumulate_file_index" and "instantaneous_file_index" parameters and use those instead of "1" and "2" throughout the code. But I'm open to other options (I just don't yet understand yours...)

[gold2718]

That is exactly what I was talking about, just using real language :)
I think those parameter names are fine except that I still don't really like 'instantaneous' because that does not describe which instant (I like last value because you get the last sample you happened to talk before writing the file).

[cacraigucar]

I would encourage you to keep "instantaneous" as this is what the ultimate file is called. If we were to change to "last value", we'd need to overhaul everything (including changing the "I" designator) and coordinate with CTSM. I believe "I" is what we are currently stuck with.

[brian-eaton]

@gold2718, I think what we're trying to convey in the filename is something about the temporal characteristics of the data. Focusing on the implementation of collecting the time samples (e.g., by overwritting the history buffer each time a sample is collected until it's time to write the buffer) ignores that other implementations are possible, like not putting anything in the buffer until the desired timestep is reached. The sample collection implementation doesn't describe the temporal characteristics of the data. Instantaneous does.

[gold2718]

In the new instantaneous files, does the time variable always match the time step of the sample?

[brian-eaton]

Yes.

It has always been the case that the time coordinate matches the end of the time step producing the data for an instantaneous field. From the code I've looked at so far the time coordinate in the hXi file is the same as it's always been for instantaneous fields. I am running tests as I review this PR to check these details.

[gold2718]

I do not think you answered my question. You are saying that the time that shows up in the file is the same as the time step active when the write occurs. I'm asking if the time that shows up in the file matches the time step that was active when the outfld buffer was captured for each and every field in the file. For instance, what if a particular diagnostic is only called every other time step and the write step is one where it was not captured.

[brian-eaton]

Take 2. Yes, the time variable always matches the time step of an instantaneous sample assuming that outfld was called during that time step.

You are correct to point out that outfld isn't called every time step for all fields that are output. Radiation fields are a prime example. To analyze fields like that one needs to look at the details of when the outfield calls are made, and at what happens during the time step(s) when outfld is not called. In the case of radiation, the heating rates that are applied during time steps when radiation isn't run (and outfld calls are not made) are from the most recent calculation (they are saved in the physics buffer). So even though the outfld call for the instantaneous field was not made at the time indicated by the time coordinate, the field is in fact valid at that time. I'm not claiming this to be the case for all fields that fall into this category. But I do claim that all fields in this category are instantaneous, even if there is a mismatch in the associated time coordinate.

[cacraigucar]

Whew - I finished my review and I've resolved the conversations which are fixed. Again, good job on these changes!

[cacraigucar]

Thanks for adding those new parameters. It helped me find some areas which I believe are bugs.

[nusbaume]

Great work @peverwhee! I mostly just have some minor clean-up suggestions.

[nusbaume]

Should we only be looping to t here, or should we be looping through all of the tapes (ptapes), which is what it looks like it was doing in the original version?

[peverwhee]

I changed it to t since this is a loop nested within a loop over ptapes, but now I'm questioning how that will work with threading, so i've changed it back to ptapes

[gold2718]

I seriously doubt wshist (if I got the right subroutine) can ever be threaded.

[nusbaume]

Ahh, I think I understand now! I am fine then with either ptapes or t (and your original t method is very likely more performant, at least for the average CAM configuration).

[peverwhee]

changed back to t

[nusbaume]

Thanks for dealing with my concerns! I am happy with this PR now.

[cacraigucar]

I left my one conversation unresolved as I'm not sure if the side discussion was finished or not. But my concern was addressed to my satisfaction

[brian-eaton]

Looks good. Nice work!

[ekluzek]

@peverwhee nice to see this in. We are going to study it in CTSM and figure out what we need to do using this as strong inspiration. Thanks so much.

[billsacks]

Thank you very much for your work on this, @peverwhee ! It's exciting to see this completed!!!