ctsm5.4.050: Remove FatesColdSatPhen testmod

[samsrabin]

Description of changes

Removes the FatesColdSatPhen testmod, which didn't actually set anything having to do with SP mode (see #4150). Tests that included FatesColdSatPhen now include FatesColdDailyHist instead. The latter has replaced FatesColdBasic, which was previously included by FatesColdSatPhen and has now been updated to include the history file setting changes from FatesColdSatPhen's user_nl_clm.

All remaining Fates*SatPhen* testmods now explicitly set use_fates_sp = .true.. The justification: If you're only testing that standard FATES SP mode works, use a FatesSp compset so that you also test the compset. That's why we deleted the FatesColdSatPhen testmod—to ensure the FatesSp compsets are tested. However, these other testmods exercise special SP-mode settings. They should still exist, and they should all set FATES SP mode as a backup in case a non-FatesSp compset is chosen.

• Remove temporary remove-fatescoldsatphen-testmod test suite

Specific notes

Contributors other than yourself, if any: None

CTSM issues resolved or otherwise addressed, if any:

• Resolves FatesColdSatPhen testmod doesn't actually set SP mode #4150

If answers are expected to change, describe (delete this line otherwise): Answers are not expected to change in any meaningful sense. However, there will be diffs in the former

SMS_Lm1.f45_f45_mg37.I2000Clm60FatesSpCruRsGs.derecho_intel.clm-FatesColdBasic

test, which is now

SMS_Lm1.f45_f45_mg37.I2000Clm60FatesSpCruRsGs.derecho_intel.clm-FatesColdDailyHist

and is for the first time getting the history file setting changes that were in FatesColdSatPhen's user_nl_clm.

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

Testing planned or performed, if any:

• aux_clm suite
• fates suite

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):
• 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)

• 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. If it does, describe:
• This PR either (a) does not create a need to update the documentation or (b) includes required documentation updates (see guidelines for contributing documentation). Which?: (b)

[ekluzek]

OK this is good. I don't really like the testmods that explicitly set use_fates_sp==TRUE. But, I can relent and accept that. However, do put comments in the user_nl_clm files that give the justification and that in general you should use them in tests with FatesSp compsets. And then I can accept that. This could also be pointed out in the testlist, and I have one suggestion at a change to the comment in the testlist to say that. I think that's helpful, but not as important as a comment in the testmod.

[ekluzek]

I really think that we shouldn't set this in the testmod, that we should use the FatesSp compset. I think there are other things besides use_fates_sp that are set there now, and even if there aren't they may well be more in the future.

[ekluzek]

Now, your notes give this:

"The justification: If you're only testing that standard FATES SP mode works, use a FatesSp compset so that you also test the compset. That's why we deleted the FatesColdSatPhen testmod—to ensure the FatesSp compsets are tested. However, these other testmods exercise special SP-mode settings. They should still exist, and they should all set FATES SP mode as a backup in case a non-FatesSp compset is chosen."

OK, I can see that.

But, add a comment that says that in the user_nl_clm. I think the tests for these modes should use the FatesSp compset, but as they are generally testing something slightly outside of that, it's probably OK to have that here.

[ekluzek]

Same as above add a comment that talks about this and that in general this should be used with a FatesSP compset.

[ekluzek]

Same as above add a comment about this.

[ekluzek]

From your notes you'll remove this new temporary testlist when you are done, which is good.

[ekluzek]

I do really like this more descriptive name.

[ekluzek]

You could add something here that points out that this should use a FatesSp compset.

[ekluzek]

Suggested change

	<option name="comment">Test with prescribed LAI and soil moisture with FatesSP mode</option>
	<option name="comment">Test with prescribed LAI and soil moisture with FatesSP mode (be sure to use a FatesSp compset along with the FatesColdSatPhen_prescribed testmod)</option>

[ekluzek]

This might also go other places as well...

[samsrabin]

Why, though? Is the thinking that these testmods won't work without a FatesSp compset?

[ekluzek]

OK, good question. This is really a testing philosphy type thing for me. Which might need a longer conversation.

But, as one point, I think our testing should be small deviations from standard ways that the model is run. And we should test and retest those standards ways as often as possible. So if you need FatesSp for a test you do it with the FatesSp compset, so that you are testing the standard compset alongside the test that's a deviation from the standard compset.

The other part is to ensure that as things change things don't work in the test -- but would've broken if the FatesSp compset was used. With this the two can evolve seperately, and that could cause problems as those change.

Now, an alturnative way to do this would be to add this to the include_user_mods:

../../../usermods_dirs/clm/fates_sp

as that will ensure they are aligned as the FatesSp compsets are done using user-mods right now. That may change in the future, but adding it explicitly would make it more visible.

Is that sufficient? Or would you like to do a quick chat on this? A longer term discussion on testing philosphy would also be good to do with the group, but that can be later.