LANDIS-II-Foundation / Extension-Base-Harvest

Base Harvest extension for the LANDIS-II landscape change model.
https://sites.google.com/site/landismodel/extensions/base-harvest
Apache License 2.0
0 stars 7 forks source link

Planting recurs at each succession timestep #21

Open brmiranda opened 7 years ago

brmiranda commented 7 years ago

I'm not sure this is definitely a harvest issue, it could be succession, but I haven't dug deep enough to know for sure. Because it only shows up when running harvest, I have posted the issue here.

The issue is that once a prescription implements planting, the planting of the species repeats on the planted sites each time that succession runs. The planting does not depend on the harvest timestep or the end times of the prescription. The issue occurs for both Age-Only and Biomass-Succession and both Base Harvest and Biomass Harvest.

The issue can be seen by running the attached example. The AGE-COUNT output maps show 41 cohorts on each cell at year 50. This is the result of planting occurring each year from 10 to 50. With very low establishment probabilities there should be only 1 cohort on most sites at year 50, which should be the cohort planted in year 10.

BaseHarvest.zip

rmscheller commented 7 years ago

Note: Brian is going to test to see if a succession problem first.

brmiranda commented 7 years ago

Confirmed that the same behavior exists for PnET-Succession with Biomass Harvest.

brmiranda commented 7 years ago

I think what is happening is that a harvest prescription adds species to the planting schedule (Library.Succession). I suspect that the schedule should get cleared at the end of the succession time step, but that is not happening. So probably the problem actually lies in Library.Succession.

brmiranda commented 7 years ago

It looks like the resolution to this issue will be in Library.Succession.Reproduction.Reproduce. There should be a step that removes the planting schedule for the site once it has been processed for planting. A similar action happens for serotiny and resprout: serotiny[site].SetAll(false); resprout[site].SetAll(false);

But planting is not structured the same was as serotiny and resprout, so it's not as simple. I can't find a simple way to do the equivalent for planting.

brmiranda commented 7 years ago

Didn't mean to close the issue.

brmiranda commented 7 years ago

I think I got this one fixed. Edited Landis.Library.Succession.FormOfReproduction to reset selectedSpecies for the site after attempting to plant. Pushed changes in 'Issue_21' branch. Because this fix was applied to the Succession Library, all succession extensions will need to be released with this updated library.

bmarron18 commented 7 years ago

hi Brian -- just started in on first stage of the QAQC for this issue which is replication of the problem. Downloaded your .zip (above) and successfully reproduced your data (AGE-COUNT-*.img) outputs showing the cohort establishment sequence of (1, 1, 11, 21, 31, 41) for year sequence (0, 10, 20, 30, 40, 50) running AgeOnlySuccession plus the prescription for BaseHarvest, which includes, "Plant piceglau".

In attempting to reproduce the problem using the exact same prescription for BaseHarvest but running BiomassSuccession, got data (AGE-COUNT-.img) outputs showing the cohort establishment sequence of (1, 1, 2, 3, 4, 5) for year sequence (0, 10, 20, 30, 40, 50). Likewise, in attempting to reproduce the problem running BiomassSuccession plus the same prescription but for BiomassHarvest, got data (AGE-COUNT-.img) outputs showing the cohort establishment sequence of (1, 1, 2, 3, 4, 5) for year sequence (0, 10, 20, 30, 40, 50). Are these the outputs you would expect? The complete scenario files are attached for you to double check.

The next QAQC step would be to 1) rebuild Library-Succession with the merged changes (https://github.com/bmarron18/Library-Succession/pull/1), 2) rebuild AgeOnly Succession and BiomassSuccession and 3) try the same set of tests. If you confirm the outputs above as correct and indicative of the problem, we'll go to the next QAQC step. Thanks!

BaseHarvest2_test_PLANT.zip

BiomassHarvest1_test_PLANT.zip

brmiranda commented 7 years ago

The results posted above demonstrate confirmation of the issue. The increasing number of cohorts each decade indicates that planting is occurring at each succession time step. Because the establishment probability is set to be very low, we would not expect that uniform increase in cohorts from seeding establishment. The Age Only scenario runs succession at a 1-year time step, which results in 10 cohorts added per decade after planting starts (1, 11, 21, 31, 41). The Biomass Succession scenario is running succession at a 10-year time step, which results in 1 cohort added per decade after planting starts (1, 2, 3, 4, 5).

The sequence of age counts that we expect when this issue is resolved is generally (1, 1, 1, 1, 1, 1) for the majority of cells. A small number of cells will have additional cohorts due to successful seeding (only after 25-years post-planting due to maturity age), but the vast majority of cells should maintain a single cohort.

bmarron18 commented 7 years ago

A first round of QAQC was performed as follows:

  1. Rebuilt Landis.Library.Succession-v2.dll with the changes from BMiranda detailed in https://github.com/bmarron18/Library-Succession/pull/1

  2. Rebuilt and reinstalled Extension-Age-Only-Succession, Extension-Biomass-Succession, and Extension-NECN-Succession using BMiranda's Landis.Library.Succession-v2.dll

  3. Ran three (3) rounds of BMiranda's original scenario test set with different succession extensions and harvest extensions for QAQC; specifically, 1) Age-only Succession + Base Harvest, 2) Biomass Succession + Base Harvest, and 3) Biomass Succession + Biomass Harvest. No QAQC was performed for the NECN succession.

Exceptions were thrown in every case for the QAQC items in 3. above. The attached .zip file contains details of the QAQC process (described in the doc, "PWL78b_QAQC_Planting_20170807") as well as the Landis-log.txt files for each of the three QAQC rounds.

QAQC_Extension-Base-Harvest-Issue21_20170808 .zip

brmiranda commented 7 years ago

Many other extensions and libraries are dependent on the Succession Library, including Harvest-Mgmt-Lib, Site-Harvest-Lib, Base-Harvest extension and Biomass-Harvest extension. If the fixed version of the Succession Library has a different name, then all of the dependent libraries and extensions will also need to be recompiled with the reference to the new version. If the fixed version maintains the same name, then the other dependent code will not need to be recompiled, they will utilize the installed version with the fix. The last tests that were done have 2 different Success Libraries in use, the fixed v2 is being used by the Succession extension, but the original version is still being used by the harvest libraries and extensions.

bmarron18 commented 7 years ago

Ah yes, thank you. This is why the Harvest Suite of extensions and libraries is so tricky -- changes reverberate throughout. I can re-run the same set of QAQC runs but with the new version of Landis.Library.Succession-v2.dll re-named to Landis.Library.Succession.dll and see if everything works as expected. Ultimately though, we will probably need to update all extensions and libraries in the Harvest Suite to reference Landis.Library.Succession-v2.dll because of the changes made. I'll check with Rob but am thinking that the changes here can be included in the v2 name w/o having to go to v2.1.

bmarron18 commented 7 years ago

Re-ran the same set of QAQC runs but with the new version of Landis.Library.Succession-v2.dll re-named to Landis.Library.Succession.dll and threw the same exception, whether running the BiomassHarvest or the BaseHarvest extension:

Running Biomass Harvest ... Internal error occurred within the program: Object reference not set to an instance of an object.

Stack trace: at Landis.Library.Succession.Reproduction.ScheduleForPlanting(SpeciesList speciesToPlant, ActiveSite site) at Landis.Library.HarvestManagement.Prescription.Harvest(Stand stand) at Landis.Library.HarvestManagement.AppliedPrescription.HarvestHighestRankedStand() at Landis.Library.HarvestManagement.ManagementArea.HarvestStands() at Landis.Extension.BiomassHarvest.PlugIn.Run() at Landis.Model.Run(ExtensionMain extension) at Landis.Model.Run(String scenarioPath, IUserInterface ui) at Landis.App.Main(String[] args)

Running Base Harvest ... Internal error occurred within the program: Object reference not set to an instance of an object.

Stack trace: at Landis.Library.Succession.Reproduction.ScheduleForPlanting(SpeciesList speciesToPlant, ActiveSite site) at Landis.Library.HarvestManagement.Prescription.Harvest(Stand stand) at Landis.Library.HarvestManagement.AppliedPrescription.HarvestHighestRankedStand() at Landis.Library.HarvestManagement.ManagementArea.HarvestStands() at Landis.Extension.BaseHarvest.PlugIn.Run() at Landis.Model.Run(ExtensionMain extension) at Landis.Model.Run(String scenarioPath, IUserInterface ui) at Landis.App.Main(String[] args)

brmiranda commented 7 years ago

Uploading examples using Biomass Succession and PnET-Succession along with Biomass Harvest. BiomassSuccession.zip PnETSuccession.zip

bmarron18 commented 7 years ago

QAQC Summary 1: Consistency Checks

  1. Rebuilt Landis.Library.Succession-v2.dll with changes from B.Miranda as detailed in https://github.com/bmarron18/Library-Succession/pull/1

  2. Rebuilt the entire Harvest Suite of libraries because there are direct or indirect calls to Landis.Library.Succession-v2.dll; namely,

Landis.Library.SiteHarvest-v1.dll << direct Landis.Library.HarvestManagement-v2.dll << direct Landis.Library.BiomassHarvest-v2.dll << indirect thru SiteHarvest
Landis.Extension.BaseHarvest.dll << direct and indirect thru SiteHarvest Landis.Extension.BiomassHarvest.dll << direct and indirect thru SiteHarvest

3 . All rebuilds for 2. above were carefully sequenced and built on 'tmp1' branches

  1. Rebuilt and reinstalled Extension-Age-Only-Succession, Extension-Biomass-Succession, and Extension-NECN-Succession with B.Miranda's Landis.Library.Succession-v2.dll and the freshly-built Harvest Suite libraries in 2. above

  2. Ran four (4) rounds of B.Miranda's scenario test set with different succession extensions and harvest extensions; specifically, a) QAQC2_TestScenario_Issue21_BaseHarvest1 (Age-only Succession + Base Harvest) b) QAQC2_TestScenario_Issue21_BaseHarvest2 (Biomass Succession + Base Harvest) c) QAQC2_TestScenario_Issue21_BiomassHarvest1 (Biomass Succession + Biomass Harvest) d) QAQC2_TestScenario_Issue21_BiomassHarvest2 (NECN Succession + Biomass Harvest)

  3. No exceptions were thrown and every QAQC scenario produced identical counts of "HarvestedSites" and "CohortsHarvested_piceglau"

  4. Passed QAQC!

PWL78d_QAQC_LibrarySuccession20170910.txt

PWL77_CompleteRebuild_HarvestSuite_20170910.txt

bmarron18 commented 7 years ago

QAQC Summary 2 : Issue Resolution

1. In addition to consistency checks (PWL78d_QAQC_Planting_20170911), a check of 
    AGE-COUNT-*.img outputs was performed.

2.  The four (4) QAQC rounds below were re-run with "Output Cohort Statistics" active to obtain
   AGE-COUNT-*.img outputs
    a) QAQC2_TestScenario_Issue21_BaseHarvest1 (Age-only Succession + Base Harvest) 
    b) QAQC2_TestScenario_Issue21_BaseHarvest2 (Biomass Succession + Base Harvest)
    c) QAQC2_TestScenario_Issue21_BiomassHarvest1 (Biomass Succession + Biomass Harvest)
    d) QAQC2_TestScenario_Issue21_BiomassHarvest2 (NECN Succession + Biomass Harvest)

3. In every case, the issue was resolved per B. Miranda's resolution criterion:
   "The sequence of age counts that we expect when this issue is resolved 
   is generally (1, 1, 1, 1, 1, 1) for the majority of cells. A small number of cells will 
   have additional cohorts due to successful seeding (only after 25-years post-planting due to 
   maturity age), but the vast majority of cells should maintain a single cohort."

   4. QAQC passed!

PWL78e_QAQC_LibrarySuccession_20170912.txt

R_img_evaluation .txt

QAQC_TestRuns.zip

rmscheller commented 7 years ago

Great! Next I'll run this by the Technical Committee.

rmscheller commented 6 years ago

The program no longer passes Bruce's QAQC.