search

LANDIS-II-Foundation / Extension-Output-Biomass-PnET

https://landis-ii-foundation.github.io/Extension-Output-Biomass-PnET/
Apache License 2.0
1 stars 5 forks source link

Rasters of foliage biomass for all species do not correspond to the sum of the same rasters for each individual species #11

Open Klemet opened 2 months ago

Klemet commented 2 months ago

Hello there !

I've had another issue running the same test scenario as provided in #10 .

The foliageBiomass-{timestep}.img rasters in the AllSpecies folder should (in my understanding) contain the sum of the same rasters but for each individual species.

However, there are time steps (e.g. time step 100) where the AllSpecies raster shows a lot of cells with 0, while the other rasters show that individual species have foliage biomass in the same cell.

I tried to find where the problem came from in the code, but to no avail. I'm wondering is some species are not ignored, or if some values are not truncated somewhere (as in #10 ).

rmscheller commented 2 months ago

Clement, I don't have any familiarity or responsibility for this extension. Note though that Eric Gustafson, who does, does not automatically receive this messages as he is not a registered GitHub user. In other words, be sure to also email him. Cheers, R

Klemet commented 2 months ago

Thanks for the warning, Robert ! I'll email him ASAP.

Klemet commented 2 months ago

I'll just add the evidence I have of the issue, as I forgot to do it : this .zip folder (FoliageBiomassRasters.zip) contains the foliage biomass rasters for all species made by the extension (AllSpecies folder), and the rasters that I calculated by summing the foliage biomass raster for each individual species at each time step ( foliageBiomassAllSpecies_RedoneManually folder).

The Python code I used to make the sum is in the following file : testingFoliageBiomassRasters_githubIssue.txt

Klemet commented 1 month ago

Changing the title of the issue, as I've discovered the same is happenning for LAI rasters. It shows that the issue also tend to exagerate differences between ecoregions and make outlier values in the raster.

Here is the LAI raster for all species (output\biomass\AllSpecies\LAI-100.img) originally made by Output PnET with the test files provided in #10 at t = 100 :

image

We can easily notice a difference in values with the ecoregion 2 that is on a band left of the landscape (values are higher there), and 5 outlier pixels with very high values.

Now, the same raster, but made by summing the individual LAI raster for each species through a Python script :

image

No more differences due to the ecoregions, and no more outlier pixels.

Again, I have no idea where the problem is coming from in the code.

Klemet commented 1 month ago

Welp, after re-testing, it looks like the problem has disappeared 😳. I have no idea what went on. Maybe a problem when writing the rasters on my side ?

I will re-open the issue if this comes back again; in the meanwhile, I deeply apologize for any waste of time !

brmiranda commented 1 month ago

I spent a few minutes looking at this, and I actually do see a discrepancy in the code. Not for LAI, but for foliage biomass. For foliage, it uses a different cohort attribute when calculating individual species foliage biomass [MaxFoliageYearPerSpecies] than it does when calculating AllSpecies foliage biomass [FoliageSum]. FoliageSum is cohort.Fol * cohort.CanopyLayerProp, which means it is using the current Fol value, which for deciduous species will generally be 0 because this is evaluated at the end of the year (December). MaxFoliageYearPerSpecies is actually the maximum value for any cohort of the species, not the sum of the foliage across cohorts of the species. Additionally, it’s using the maximum foliage value for any month of the year, not the end of year value. And, it is not adjusting for canopy proportion. So in this sequence, I see a number of flaws that makes these foliage biomass maps inconsistent with what they are intended to represent. I can suggest a solution here that you may or may not be able to implement.

In Library.PnETCohorts.SiteCohorts, Ln 3716 is currently: MaxFoliageYearPerSpecies[spc] = cohorts[spc].Max(o => (int)o.MaxFolYear);

I think it should be: MaxFoliageYearPerSpecies[spc] = cohorts[spc].Sum(o => (int)(o.MaxFolYear * o.CanopyLayerProp));

This would correct that it is summing across cohorts of the same species, and accounting for the canopy proportion.

I also think the AllSpecies value should be calculated directly as the sum of these species values within the Output.PnET.PlugIn code, like it does for LAI with LAI_sum[site]. I think the LAI output code (Extension.Output.PnET.PlugIn Ln 320-352) is a good example that the other species-specific outputs should be modified to follow, because it seems to ensure consistency.

Thanks for bringing attention to this issue, and hopefully we can find a way to correct it going forward.

Cheers, -Brian

[Forest Service Shield] Brian Miranda (he/him) Senior Analyst Eastern Planning Service Group Forest Service Eastern and Southern Regions 608-461-2966 @.**@.> La Crosse, WI www.fs.usda.govhttp://www.fs.usda.gov/ [USDA Logo]https://usda.gov/ [Forest Service Twitter] https://twitter.com/forestservice [USDA Facebook] https://www.facebook.com/pages/US-Forest-Service/1431984283714112 Caring for the land and serving people

From: Clement Hardy @.> Sent: Tuesday, October 8, 2024 1:50 PM To: LANDIS-II-Foundation/Extension-Output-Biomass-PnET @.> Cc: Miranda, Brian - FS, WI @.>; Assign @.> Subject: Re: [LANDIS-II-Foundation/Extension-Output-Biomass-PnET] Rasters of foliage biomass and LAI (and maybe others) for all species do not correspond to the sum of the same rasters for each individual species (Issue #11)

Welp, after re-testing, it looks like the problem has disappeared 😳. I have no idea what went on. Maybe a problem when writing the rasters on my side ?

I will re-open the issue if this comes back again; in the meanwhile, I deeply apologize for any waste of time !

— Reply to this email directly, view it on GitHubhttps://github.com/LANDIS-II-Foundation/Extension-Output-Biomass-PnET/issues/11#issuecomment-2400583953, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ACHE7A25XMSMWZURYKLUPI3Z2QSNJAVCNFSM6AAAAABN5HPTAGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMBQGU4DGOJVGM. You are receiving this because you were assigned.Message ID: @.**@.>>

This electronic message contains information generated by the USDA solely for the intended recipients. Any unauthorized interception of this message or the use or disclosure of the information it contains may violate the law and subject the violator to civil or criminal penalties. If you believe you have received this message in error, please notify the sender and delete the email immediately.

Klemet commented 1 month ago

So, as @brmiranda pointed out, the issue wasn't fixed. I'm changing the name.

Klemet commented 1 month ago

In the end, there were two issues, as highlighted by Brian :

I fixed this by making small edits in the PnET Cohort Library, and in this repository (PnET Output). I'll try to make pull requests soon to fix everything.

Klemet commented 1 month ago

I've made the pull requests :

However, I've discovered that a PnET Cohort Library v2 exists (as it's referenced in the PnET Succession repo), even though it's not available on the current PnET Cohort Library repo. I've highlighted what I think should be done to account for this here : https://github.com/LANDIS-II-Foundation/Extension-PnET-Succession/issues/14 .

Once the modifications are merged, I think the issue can be closed for good.