Make separate instantaneous and non-inst. history tapes #2445

slevis-lmwg · 2024-03-28T23:59:52Z

Description of changes

I started from issue #1059 and PR #2019 and am restarting here. From closed PR #2019 I preserved a few things:

If avgflag /= 'I', time_bounds is present and time = mid of time_bounds
Some long_name changes
Treating avgflag as a tape (not field) trait for 'I' and 'L' tapes

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.

UPDATE
Due to higher priority, changing "time" to equal the middle of time_bounds will now get done 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

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.

...and other mods that I'm preserving from closed PR ESCOMP#2019, such as - changes to long_names and - treating avgflag as a tape (not field) trait for 'I' and 'L' tapes

slevis-lmwg · 2024-03-29T17:20:50Z

This far I have tried one test:
ERP_D_P48x1.f10_f10_mg37.IHistClm51Bgc.izumi_nag.clm-decStart
The test completes as expected and shows different answers from dev175 only for the "time" variable.

wwieder · 2024-10-16T16:37:24Z

Conceptually, this seems like a good idea. Practically, this seems like a heavy lift.
I wonder how high a priority it should be for the CESM project and the CLM team?
How critical is this kind of consistency for CESM3?
@dlawrenncar and @briandobbins, could you weigh in here?
@billsacks and @phillips-ad, I think you've thought more about this too?

phillips-ad · 2024-10-16T17:52:24Z

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.

billsacks · 2024-10-16T18:12:42Z

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.

slevis-lmwg · 2024-10-16T18:40:16Z

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.

billsacks · 2024-10-16T18:51:49Z

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?

phillips-ad · 2024-10-16T20:22:33Z

Yes I 100% agree, getting the time set to the middle of the averaging period for time-averaged fields is the higher priority.

samsrabin · 2024-10-17T20:41:52Z

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.

slevis-lmwg · 2024-10-17T22:05:24Z

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

slevis-lmwg · 2024-10-18T00:23:23Z

Based on the recent conversation here, I have opened a new PR #2838 that has the same starting point as this one (#2445). The new PR will not make separate instantaneous and non-instantaneous history tapes.

ekluzek · 2024-10-23T16:32:39Z

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?

@billsacks

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

samsrabin · 2024-11-25T17:07:09Z

As part of work on #2838, I've added:

Code in python/ctsm/crop_calendars/ to handle either instantaneous or non-inst. tapes
RXCROPMATURITYINST and RXCROPMATURITYSKIPGENINST SystemTests, to test things when a certain tape is forced to be instantaneous.

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

slevis-lmwg · 2024-12-19T21:55:26Z

src/main/histFileMod.F90

@@ -49,6 +49,7 @@ module histFileMod
  integer , public, parameter :: scale_type_strlen = 32 ! maximum number of characters for scale types


Suggested change

integer , public, parameter :: scale_type_strlen = 32 ! maximum number of characters for scale types

integer , private, parameter :: scale_type_strlen = 32 ! maximum number of characters for scale types

As far as I can tell this parameter could be declared private.

The 3 public parameters above it get used outside of histFileMod.

In the context of this PR, do I need to worry about the corresponding section of code in src/utils/clmfates_interfaceMod.F90 which looks at fincl statements for FATES vars? Follow up.

slevis-lmwg · 2024-12-19T22:17:28Z

src/main/histFileMod.F90

@@ -402,6 +412,7 @@ subroutine hist_printflds()
    ! the CTSM's web-based documentation.

    ! First sort the list to be in alphabetical order
+    ! TODO Is t = 1 argument needed?
    call sort_hist_list(1, nallhistflds, allhistfldlist)


Suggested change

call sort_hist_list(1, nallhistflds, allhistfldlist)

call sort_hist_list(nallhistflds, allhistfldlist)

As far as I can tell, the t = 1 argument and later the plain t argument serve no purpose in subroutine sort_hist_list.

slevis-lmwg · 2024-12-19T23:09:33Z

I got brave and submitted a test:
SMS_D.1x1_brazil.I2000Clm60Sp.derecho_gnu

See errors in the bld.log:
/glade/derecho/scratch/slevis/SMS_D.1x1_brazil.I2000Clm60Sp.derecho_gnu.20241219_160820_coctcl/bld/lnd.bldlog.241219-160827

This commit does not resolve all the errors

src/main/histFileMod.F90

slevis-lmwg · 2024-12-28T00:06:22Z

These three tests finish successfully, though I have not started looking at history output:

