[Land Use] C-based wood harvest #888

sshu88 · 2022-08-02T21:48:08Z

Added C-based harvest as another option (wood_harvest_units = 2) in FATES.

Description:

See issue #869

Corresponding E3SM PR: E3SM-Project/E3SM#5106

Collaborators:

@jenniferholm @aldivi @ckoven @rgknox @glemieux

Expectation of Answer Changes:

Checklist:

My change requires a change to the documentation.
I have updated the in-code documentation .AND. (the technical note .OR. the wiki) accordingly.
[ X ] 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:

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

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

FATES baseline hash-tag: FATES - aaa1062

Test Output:

See issue #869

ckoven

Thanks @sshu88 for this, I think it is a great addition! I have a bunch of comments and questions, mostly I am trying to wrap my head around these changes.

main/FatesHistoryInterfaceMod.F90

ckoven · 2022-09-22T22:08:42Z

biogeochem/EDLoggingMortalityMod.F90

+   ! ! transfer factor from kg biomass (dry matter) to kg carbon
+   ! ! now we applied a simple fraction of 50% based on the IPCC
+   ! ! guideline
+   ! real(r8), parameter :: carbon_per_kg_biomass = 0.5_r8


delete this lines since they are no longer used?

Will remove.

ckoven · 2022-09-22T22:10:58Z

biogeochem/EDLoggingMortalityMod.F90

+               harvestable_forest_c, available_forest_c
+            end if
+            !write(fates_log(),*) 'HLM harvest carbon data not implemented yet. Exiting.'
+            !call endrun(msg=errMsg(sourcefile, __LINE__))


delete above two lines since it is now working?

ckoven · 2022-10-05T21:31:47Z

biogeochem/EDLoggingMortalityMod.F90

+     harvest_rate = 0._r8
+     harvest_rate_c = 0._r8
+     harvest_rate_supply = 0._r8
+     harvest_tag = 2


Suggested change

harvest_tag = 2

harvest_tag(:) = 2

The initial value for this tag shall be 1 since we removed the logics to account for available_forest_c. Will change.

biogeochem/EDLoggingMortalityMod.F90

ckoven · 2022-10-26T23:15:54Z

biogeochem/EDLoggingMortalityMod.F90

+      real(r8) :: harvest_rate_c    ! Temporary variable
+      real(r8) :: harvest_rate_supply  ! Temporary variable


what are the units of the above two variables?

kgC site-1. Will add.

ckoven · 2022-10-26T23:16:34Z

biogeochem/EDLoggingMortalityMod.F90

+      real(r8), intent(in) :: harvestable_forest_c(:)  ! site level forest c matching criteria available for harvest
+      real(r8), intent(out) :: harvest_rate


what are units for these two variables? one is a carbon stock and one is an areal rate of harvest?

Correct. Will add.

ckoven · 2022-10-26T23:21:59Z

biogeochem/EDLoggingMortalityMod.F90

+     do h_index = 1,hlm_num_lu_harvest_cats
+        if (patch_anthro_disturbance_label .eq. primaryforest) then
+           if(hlm_harvest_catnames(h_index) .eq. "HARVEST_VH1") then! .or. &
+           !     hlm_harvest_catnames(h_index) .eq. "HARVEST_VH2") then


again curious why not using HARVEST_VH2 here?

ckoven · 2022-10-26T23:27:48Z

biogeochem/EDLoggingMortalityMod.F90

+     do h_index = 1,hlm_num_lu_harvest_cats
+        if (patch_anthro_disturbance_label .eq. primaryforest) then
+           if(hlm_harvest_catnames(h_index) .eq. "HARVEST_VH1") then! .or. &
+           !     hlm_harvest_catnames(h_index) .eq. "HARVEST_VH2") then
+              harvest_rate_c = harvest_rate_c + hlm_harvest_rates(h_index)
+              ! Determine the total supply of available C for harvest
+              if(harvestable_forest_c(h_index) >= harvest_rate_c) then
+                 harvest_rate_supply = harvest_rate_supply + harvestable_forest_c(h_index)
+                 harvest_tag(h_index) = 0
+              else
+                 harvest_tag(h_index) = 1
+              end if
+           endif
+        else if (patch_anthro_disturbance_label .eq. secondaryforest .and. &
+             secondary_age >= secondary_age_threshold) then
+           if(hlm_harvest_catnames(h_index) .eq. "HARVEST_SH1") then
+              harvest_rate_c = harvest_rate_c + hlm_harvest_rates(h_index)
+              if(harvestable_forest_c(h_index) >= harvest_rate_c) then
+                 harvest_rate_supply = harvest_rate_supply + harvestable_forest_c(h_index)
+                 harvest_tag(h_index) = 0
+              else
+                 harvest_tag(h_index) = 1
+              end if
+           endif
+        else if (patch_anthro_disturbance_label .eq. secondaryforest .and. &
+             secondary_age < secondary_age_threshold) then
+           if(hlm_harvest_catnames(h_index) .eq. "HARVEST_SH2") then! .or. &
+           !     hlm_harvest_catnames(h_index) .eq. "HARVEST_SH3") then
+              harvest_rate_c = harvest_rate_c + hlm_harvest_rates(h_index)
+              if(harvestable_forest_c(h_index) >= harvest_rate_c) then
+                 harvest_rate_supply = harvest_rate_supply + harvestable_forest_c(h_index)
+                 harvest_tag(h_index) = 0
+              else
+                 harvest_tag(h_index) = 1
+              end if
+           endif
+        endif
+     end do


