Implement the join_inputs_design into the current SDA workflows.
#3634
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,19 +4,20 @@ | |
| #' are shared across sites to ensure consistent parameter sampling. | ||
| #' | ||
| #' @param settings A PEcAn settings object containing ensemble configuration | ||
| #' @param ensemble_size Integer specifying the number of ensemble members | ||
| #' @param ensemble_samples list of ensemble parameters across PFTs. The default is NULL. | ||
| #' @param sobol for activating sobol | ||
|
There was a problem hiding this comment. Add the |
||
| #' @return A list containing ensemble samples and indices | ||
| #' | ||
| #' @export | ||
|
|
||
| generate_joint_ensemble_design <- function(settings, | ||
| ensemble_size, | ||
| ensemble_samples = NULL, | ||
| sobol = FALSE) { | ||
| if (sobol) { | ||
| ensemble_size <- as.numeric(ensemble_size) * 2 | ||
| } | ||
| ens.sample.method <- settings$ensemble$samplingspace$parameters$method | ||
| design_list <- list() | ||
| sampled_inputs <- list() | ||
|
|
@@ -51,29 +52,34 @@ generate_joint_ensemble_design <- function(settings, | |
| sampled_inputs[[input_tag]] <- input_result$ids | ||
| design_list[[input_tag]] <- input_result$ids | ||
| } | ||
|
|
||
| # Sample parameters if we don't have it. | ||
| if (is.null(ensemble_samples)) { | ||
| PEcAn.uncertainty::get.parameter.samples( | ||
| settings, | ||
| ensemble.size = ensemble_size, | ||
| posterior.files, | ||
| ens.sample.method | ||
| ) | ||
| samples.file <- file.path(settings$outdir, "samples.Rdata") | ||
| } | ||
|
There was a problem hiding this comment. Add There was a problem hiding this comment. Could you clarify the argument for when it makes sense to assume that we have ensemble_samples already? In other words, in what cases would you want/need to recycle a previous ensemble_samples rather than generate a new sample from the posteriors? |
||
|
|
||
| # Load samples from file | ||
| samples <- new.env() | ||
| # if we don't have the parameters from the outside. | ||
| if (is.null(ensemble_samples)) { | ||
|
There was a problem hiding this comment. Why have to consecutive if(is.null(ensemble_samples) blocks? why can't these be combined into one block? |
||
| if (ile.exists(samples.file)) { | ||
| load(samples.file, envir = samples) | ||
| } else { | ||
| PEcAn.logger::logger.error(samples.file, "not found, this file is required") | ||
| } | ||
| } | ||
| if (!is.null(samples$ensemble.samples) | !is.null(ensemble_samples)) { | ||
| # Just a placeholder: extract representative trait index per ensemble member | ||
| # You may want to flatten or select indices per trait | ||
| design_list[["param"]] <- seq_len(ensemble_size) | ||
| } else { | ||
| PEcAn.logger::logger.warn("ensemble.samples not found in samples.Rdata") | ||
|
There was a problem hiding this comment. why drop this from error to warn? |
||
| } | ||
|
There was a problem hiding this comment. Add |
||
| design_matrix <- data.frame(design_list) | ||
|
|
||
|
|
@@ -85,7 +91,5 @@ generate_joint_ensemble_design <- function(settings, | |
| return(sobol_obj) | ||
| } | ||
|
|
||
| return(list(X = design_matrix)) | ||
|
There was a problem hiding this comment. Is it important that this be a list? All the uses I've seen so far immediately subset the result to only store the X component, so would it make sense to return There was a problem hiding this comment. It is important so that the sobol version works correctly. This ensures that regardless of whether the sobol or non-sobol version is called that the output is a list that includes the design as X. In the sobol version the list includes additional info beyond just X that's required by the function that does the sobol index calculations, but not required to do the runs themselves There was a problem hiding this comment. 👍 Can you add this to the function docs, please? I think it'd be enough to list in the @return block the possible items for the Sobol case and mention that in the non-sobol case the list only contains X |
||
| } | ||
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.
Remove the
samplesargument as it is no longer needed.