Open billsacks opened 6 years ago
Bill Sacks < sacks@ucar.edu > - 2014-07-31 14:47:52 -0600
We probably want to use this option for one test for each configuration... this could replace the addition of select fields, which we do for some tests.
Blocks #1347
I took a closer look at this. It looks like the logic is spread over two places: 'global' settings are applied from namelist variables here and then if the global settings have no opinion, the 'local' settings are applied. The local logic is in Bill's code snippet above; basically all history fields are written by default, but the caller can opt out of that by passing in 'inactive' to an optional parameter called 'default' (example).
There are already a few relevant namelist flags affecting the global logic (per the code link above), the logic is a bit complex, and the ordering matters:
fwiw all local votes are recorded in a global masterlist.actflag variable, and later (at the end of the simulation I'm guessing), the logic above is executed.
I think it makes most sense to keep all the global flags together, so I propose to modify the logic above to check some new flag ('hist_all'?) after checking the other three global flags. With that ordering, the fexcl flag can still be used to remove specific history fields.
In addition I thought it'd make sense to leave some documentation breadcrumbs in the code: 1) if you find one global flag, find out about the others 2) if you find the local logic, find out about the global logic, and vice versa.
On a minor note, I could make it crash if both hist_empty_htapes and the new flag (hist_all?) are set, since that's contradictory, but I'm not sure if that's CESM's usual policy for handling inconsistent flag settings.
@johnpaulalex thank you for your careful work on this, as usual! You have come to understand this better than I have at this point :-) And yes, histFileMod is some of our oldest infrastructure code in CTSM, so it has accumulated some complexity and cruft over time....
I like your suggestion of where to insert hist_all into the ordering so that hist_fexcl can still be used to remove specific history fields.
One thing I may not have said when we talked – and I'm not sure if it's clear to you from your trace of the code logic – is that the default/inactive logic only applies to the first history "tape" (h0). My initial thought was that this new namelist option should just apply to the first history tape, and I think that's what my hacky code snippet would have accomplished, but I suppose it could be a vector namelist option like many of the other history-related namelist flags and so could theoretically apply to any history tape. Since, in practice, I only see this being used for software testing, I'd say do whichever one is cleanest to implement (unless @ekluzek or others have opinions).
As for the name, we now tend towards longer, more explicit flag names than have been used in the past, so maybe something like, "hist_all_fields"?
Yes, adding more documentation would be fantastic - thank you!
And yes, we do try to add error-checking to prevent incompatible flags. We try to put this in at least one of (1) CLMBuildNamelist.pm and/or (2) the Fortran code. We try for (1) when feasible (the advantage is that this catches incompatibilities at build-namelist time, which typically means at build time or model submission time rather than waiting for run time), and it's not uncommon to do both (on the off-chance that a user has force-created a lnd_in file without going through the build-namelist script, and also because this makes the code more self-documenting as to incompatibility of flags).
@ekluzek see above if you aren't currently following this issue.
Also @slevis-lmwg I want to bring you in the loop here because this might have a bit of interaction with #1059 .
Bill Sacks < sacks@ucar.edu > - 2014-07-31 14:44:46 -0600 Bugzilla Id: 2022 Bugzilla CC: bandre@lbl.gov, erik@ucar.edu, muszala@ucar.edu, rfisher@ucar.edu,
You can turn on all history fields by ignoring a few lines of code in histFileMod:
It would be useful to have a namelist option that does this, for testing.
Then we should add a test that uses this.