-
Notifications
You must be signed in to change notification settings - Fork 29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Simplify biomee drivers #270
Simplify biomee drivers #270
Conversation
marcadella
commented
Dec 2, 2024
- Reduce size of params_species in biomee drivers
- Reduce size of init_cohort in biome driver
- First column of biomee drivers is now ignored (the number of initial cohorts is given by the array itself). For safety, this column must be set to 0.
- Update documentation
- Reduce size of init_cohort in biome driver - First column of biomee drivers is now ignored (the number of initial cohorts is given by the array itself). For safety, this column must be set to 0. - Update documentation
…drivers-is-needlessly-filled-up-to-16-rows
R/run_biomee_f_bysite.R
Outdated
|
||
if (registered_n_cohorts != 0) { | ||
warning( | ||
sprintf("Error: init_cohorts' init_cohort_species is deprecated and should contain a negative value (found %i)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Should contain a 0"
R/run_biomee_f_bysite.R
Outdated
@@ -343,6 +343,15 @@ run_biomee_f_bysite <- function( | |||
return(TRUE) | |||
} | |||
}) | |||
|
|||
registered_n_cohorts <- init_cohort$init_n_cohorts[[1]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 0 is just a temporary safety solution to avoid a breaking change, right? At the same time it is actually breaking the code anyway.
Should we really use this numeric checking?
It seems simpler to make a check for the element names and if init_n_cohorts
is found just emit a warning and append FALSE to data_integrity
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed
- Detect 'init_n_cohorts' and emit a warning when present.
R/data.R
Outdated
#' \item{init_cohort_bsw}{Initial biomass of sapwood, in kg C per individual.} | ||
#' \item{init_cohort_bHW}{Initial biomass of heartwood, in kg C per tree.} | ||
#' \item{init_cohort_bHW}{Initial biomass of heartwood, in kg C per individual.} | ||
#' \item{init_cohort_seedC}{Initial biomass of seed, in kg C per individual.} | ||
#' \item{init_cohort_nsc}{Initial non-structural biomass.} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the NSC biomass also in kg C per individual?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently.