allow disturbance in nocomp mode #833

ckoven · 2022-02-07T17:47:32Z

Currently, disturbance processes are turned off while in nocomp mode. This PR includes changes that enables the tracking of patch-level nocomp PFT labels during patch creation and fusion processes, which allows for turning on disturbance when nocomp mode is active.

Note that this still does not allow for disturbance in SP mode, because that wouldn't really make sense. But it does enable it in non-SP nocomp modes.

This should not require any changes to the interface or parameter file.

Description:

To do this, a few things needed to be done:

Removed hard-coded switches that turn off disturbance when nocomp was active
Added logic during patch-generating disturbance so that it loops over nocomp PFTs and does the entire disturbance logic separately for each nocomp PFT, so that each newly-disturbed patch only draws from older patches with the same nocomp PFT identity.
Added logic to force the preservation of nocomp PFT identity during patch fusion.

Since the disturbance and in particular patch fusion code already has a lot of nested if and do blocks, I also went through fairly liberally and added names to a lot of these blocks. I had to do this to be able to figure out where to insert the code in the right places. Hopefully this adds a greater degree of readibility to those codes. I have not (so far) gone through and re-indented the code, as that will generate a ton of whitespace changes.

Collaborators:

Some discussion with @rosiealice and @rgknox

Expectation of Answer Changes:

should be bit for bit in all non-nocomp and SP configurations. Should be answer changing in non-SP nocomp configurations.

Checklist:

My change requires a change to the documentation.
I have updated the in-code documentation .AND. (the technical note .OR. the wiki) accordingly.
I have read the CONTRIBUTING document.
FATES PASS/FAIL regression tests were run
If answers were expected to change, evaluation was performed and provided

Test Results:

not yet tested.

CTSM (or) E3SM (specify which) test hash-tag:

CTSM (or) E3SM (specify which) baseline hash-tag:

FATES baseline hash-tag:

Test Output:

…s adds up to 1-pftpatcharea

rosiealice · 2022-02-07T20:40:04Z

biogeochem/EDPatchDynamicsMod.F90

    do while(associated(currentPatch))