SMS_D.1x1_brazil.I2000Clm60Sp.derecho_gnu
SMS_D.1x1_brazil.I2000Clm60BgcCrop.derecho_gnu
SMS_D.1x1_brazil.I2000Clm60BgcCrop.derecho_gnu.clm-output_crop_highfreq

This longer test
SMS_D_Lm2.1x1_brazil.I2000Clm60BgcCrop.derecho_gnu.clm-output_crop_highfreq
fails with
ERROR: 0 NetCDF: Not a valid ID err_num = -33
from what I can tell while writing
current time sample to local history file ./SMS_D_Lm2.1x1_brazil.I2000Clm60BgcCrop.derecho_gnu.clm-output_crop_highfreq.20241227_173328_jlalex.clm2.h01.2000-01.nc at nstep = 1488 for history time interval beginning at 0. and ending at 31.

(See write-statements in latest test in /glade/derecho/scratch/slevis)

This Lm2 test passed here:
/glade/derecho/scratch/slevis/SMS_D_Lm2.1x1_brazil.I2000Clm60BgcCrop.derecho_gnu.clm-output_crop_highfreq.20250102_172648_o6z3me/run
HOWEVER, comparing the clm2.h01 and clm2.h02 files, the former seems correct (though I have not compared to a baseline) but the second is incorrect. I may want to look into adding the "file" dimension to time and/or the list of fields...

Matches commit eeedbc6 in ESCOMP#2838

slevis-lmwg · 2025-01-04T00:38:46Z

The same Lm2 test continues to pass with the latest commits AND it correctly separated an 'I' field from the rest (this may be the only default active 'I' field). Time-related fields also look correct but their metadata appear wrong...

slevis-lmwg · 2025-01-04T01:27:44Z

The latest problem that I have spotted:
The 'I' field that appears in h0i (as it should), continues to also appear in h0a...

UPDATE

The history field lists seem ok in the lnd.log but somehow the 'I' field (SNOW_PERSISTENCE) shows up in h0a. Why?
The history field lists seem to include h0, h1, h2, ... in the lnd.log, yet a later comment states that we will get only 1 tape. Why?

I have added new write statements but have not tested them. Also, does (1) possibly come in with the call hfields_write?
nflds must have the wrong value in line 1001 for ntapes to end up equal to 1. I now think ntapes needs to be dimensioned by maxsplitfiles OR (preferred?) line 1001 may need to say
if (any(tape(t)%nflds(:)) > 0) then

If avgflag /= 'I', time_bounds is present and time = mid of time_bounds

258792e

...and other mods that I'm preserving from closed PR ESCOMP#2019, such as - changes to long_names and - treating avgflag as a tape (not field) trait for 'I' and 'L' tapes

slevis-lmwg self-assigned this Mar 28, 2024

slevis-lmwg added enhancement new capability or improved behavior of existing capability priority: high High priority to fix/merge soon, e.g., because it is a problem in important configurations labels Mar 29, 2024

slevis-lmwg and others added 4 commits April 1, 2024 16:36

Begin to add a file number dimension to permit 2+ files per hist. tape

9c7df7f

