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

add namelist option to turn on all history fields, add a CLM test that uses this #29

Open
billsacks opened this issue Dec 16, 2017 · 4 comments · May be fixed by #2061
Open

add namelist option to turn on all history fields, add a CLM test that uses this #29

billsacks opened this issue Dec 16, 2017 · 4 comments · May be fixed by #2061
Labels
bfb bit-for-bit blocker another issue/PR depends on this one good first issue simple; good for first-time contributors testing additions or changes to tests

Comments

@billsacks
Copy link
Member

billsacks commented Dec 16, 2017

Bill Sacks < [email protected] > - 2014-07-31 14:44:46 -0600
Bugzilla Id: 2022
Bugzilla CC: [email protected], [email protected], [email protected], [email protected],

You can turn on all history fields by ignoring a few lines of code in histFileMod:

Index: src/clm4_5/main/histFileMod.F90
===================================================================
--- src/clm4_5/main/histFileMod.F90    (revision 61424)
+++ src/clm4_5/main/histFileMod.F90    (working copy)
@@ -4139,9 +4139,6 @@
          p2c_scale_type=scale_type_p2c, c2l_scale_type=scale_type_c2l, l2g_scale_type=scale_type_l2g)

     l_default = 'active'
-    if (present(default)) then
-       l_default = default
-    end if
     if (trim(l_default) == 'inactive') then
        return
     else
@@ -4429,9 +4426,6 @@
          no_snow_behavior=no_snow_behavior)

     l_default = 'active'
-    if (present(default)) then
-       l_default = default
-    end if
     if (trim(l_default) == 'inactive') then
        return
     else

It would be useful to have a namelist option that does this, for testing.

Then we should add a test that uses this.

@billsacks billsacks added this to the clm5 milestone Dec 16, 2017
@billsacks billsacks added the enhancement new capability or improved behavior of existing capability label Dec 16, 2017
@billsacks
Copy link
Member Author

Bill Sacks < [email protected] > - 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.

@billsacks
Copy link
Member Author

Blocks #1347

@johnpaulalex
Copy link
Contributor

johnpaulalex commented May 24, 2023

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:

  • If hist_fincl says the specific field is on, return ON
  • If hist_empty_htapes says every field is off, return OFF
  • If hist_fexcl says the specific field is off, return OFF
  • otherwise, return the local vote (as described above)

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.

@billsacks
Copy link
Member Author

@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 .

slevis-lmwg pushed a commit to slevis-lmwg/ctsm that referenced this issue Aug 1, 2023
jedwards4b added a commit to jedwards4b/ctsm that referenced this issue Mar 28, 2024
ad6b84e Bump to 0.7.3
e6264da Merge pull request ESCOMP#29 from ESMCI/explainM
3a9201d better error handling for not a tag
b4e07be fixes issue with ssh access to github (should not be required)
3fba385 explain M flag

git-subtree-dir: .lib/git-fleximod
git-subtree-split: ad6b84e
jedwards4b added a commit to jedwards4b/ctsm that referenced this issue Jun 2, 2024
AGonzalezNicolas pushed a commit to HPSCTerrSys/clm5_0 that referenced this issue Jun 27, 2024
…OMP#29)

Key commits:

* 3b3f94d Fixed "Map Grid Size mismatch error" and "array PARTITION error"
* f1069e6 Set MPIFC when configuring MCT
AGonzalezNicolas added a commit to HPSCTerrSys/clm5_0 that referenced this issue Jun 27, 2024
…N error" (ESCOMP#29) 'f06da89b136147493ee0d555429ca888621a473c' into eclm
@samsrabin samsrabin added simple bfb bit-for-bit and removed simple bfb labels Aug 8, 2024
@samsrabin samsrabin removed the simple label Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bfb bit-for-bit blocker another issue/PR depends on this one good first issue simple; good for first-time contributors testing additions or changes to tests
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants