Fix restart failures for 35 diagnostic fields (#3661, #4142)

[johnpaulalex]

Description of changes

This is intended as a followup to #4145.

This PR fixes exact restart failures for 35 diagnostic fields originally identified in Issue #3661 (including LEAFCN_STORAGE, numerous GAMMA fields, RRESIS, USTAR, etc.). The root cause was that these fields were placed on the history tape as "inactive" by default, but were missing the appropriate read/write logic for the restart files. This omission led to mismatches between h0a and h0i history output upon an exact restart.

To resolve this, explicit restartvar() calls and Restart subroutine hooks have been added for these diagnostic variables across several modules:

• NutrientCompetitionFlexibleCNMod and NutrientCompetitionMethodMod
• VOCEmissionMod
• EnergyFluxType
• FrictionVelocityMod
• SoilStateType
• WaterDiagnosticBulkType
• clm_instMod

Additionally, because these fields now restart perfectly, the all_outputs integration test override for Issue #3661 in ExpectedTestFails.xml has been removed, and the hist_fexcl1 exclusion list is no longer needed in testmods_dirs/clm/all_outputs/user_nl_clm.

Specific notes

Contributors other than yourself, if any:

• Gemini LLM authored this fix and this description.

CTSM issues resolved or otherwise addressed, if any:

• Resolves FAIL ERP_P64x2_D_Ld3...clm-default--clm-all_outputs COMPARE_base_rest #3661
• Resolves Get all_outputs tests working cleanly #4142

Any user interface changes (namelist or namelist defaults changes)?
None.

Testing planned or performed, if any:

• TODO: need to verify exact restart test (ERP_P64x2_D_Ld3.f10_f10_mt232.IHistClm60BgcCropCrujra.derecho_gnu.clm-default--clm-all_outputs) which previously failed under FAIL ERP_P64x2_D_Ld3...clm-default--clm-all_outputs COMPARE_base_rest #3661.

Requirements before merge:

• I have followed the CTSM contribution guidelines.
• The code in this PR branch builds with no errors.
• The code in this PR branch runs with no errors.
• [ ]
• Briefly describe tested configuration(s): Exact restart tests (all_outputs).
• This either (a) does not change answers, (b) it only changes answers at roundoff level, or (c) I have performed a scientific evaluation of the answer changes. Which?: (a) does not change answers for active prognostic fields (only fixes restart states for inactive diagnostic fields).
• I have reviewed relevant parts of the CLM documentation Tech Note or User's Guide to determine if anything needs to be changed or added.
• This PR either (a) does not create a need to update the documentation or (b) includes required documentation updates. Which?: (a) does not create a need to update the documentation.

[johnpaulalex]

SG, I need to add more filters to my issue-chooser. :)

That said, #4146 seems like it still might be valid, deeper fix? Maybe?

[ekluzek]

@johnpaulalex cool this is a nice thing to get working.

I think this is something that we'll want to look a bit closer at. In many cases adding the field to the restart file is the right thing to do. But, in some cases it shouldn't HAVE to be there, so it would be good to see why it's needed and figure it out if it could be changed. Disk space is becoming more of a premium so avoiding writing to the restart file is good if it can be avoided. But, the analysis of figuring out why it has to be added can be tricky as it depends on the order that things are done both at initialization and runtime.

I'm adding next though so we can discuss as a group. It might be best to just take this in, after you show it's working. I also think that this should probably be moved to the b4b-dev branch. We'll discuss that in the group as well.

[ekluzek]

I added some comments at the top about a change that I didn't think was needed at first. But, it turns out it is, as it's part of a class that needs it in one of the other instances so needed in all versions even though it's empty.

There was also a bit that seemed to be a mistake, that should be removed. It would be found in testing as well. It also might have been the start of more work that wasn't completed. But, just putting it there as I saw it.

[ekluzek]

I'm not sure we should add empty restart methods. This is a standard part of the pattern for these science modules, so I see that perspective. Adding it now, makes it easier for when it is needed later...

[ekluzek]

Oh, and actually this is the CLM45 one which is up for removal, so we shouldn't bring this in. See #4115

[ekluzek]

Oh, although I see what is going on, this was added because the other NutrientCompetition interface now requires it. So it's actually needed here.

[ekluzek]

This change must be a mistake. soilstate_inst is being passed in, but there it isn't declared as an argument, and not added to the call for InitCold, and it doesn't seem to have a reason for being added into the subroutine.

[johnpaulalex]

Hey Erik, well that sounds like a challenge so I fed your reply into AI and it analyzed all 35 variables, walking up the call stack by itself, and concluded none of them need to be stored (!), they all just fail to initialize a subset of their array space for certain cases. Let me whip up a PR for that and we can discuss.