WIP (cont'd): Adding file number dimension to permit 2+ files per tape

42d944d

WIP (cont'd): Advancing through TODOs to permit 2+ files per tape

a37d9da

Minor comment update to keep track of progress

15535a0

wwieder added this to the CESM3 Answer changing freeze milestone Jun 20, 2024

slevis-lmwg mentioned this pull request Oct 18, 2024

ctsm5.3.018: Change history time to be equal to the middle of the time bounds #2838

Merged

slevis-lmwg changed the title ~~Make separate instantaneous and non-inst. history tapes; for the latter set time = mid of time_bounds~~ Make separate instantaneous and non-inst. history tapes Oct 18, 2024

slevis-lmwg removed the priority: high High priority to fix/merge soon, e.g., because it is a problem in important configurations label Oct 18, 2024

Merge tag 'ctsm5.3.009' into sep_hXi_hXa_tapes_iss1059

aff6ac4

Reduce outputs from matrixcnOn tests slevis resolved conflicts: src/main/histFileMod.F90

This comment was marked as resolved.

Sign in to view

slevis-lmwg added 2 commits December 17, 2024 12:50

Minor update to histFileMod, mainly in TODO comments

7803fd4

slevis-lmwg added 5 commits December 18, 2024 14:39

WIP (cont'd): Allow 2+ restart files per tape to mirror the hist files

2304d59

WIP (cont'd): Part (b) of the last commit

93007dd

WIP (cont'd): Part (c) of the last commit

16217fe

Clean-up some new and existing do-loops

5810d1d

Small correction and TODO updates

fbabe7c

slevis-lmwg commented Dec 19, 2024

View reviewed changes

Small correction and some clean-up

e574acb

Corrections to resolve various errors in the SHAREDLIB_BUILD phase

30d7904

This commit does not resolve all the errors

slevis-lmwg commented Dec 27, 2024

View reviewed changes

src/main/histFileMod.F90 Show resolved Hide resolved

Change history_tape_in_use* from logical to integer for ncd_io to work

63a4db6

slevis-lmwg added 4 commits January 3, 2025 11:47

Add the 'file' dimension to ntimes; test passes but output is not good

6f56d01

Change hist file labels from h01, h02 to h0i, h0a

ebf652e

Change "if instantaneous" statemt with more appropriate conditional

d9fdab0

Matches commit eeedbc6 in ESCOMP#2838

Add "file" dimension to actflag to separate the 'I' fields

16d1e29

Correct time-related fields that appear on instantaneous file

f1b0685

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make separate instantaneous and non-inst. history tapes #2445

Make separate instantaneous and non-inst. history tapes #2445

slevis-lmwg commented Mar 28, 2024 •

edited

Loading

slevis-lmwg commented Mar 29, 2024

wwieder commented Oct 16, 2024 •

edited

Loading

phillips-ad commented Oct 16, 2024

billsacks commented Oct 16, 2024

slevis-lmwg commented Oct 16, 2024

billsacks commented Oct 16, 2024

phillips-ad commented Oct 16, 2024

samsrabin commented Oct 17, 2024

slevis-lmwg commented Oct 17, 2024

slevis-lmwg commented Oct 18, 2024

ekluzek commented Oct 23, 2024 •

edited

Loading

This comment was marked as resolved.

samsrabin commented Nov 25, 2024

slevis-lmwg Dec 19, 2024

slevis-lmwg Dec 19, 2024

slevis-lmwg commented Dec 19, 2024 •

edited

Loading

slevis-lmwg commented Dec 28, 2024 •

edited

Loading

slevis-lmwg commented Jan 4, 2025

slevis-lmwg commented Jan 4, 2025 •

edited

Loading

		@@ -49,6 +49,7 @@ module histFileMod
		integer , public, parameter :: scale_type_strlen = 32 ! maximum number of characters for scale types

	integer , public, parameter :: scale_type_strlen = 32 ! maximum number of characters for scale types
	integer , private, parameter :: scale_type_strlen = 32 ! maximum number of characters for scale types

	call sort_hist_list(1, nallhistflds, allhistfldlist)
	call sort_hist_list(nallhistflds, allhistfldlist)

Make separate instantaneous and non-inst. history tapes #2445

Are you sure you want to change the base?

Make separate instantaneous and non-inst. history tapes #2445

Conversation

slevis-lmwg commented Mar 28, 2024 • edited Loading

Description of changes

Specific notes

slevis-lmwg commented Mar 29, 2024

wwieder commented Oct 16, 2024 • edited Loading

phillips-ad commented Oct 16, 2024

billsacks commented Oct 16, 2024

slevis-lmwg commented Oct 16, 2024

billsacks commented Oct 16, 2024

phillips-ad commented Oct 16, 2024

samsrabin commented Oct 17, 2024

slevis-lmwg commented Oct 17, 2024

slevis-lmwg commented Oct 18, 2024

ekluzek commented Oct 23, 2024 • edited Loading

This comment was marked as resolved.

samsrabin commented Nov 25, 2024

slevis-lmwg Dec 19, 2024

Choose a reason for hiding this comment

slevis-lmwg Dec 19, 2024

Choose a reason for hiding this comment

slevis-lmwg commented Dec 19, 2024 • edited Loading

slevis-lmwg commented Dec 28, 2024 • edited Loading

slevis-lmwg commented Jan 4, 2025

slevis-lmwg commented Jan 4, 2025 • edited Loading

slevis-lmwg commented Mar 28, 2024 •

edited

Loading

wwieder commented Oct 16, 2024 •

edited

Loading

ekluzek commented Oct 23, 2024 •

edited

Loading

slevis-lmwg commented Dec 19, 2024 •

edited

Loading

slevis-lmwg commented Dec 28, 2024 •

edited

Loading

slevis-lmwg commented Jan 4, 2025 •

edited

Loading