Fix FATES branch runs #2955
Fix FATES branch runs #2955
Conversation
samsrabin
added
bug
samsrabin
added
the
next
|
Two files are getting produced in each run, but only the The The entire @ekluzek any idea why this might be happening? |
|
Figured it out— |
This ensures matching filenames in both parts of ERI tests.
|
@rgknox has new ERI tests that are coming in, but they maybe on the FATES side (not aux_clm). |
samsrabin
added
PR status: awaiting review
samsrabin
added
PR status: ready
| @@ -738,7 +738,7 @@ subroutine initialize2(ni,nj, currtime) | |||
| deallocate(wt_nat_patch) | |||
|
|
|||
| ! Initialise the fates model state structure | |||
| if ( use_fates .and. .not.is_restart() .and. finidat == ' ') then | |||
| if ( use_fates .and. .not. (is_restart() .or. nsrest .eq. nsrBranch) .and. finidat == ' ') then | |||
There was a problem hiding this comment.
@samsrabin I was expecting the top part of this logical block to also exclude hybrid runs along with branch runs. Is a hybrid run equivalent to is_restart()=.true. ?
For future work I feel like we should clean some of this logic up, I cobbled a lot of this together many years ago when I did not have a good sense of restart options. But my take is that our decision making on how to restart fates (ie cold or from pre-existing vegetation structure in a restart file) really comes down to if it is a startup, branch or hybrid. The finidat string is less straight forward to use here in decide what part of the initialization sequence to use. My understanding is that the finidat should have a meaningful file path only in the latter two scenarios (branch and hybrid), so its better to use the switches that are the root of deciding these things rather variables like finidat that are "downstream" of them. Does that make sense? Again, I'd like to make an issue of this and flag it for future work and not complicate this PR.
There was a problem hiding this comment.
I do know that is_restart() returns true in hybrid runs, but I don't know if they're fully equivalent. As for the rest of it, since you think that work makes sense to do somewhere else, would you mind making an issue with your thoughts and then resolving this conversation?
samsrabin
added
next
Description of changes
In FATES branch runs, prevents attempts to re-allocate arrays that already are allocated (in the restart file).
This prevents run failures. However, it exposes a failure in the
COMPARE_base_hybridstep of ERI tests, due to a badhist_mfilt. This PR will also fix that.Remaining tasks:
COMPARE_base_hybridfailure.ERItests toaux_clmandfatestest suites.Notes for testing
Baseline comparisons may fail for the following testmods:
FatesFatesColdAllVarsFatesColdDryDepSatPhenFatesColdHydroFatesColdMeganSatPhenFatesColdSatPhenfire_emisglcMEC_longHowever, this is only because I have changed
hist_mfiltfor those to 1. (From 365 for theFatestestmods; from 30 forfire_emis; from 12 forglcMEC_long). This ensures that the dates in the history filenames are the same for both parts of the ERI test.When this is next in the merge queue:
mfilt. Runaux_clmandfatessuites, comparing to the latest baseline, expecting no diffs.aux_clmandfatessuites, comparing to the latest baseline, expecting baseline comparison failures (a) only for those tests and (b) only because of missing baseline files.Specific notes
Contributors other than yourself, if any: @ckoven
CTSM Issues Fixed:
Are answers expected to change (and if so in what way)? No, although see "Notes for testing" above.
Any User Interface Changes (namelist or namelist defaults changes)? No
Does this create a need to change or add documentation? Did you do so? No
Testing performed, if any:
ERI_Ld60.f45_f45_mg37.I2000Clm50FatesCruRsGs.derecho_intel.clm-Fatespasses.