ropensci / allodb

An R package for biomass estimation at extratropical forest plots.
https://docs.ropensci.org/allodb/
GNU General Public License v3.0
36 stars 11 forks source link

inconsistent agb calculations #109

Closed ValentineHerr closed 4 years ago

ValentineHerr commented 4 years ago
# load data ####
all_cores <- read.csv("https://raw.githubusercontent.com/EcoClimLab/ForestGEO_dendro/master/data_processed/all_cores_for_Camille_to_check.csv?token=AEWDCIOYCC3HW5E2M5A5G4S637OJ2", stringsAsFactors = F) #takes a while... sorry

# calculate AGB####
all_cores$agb <- NA

## for all sites but BCI, use allodb
library(allodb) #remotes::install_github("forestgeo/allodb")

idx <- which(!all_cores$site %in% "BCI")

all_cores$agb[idx] <- get_biomass(dbh=all_cores$dbh[idx],
                                  genus=all_cores$genus[idx],
                                  species = all_cores$species[idx],
                                  coords = as.matrix(all_cores[idx, c("Longitude", "Latitude")]) ) / 1000 # /1000 to get in Mg

summary(all_cores$agb) # there are negative values (NA's are either BCI or cases where we don't have dbh)

# get the index of the negative agb
idx <- which(all_cores$agb <0)

#rerun the exact same thing as above but only for those that got negative agb
summary(get_biomass(dbh=all_cores$dbh[idx],
                    genus=all_cores$genus[idx],
                    species = all_cores$species[idx],
                    coords = as.matrix(all_cores[idx, c("Longitude", "Latitude")]) ) / 1000) # now there is no negative values.... why ???
cpiponiot commented 4 years ago

Thanks for putting this issue Valentine, as suspected the problem comes from a mixup in the attribution of weights to different equations, but I haven't yet identified the exact moment where it happens in the get_biomass function. I'm thinking of reorganizing the function a little anyway (because it can't handle very large data frames as written now), and I'll make sure that equation IDs are used all the time to avoid inconsistent weight allocation. I'll write to you when I've finished and we can try it again with the tree ring data.

ValentineHerr commented 4 years ago

Thanks Camille! I hope it is not too complexe of a problem... let me know when you need me to test the changes.

cpiponiot commented 4 years ago

Don't worry, it's great that you spotted that, it could have gone unnoticed and it's actually a big problem, but (hopefully) relatively easy to fix

cpiponiot commented 4 years ago

I just committed some changes to the get_biomass function that should make sure that the weights correspond to the right equation; I still have to update the taxonomic weight, but the function should work and not give negative values anymore

ValentineHerr commented 4 years ago

@cpiponiot , I just tried the new function and I am getting this error:

image

cpiponiot commented 4 years ago

Okay, I think I know where it comes from, I'll try to fix it this afternoon

cpiponiot commented 4 years ago

I tried something but when using it from the package it's still not working, I'll try something new tomorrow

ValentineHerr commented 4 years ago

Hi @cpiponiot, Just checking in about this. Have you been abke to work on it? I just tried the function again (after re-installing the package) and I am getting the same error message about function ".".

cpiponiot commented 4 years ago

@ValentineHerr yes I thought that particular issue was solved, and when I use get_biomass from the allodb package (after downloading it from github) it works with the example in the function description (with coordinates either as vector with 2 values or as matrix with 2 columns, and for several sites). Can you send me the codes for which it doesn't work so I can try?

ValentineHerr commented 4 years ago

@cpiponiot, I restarted my session and that error went away... (my bad) but, now I am getting memory limit issues... I'll try to see if I can deal with that on my own by doing some parallelization but I thought you'd like to know. Something is eating more than it did before! :-)

cpiponiot commented 4 years ago

Yes I've noticed it does that for very large data sets, it's the weight matrix that's super large but I haven't found a way around it yet... I'll try breaking the AGB/weight calculation down inside the function, but I'm afraid it's going to increase the total running time

ValentineHerr commented 4 years ago

Hi @cpiponiot, is it normal that the function does not work anymore for only one tree at a time?

> get_biomass(dbh = 1.99, genus = "Acer", species = "rubrum", coords = c(-72.1755 ,  42.5388 ))
Error in rowSums(weight, na.rm = TRUE) : 
  'x' must be an array of at least two dimensions
ValentineHerr commented 4 years ago

the problem of inconsistent results is solved but I still get negative values. Let me know if you are planning to fix that or if I should just give agb<0 a value of 0 or something.

cpiponiot commented 4 years ago

Hi @cpiponiot, is it normal that the function does not work anymore for only one tree at a time?

I'll go check what happens when we only have 1 stem

the problem of inconsistent results is solved but I still get negative values. Let me know if you are planning to fix that or if I should just give agb<0 a value of 0 or something.

I plan to fix that but haven't had time to do it yet, so if you need to use the function you can put 0 to negative agb values (normally this only happens for small dbhs), and I'll notify you when the problem is fixed

ValentineHerr commented 4 years ago

sounds good, thanks!