CAM history erroneously overrides averaging flags for nhtfrq=1

[peverwhee]

What happened?

If nhtfrq=1, CAM history currently overrides (since cam6_3_140) the averaging flag to 'I'

    !
    ! Initialize history variables
    !
    do t=1,ptapes
      do fld=1,nflds(t)
        if (nhtfrq(t) == 1) then
           ! Override any non-I flags if nhtfrq equals 1
           tape(t)%hlist(fld)%avgflag = 'I'
        end if
        if (tape(t)%hlist(fld)%avgflag .ne. 'I') then
           hfile_accum(t) = .true.
        end if

BUT: as @adamrher pointed out, this neglects subcycling averages.

What are the steps to reproduce the bug?

Run CAM, outputting a variable that exists in a subcycle (like a CLUBB variable in the macmic loop) with the 'A' flag and observe that the results are the same as for the 'I' flag.

What CAM tag were you using?

cam6_4_032

What machine were you running CAM on?

CGD machine (e.g. izumi)

What compiler were you using?

GNU

Path to a case directory, if applicable

No response

Will you be addressing this bug yourself?

Any CAM SE can do this

Extra info

No response

[gold2718]

This happened in #903, see this comment.

[adamrher]

Is there a way to query how many times an outfld call for a particular variable is called in a time-step? If we could get this number N than we could make logic that, for this special case of nhtfrq=1 and avgflag_pertape='A', forces all N=1 variables into the 'I' tapes and the N>1 variables into the 'A' tapes (and adjust their metadata accordingly).

[gold2718]

I would vote for a new, optional input to addfld to indicate this type of field. Something like sampled_on_subcycle or allow_single_timestep_average.

[adamrher]

One complication with that approach is I could set up the namelists to do no subcycling -- e.g, for ne240pg3, we don't subcycle in the macmic loop at all.

[gold2718]

One complication with that approach is I could set up the namelists to do no subcycling -- e.g, for ne240pg3, we don't subcycle in the macmic loop at all.

Of course but is that a bad thing? What is the impact of allowing averaging (or other collection methods) for this case? Is there an impact beyond the timestamp being in the middle of the time step instead of the current time (lines 5816 - 5822) in cam_history.F90)?

[adamrher]

I don't know if there's an adverse impact other than creating inconsistent time metadata relative to the non-subcycled variables in this case, but I agree this is preferable to the current method.

[peverwhee]

@adamrher I somehow missed this entire conversation when I implemented https://github.com/ESCOMP/CAM/pull/1163. I am happy to add a flag to addfld, but can you point me to the addfld calls that (e.g. in the macmic loop) that should have this flag turned on?

[brian-eaton]

Hi @adamrher, @gold2718. Sorry to jump in late here but I was not working in September. Steve correctly points to the original discussion about why the averaging flag is automatically set to 'I' when nhtfrq=1. In a nutshell, because of the decision to output mid-interval timestamps for non-instantaneous data, an averaged field output every timestep would get a mid-timestep timestamp which is not correct, except possibly for this edge case of sampling inside a subcycle. I don't think this override should be reverted.

outfld calls inside a subcycle loop is a case not dealt with by the current history functionality. I could want to output instantaneous states inside the subcycle loop, or do some operation like taking an average or a max/min value over the subcycle loop. Considering where we are in CAM's development process (moving towards the SIMA implementation), it doesn't seem worth the effort to implement this functionality in the history module. I would advocate that a subcycle average be computed in a local variable and output by an outfld call outside the subcycling, or with logic to just call outfld on the final subcycle. What do you think?

[adamrher]

Hi @brian-eaton. I did benefit from having the ability to do 'A' flags for nhtfrq=1, prior to putting that override in earlier this year. Only outputting one of the subcycle values is not very useful -- it's just an incomplete picture of the tendencies and states produced during a single time-step. I understand that the history code was not intended to work with subcycled quantities, but it just happened to work, and aided in development.

Making local arrays for computing subcycled quantities in tphysac / tphysbc seems like a lot of extra arrays, when @gold2718's suggestion of adding an optional argument to addfld for subcycled variables that creates an exception to the override seems pretty clean IMO (see Courtney's PR commit here). But I could easily be overlooking something. Do you think it's more complicated than that?

[brian-eaton]

I spoke to Courtney today and it sounds like she was able to implement Steve's suggestion and leave the override in place for fields that don't set the sampled_on_subcycle=.true. option in the addfld call. That sounds good to me!

[adamrher]

Great, sounds like a plan.