+       cp_nocomp_matches_1: if ( hlm_use_nocomp .eq. ifalse .or. &


where the do loops have labels they include the word 'loop' in them, so I feel like the if statement labels might be easier for people to parse if they have 'if' in their name.

Yeah I wasn't very consistent in how I did this. It would probably be easier to read if each label does have an 'if' or 'loop' or something like that in it. So will edit to try to be more consistent.

rosiealice · 2022-02-07T20:41:33Z

biogeochem/EDPatchDynamicsMod.F90


                         !-----------------------------------------------------------------------------------
                         ! check to see if both patches are older than the age at which we force them to fuse
                         !-----------------------------------------------------------------------------------

-                         if ( tpp%age .le. max_age_of_second_oldest_patch .or. &
+                         maxage_if: if ( tpp%age .le. max_age_of_second_oldest_patch .or. &


This one being an example of an 'if' label having an 'if' in it...

rosiealice · 2022-02-07T20:45:04Z

biogeochem/EDPatchDynamicsMod.F90

-                         if(hlm_use_nocomp.eq.itrue.and. &
-                            tpp%nocomp_pft_label.ne.currentPatch%nocomp_pft_label)then
-                            fuse_flag = 0 
-                         end if
                         !-------------------------------------------------------------------------!


So this is deleted because it's redundant with the code on line 2766?

@rosiealice yes that's right

glemieux · 2022-02-11T06:46:45Z

I ran the fates suite tests for this with comparison against the baseline on Cheyenne. There's an issue with the nocomp testmod, although all other expected tests pass b4b against the baseline. The nocomp testmod is a 30 day test and should finish up in a couple of minutes, but it's hitting the 40 min wall clock limit. I've resubmitted it to double check, but as of this writing the run is a 14 mins so it's probably a real issue. Looking at the lnd.log I'm seeing the following:

2359  clm: calling FATES model            1
2360 FATES Dynamics: 2000-01-01
2361  problem with spitfire fuel averaging
2362  problem with spitfire fuel averaging
2363  problem with spitfire fuel averaging
2364  problem with spitfire fuel averaging
2365  problem with spitfire fuel averaging
2366  problem with spitfire fuel averaging
2367  problem with spitfire fuel averaging
2368  problem with spitfire fuel averaging
2369  problem with spitfire fuel averaging

I've got another run for this test going with debug modes on (i.e. both write_SF and debug) to see what additional information we can get for diagnosis.

ckoven · 2022-02-11T15:31:59Z

Thanks @glemieux. Sounds like its probably getting caught in an infinite loop, either in fuse_patches or terminate_patches. Will investigate.

glemieux · 2022-02-11T17:34:26Z

The debug mode test location on Cheyenne:
/glade/u/home/glemieux/scratch/ctsm-tests/tests_nocomp_disturb_spitfiredebug

… catch them in other cases

glemieux · 2022-02-11T20:51:31Z

The fix in b705816 allowed the nocomp testmod to pass:
/glade/u/home/glemieux/scratch/ctsm-tests/tests_nocomp_disturb_spitfiredebug1

Rerunning the fates suite with baseline comparison.

glemieux · 2022-02-11T22:56:14Z

All expected tests pass b4b against baseline now. The NoComp test mod is resulting in DIFFs as expected:
/glade/u/home/glemieux/scratch/ctsm-tests/tests_pr833-nocompdisturb1

rgknox · 2022-02-16T18:41:35Z

biogeochem/EDPatchDynamicsMod.F90

@@ -2282,6 +2304,19 @@ subroutine fuse_patches( csite, bc_in )
    primary_land_fraction_afterfusion = 0._r8

    nopatches(1:n_anthro_disturbance_categories) = 0
+


Also, shouldn't this be the actual number of unique pfts on this site? And if so, would we change this to not have the hlm_use_fixed_biogeog part of the logic.

If it not the actual number of pfts on the site, and instead is the number defined by the parameter file, then we should move this logic to FatesInterfaceMod, perform this evaluation once, and then remove the "parameter" designation of maxPatchesPerSite_by_disttype, and update maxPatchesPerSite_by_disttype.

biogeochem/EDPatchDynamicsMod.F90

rgknox · 2022-02-16T18:46:28Z

biogeochem/EDPatchDynamicsMod.F90

             tpp => currentSite%youngest_patch
-             do while(associated(tpp))
+             tpp_loop: do while(associated(tpp))


love all these labels

rgknox · 2022-02-16T18:47:56Z

biogeochem/EDPatchDynamicsMod.F90

                        currentPatch%anthro_disturbance_label .eq. i_disttype) then

+                    nocomp_pft_labels_match_if: if (hlm_use_nocomp .eq. ifalse .or. &


makes sense

biogeochem/EDPatchDynamicsMod.F90

rgknox

This PR looks good, see questions in line.

glemieux · 2022-02-18T19:29:31Z

Retest after updates per b5f2c45 results in all expected tests passing B4B against the baseline, with the expected exception of the NoComp testmod DIFFs:
/glade/u/home/glemieux/scratch/ctsm-tests/tests_pr833-nocompdisturb2

rgknox · 2022-02-28T18:41:01Z

all tests nominal, expecting the following BASELINE fail: ERS_Ld30.f45_f45_mg37.I2000Clm50FatesCruRsGs.cheyenne_intel.clm-FatesColdDefReducedComplexNoComp
/glade/scratch/rgknox/tests_0228-104151ch

ckoven added 6 commits February 2, 2022 13:26

removed logic that prevented patch dynamics when nocomp was running

5487d7c

added history vars that only apply to nocomp case

bd707e4

added bare-ground-patch diagnostic for nocomp mode

a128271

bugfix on prior

8cf15ba

updated PatchynamicsMod to allow for nocomp with disturbance

fea5658

deleted nocomp bare-ground history var after confirming that it alway…

8c0cb2c

…s adds up to 1-pftpatcharea

rosiealice reviewed Feb 7, 2022

View reviewed changes

ckoven added 2 commits February 7, 2022 17:12

tried to be more consistent in naming of logic and loop blocks

439379f

added nocomp-pft-resolved burn area output

882eeda

added logic to prevent infiite loops in nocomp w/o fixed_biogeog, and…

b705816

… catch them in other cases

glemieux self-assigned this Feb 11, 2022

rgknox reviewed Feb 16, 2022

View reviewed changes

biogeochem/EDPatchDynamicsMod.F90 Show resolved Hide resolved

rgknox reviewed Feb 16, 2022

View reviewed changes

biogeochem/EDPatchDynamicsMod.F90 Show resolved Hide resolved

rgknox requested changes Feb 16, 2022

View reviewed changes

updated logic slightly and added some documentation

b5f2c45

rgknox approved these changes Feb 18, 2022

View reviewed changes

rgknox assigned rgknox and unassigned glemieux Feb 22, 2022

rgknox merged commit 7761d72 into NGEET:master Feb 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

allow disturbance in nocomp mode #833

allow disturbance in nocomp mode #833

ckoven commented Feb 7, 2022

rosiealice Feb 7, 2022

ckoven Feb 7, 2022

rosiealice Feb 7, 2022

rosiealice Feb 7, 2022

ckoven Feb 7, 2022

glemieux commented Feb 11, 2022 •

edited

Loading

ckoven commented Feb 11, 2022

glemieux commented Feb 11, 2022

glemieux commented Feb 11, 2022

glemieux commented Feb 11, 2022

rgknox Feb 16, 2022

rgknox Feb 16, 2022

rgknox Feb 16, 2022

rgknox left a comment

glemieux commented Feb 18, 2022

rgknox commented Feb 28, 2022

		do while(associated(currentPatch))

		cp_nocomp_matches_1: if ( hlm_use_nocomp .eq. ifalse .or. &

		@@ -2282,6 +2304,19 @@ subroutine fuse_patches( csite, bc_in )
		primary_land_fraction_afterfusion = 0._r8

		nopatches(1:n_anthro_disturbance_categories) = 0

		currentPatch%anthro_disturbance_label .eq. i_disttype) then

		nocomp_pft_labels_match_if: if (hlm_use_nocomp .eq. ifalse .or. &

allow disturbance in nocomp mode #833

allow disturbance in nocomp mode #833

Conversation

ckoven commented Feb 7, 2022

Description:

Collaborators:

Expectation of Answer Changes:

Checklist:

Test Results:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glemieux commented Feb 11, 2022 • edited Loading

ckoven commented Feb 11, 2022

glemieux commented Feb 11, 2022

glemieux commented Feb 11, 2022

glemieux commented Feb 11, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rgknox left a comment

Choose a reason for hiding this comment

glemieux commented Feb 18, 2022

rgknox commented Feb 28, 2022

glemieux commented Feb 11, 2022 •

edited

Loading