Skip to content
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

Conversation

marcadella
Copy link
Collaborator

  • 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
@marcadella marcadella requested a review from fabern December 2, 2024 13:46
@marcadella marcadella marked this pull request as ready for review December 2, 2024 13:46

if (registered_n_cohorts != 0) {
warning(
sprintf("Error: init_cohorts' init_cohort_species is deprecated and should contain a negative value (found %i)",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Should contain a 0"

@@ -343,6 +343,15 @@ run_biomee_f_bysite <- function(
return(TRUE)
}
})

registered_n_cohorts <- init_cohort$init_n_cohorts[[1]]
Copy link
Member

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.

Copy link
Collaborator Author

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.
@marcadella marcadella requested a review from fabern December 3, 2024 08:57
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.}
Copy link
Member

@fabern fabern Dec 3, 2024

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?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently.

@marcadella marcadella requested a review from fabern December 3, 2024 09:40
@marcadella marcadella merged commit c800b6b into master Dec 3, 2024
7 checks passed
@marcadella marcadella deleted the 267-params_species-in-both-biomee-example-drivers-is-needlessly-filled-up-to-16-rows branch December 3, 2024 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

params_species (in both BiomeE example drivers) is needlessly filled up to 16 rows
2 participants