I'm not sure I understand the logic here of adding each category ofhlm_harvest_rates(:) to the harvest_rate_c as you go through the loop, it seems like it should be only using the hlm_harvest_rates(:) indexed for each category within each patch type comparison? I'm also unclear about the time normalization between hlm_harvest_rates(:) which is a rate and harvestable_forest_c(:) which is a stock? Is there meant to be an implicit annualization in that comparison? If so, should it be made explicit?

Yes for now. But since I will include non-forest harvest into the harvest rate, we now need these 3 categories to be separated.

hlm_harvest_rates is in the unit of kgC site-1 yr-1 and harvestable_forest_c is in the unit of kgC site -1. Thus the calculation here will get an annual harvest_rate in the unit of area fraction yr-1, by assuming the carbon fraction to be equivalent to the area fraction. I will add comments on these variables.

ckoven · 2022-10-26T23:32:55Z

biogeochem/EDLoggingMortalityMod.F90

+     !  under two different scenarios:
+     !  harvestable carbon: aggregate all cohorts matching the dbhmin harvest criteria
+     !  available carbon: aggregate all cohorts


Is this 'available' vs 'harvestable' carbon logic still used? I think it isn't so suggest changing the description.

Suggested change

! under two different scenarios:

! harvestable carbon: aggregate all cohorts matching the dbhmin harvest criteria

! available carbon: aggregate all cohorts

! according to the the dbhmin harvest criteria

Yeah. I forgot changing some of the comment here. Thank you for double checking.

…commit

sshu88 · 2022-11-10T00:51:59Z

Hi @glemieux, I found a lot of differences in EDPatchDynamicsMod.F90 when merging FATES_ELM API 24 into my version. Just want to double check if you have already done the merge in the current version. If so I shall change that file back and only add my revision.

glemieux · 2022-11-10T17:02:20Z

Thanks for checking @sshu88. I haven't merged in the latest fates master commits into this branch. Please feel free to push the merge with deconflicts to your branch or let me know if you'd like support with deconflicting the merge.

sshu88 · 2022-11-11T22:41:31Z

Hi @glemieux. I did a manual vimdiff check and found the differences are actually caused by a series changes of the indents from loop and if statements. I will return back to the previous version of EDPatchDynamics.F90 and manually add my revision to avoid these confusions.

glemieux · 2022-12-01T00:51:16Z

Hi @glemieux. I did a manual vimdiff check and found the differences are actually caused by a series changes of the indents from loop and if statements. I will return back to the previous version of EDPatchDynamics.F90 and manually add my revision to avoid these confusions.

Apologies for the delayed response @sshu88. Just to clarify: would it be helpful for me to help deconflict your branch?

rgknox · 2022-12-05T17:17:49Z

@sshu88 is EDLoggingMortalityMod.F90 still an appropriate name for this file in your opinion? Is this file handling a broader number of things than identifying mortality rates and mass fluxes from logging?

sshu88 · 2022-12-05T17:29:28Z

@sshu88 is EDLoggingMortalityMod.F90 still an appropriate name for this file in your opinion? Is this file handling a broader number of things than identifying mortality rates and mass fluxes from logging?

New subroutines transferred the harvest rate from carbon amount to disturbance rate, so I think the file name is still appropriate.

biogeochem/EDPatchDynamicsMod.F90

rgknox · 2022-12-05T17:52:10Z

parteh/PRTGenericMod.F90

-       deallocate(this%variables(i_var)%burned)
-       deallocate(this%variables(i_var)%damaged)
-    end do
+    if(allocated(this%variables)) then


@sshu88 , did you encounter problems without this check? All cohorts should have a PRT allocation, if there is not, than there is a bigger problem in our logic flow.

Only once on Cori when running global version. I added this check and model passed. But the model branch I used on other machine (e.g., lawrencium) does not have this check and no issue is found.

glemieux · 2022-12-08T17:54:24Z

I ran the ctsm-based fates suite of tests with the current version of the code against the sci.1.60.3_api.24.2.0 tag and all expected tests pass. This was primarily to make sure that nothing is accidentally breaking or answer changing from this PR (which is not expected). Only differences are due to new history variables from this PR. Folder location on cheyenne: /glade/u/home/glemieux/scratch/ctsm-tests/tests_pr888-paramfile

wwieder · 2022-12-08T19:39:47Z

Thanks for doing this testing, @glemieux. What do we need to do on the CTSM side to support this PR? Is there a parallel issue / PR than needs to be made for CTSM?

glemieux

Change needed to bring get_harvestable_carbon up to date with API25.

biogeochem/EDLoggingMortalityMod.F90

all_carbon to carbon12

glemieux

I have a few minor questions.

biogeochem/EDPatchDynamicsMod.F90

main/FatesHistoryInterfaceMod.F90

glemieux · 2023-01-31T22:09:47Z

I tested this against fates-sci.1.62.0_api.25.0.0-ctsm5.1.dev116 and aside from expected failures, this is mostly b4b aside from fieldlist differences. There is one difference due to the dev116 tag, but this is actually in the coupler output only and expected. Additionally, #701 seems to have cropped back up for the ERS hydro test. @adrifoster is going to mark that as expected failure in dev117.

This pull request contains modification in E3SM corresponding to another FATES NGEET/fates#888. In detail, the necessary input to enable FATES C-based harvest from ELM is passed to FATES through bc_in. Output from FATES used to calculate land use emission and net biosphere productivity is passed to ELM through bc_out. [non-B4B] (FATES tests only)

sshu3 added 6 commits March 7, 2022 21:12

Added C based logging in FATES. Passed validation.

420431f

Updated minor corrections from fatesluc branch.

6725822

Updated changes from parent branch "fatesluc".

d5bd304

Merge branch 'fatesluc_cbasedhrv' into HEAD

aaa1062

Added historical outputs of secondary patches.

14c3002

Pass NPP and AR to ELM for calculating NBP.

67b9211

sshu88 mentioned this pull request Aug 2, 2022

C-based harvest in ELM-FATES E3SM-Project/E3SM#5106

Closed

glemieux added the PR status: Not Ready The author is signaling that this PR is a work in progress and not ready for integration. label Aug 2, 2022

glemieux removed the PR status: Not Ready The author is signaling that this PR is a work in progress and not ready for integration. label Sep 19, 2022

sshu3 and others added 2 commits September 22, 2022 18:17

Removed hlm_harvest_bypass_criteria. Applied fix on global restart run.

ce81248

Merge branch 'master' into fatesluc_cbasedhrv_v2_merge

7827534

glemieux requested a review from ckoven October 10, 2022 16:47

Remove "available_forest_c" and simplify the logic.

1fb5220

ckoven reviewed Oct 26, 2022

View reviewed changes

sshu3 added 7 commits October 28, 2022 14:28

Minor revisions on comments, temporary commit

ad337ea

Minor revisions on comments and merged with master branch, temporary …

81ebf32

…commit

Patch up paralell disturbance and model phenology issue.

91d4576

Patch up some errors when merging with FATES API 24.

bed5e40

Ignore small logging rate which would produce tiny secondary patch.

422723e

Clarify calculations and comments on the harvest debt.

fec6597

Merge branch 'fatesluc_nocomp' into fatesluc_cbasedhrv_v2

fe6b0ab

sshu3 added 2 commits November 11, 2022 15:01

Change back EDPatchDynamicsMod.F90 for sanity purpose.

b62fe84

Ignore small logging disturbance.

b35025e

rgknox reviewed Dec 5, 2022

View reviewed changes

biogeochem/EDPatchDynamicsMod.F90 Show resolved Hide resolved

rgknox reviewed Dec 5, 2022

View reviewed changes

ckoven mentioned this pull request Dec 20, 2022

Should we add history variable dimensions for land-use-category and land-use-category x pft? #964

Open

sshu3 and others added 2 commits December 20, 2022 13:30

Create subroutine to encapsulate the calculation of harvest debt.

23ee93f

Merge branch 'main' into fatesluc_cbasedhrv_v2

f597756

glemieux requested changes Jan 5, 2023

View reviewed changes

biogeochem/EDLoggingMortalityMod.F90 Outdated Show resolved Hide resolved

rgknox and others added 2 commits January 6, 2023 16:36

all_carbon to carbon12

eb609b5

Merge pull request #3 from rgknox/fatesluc_cbasedhrv_v2-api25

56006f8

all_carbon to carbon12

glemieux reviewed Jan 18, 2023

View reviewed changes

biogeochem/EDPatchDynamicsMod.F90 Outdated Show resolved Hide resolved

main/FatesHistoryInterfaceMod.F90 Outdated Show resolved Hide resolved

Update EDPatchDynamicsMod.F90

f1f855d

glemieux mentioned this pull request Jan 21, 2023

tveg causing errors in global nocomp runs #973

Closed

glemieux added 3 commits January 26, 2023 10:29

Merge branch 'main' into fatesluc_cbasedhrv_v2

e6772ad

manually reinstate tveg history variable

2504716

add local variable initialization for get_harvest_debt

d80ef20

glemieux approved these changes Jan 31, 2023

View reviewed changes

glemieux merged commit 7775358 into NGEET:main Jan 31, 2023

This was referenced Jan 31, 2023

Update fates submodule pointer to carbon harvest compatible tag sshu88/E3SM#6

Merged

Enable C-based harvest in ELM-FATES and update to FATES API 25 E3SM-Project/E3SM#5429

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Land Use] C-based wood harvest #888

[Land Use] C-based wood harvest #888

sshu88 commented Aug 2, 2022 •

edited by glemieux

Loading

ckoven left a comment

ckoven Sep 22, 2022

sshu88 Oct 26, 2022

ckoven Sep 22, 2022

sshu88 Oct 26, 2022

ckoven Oct 5, 2022

sshu88 Oct 26, 2022 •

edited

Loading

ckoven Oct 26, 2022

sshu88 Oct 27, 2022

ckoven Oct 26, 2022

sshu88 Oct 27, 2022

ckoven Oct 26, 2022

ckoven Oct 26, 2022 •

edited

Loading

sshu88 Oct 27, 2022

ckoven Oct 26, 2022

sshu88 Oct 27, 2022

sshu88 commented Nov 10, 2022

glemieux commented Nov 10, 2022

sshu88 commented Nov 11, 2022 •

edited

Loading

glemieux commented Dec 1, 2022

rgknox commented Dec 5, 2022

sshu88 commented Dec 5, 2022 •

edited

Loading

rgknox Dec 5, 2022

sshu88 Dec 5, 2022

glemieux commented Dec 8, 2022 •

edited

Loading

wwieder commented Dec 8, 2022

glemieux left a comment

glemieux left a comment

glemieux commented Jan 31, 2023

		real(r8) :: harvest_rate_c ! Temporary variable
		real(r8) :: harvest_rate_supply ! Temporary variable

		real(r8), intent(in) :: harvestable_forest_c(:) ! site level forest c matching criteria available for harvest
		real(r8), intent(out) :: harvest_rate

[Land Use] C-based wood harvest #888

[Land Use] C-based wood harvest #888

Conversation

sshu88 commented Aug 2, 2022 • edited by glemieux Loading

Description:

Collaborators:

Expectation of Answer Changes:

Checklist:

Test Results:

ckoven left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sshu88 Oct 26, 2022 • edited Loading

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

Choose a reason for hiding this comment

ckoven Oct 26, 2022 • edited Loading

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

sshu88 commented Nov 10, 2022

glemieux commented Nov 10, 2022

sshu88 commented Nov 11, 2022 • edited Loading

glemieux commented Dec 1, 2022

rgknox commented Dec 5, 2022

sshu88 commented Dec 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glemieux commented Dec 8, 2022 • edited Loading

wwieder commented Dec 8, 2022

glemieux left a comment

Choose a reason for hiding this comment

glemieux left a comment

Choose a reason for hiding this comment

glemieux commented Jan 31, 2023

sshu88 commented Aug 2, 2022 •

edited by glemieux

Loading

sshu88 Oct 26, 2022 •

edited

Loading

ckoven Oct 26, 2022 •

edited

Loading

sshu88 commented Nov 11, 2022 •

edited

Loading

sshu88 commented Dec 5, 2022 •

edited

Loading

glemieux commented Dec 8, 2022 •

edited

Loading