Skip to content

Commit

Permalink
Fix simulation id validation (#138)
Browse files Browse the repository at this point in the history
* extract simulation ids assuming up to two levels of piParameters depth

* ensure simulation ids are identical

* update snapshots due to new error bar caps size in ospsuite::DefaultPlotConfiguration

* improve error message for missing simulation IDs

* Add tests for multi-path ID verification

* run styler:::style_active_pkg()
  • Loading branch information
rengelke authored Jul 4, 2024
1 parent 2079e3a commit 6d200d0
Show file tree
Hide file tree
Showing 9 changed files with 161 additions and 53 deletions.
2 changes: 1 addition & 1 deletion R/ParameterIdentification.R
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ ParameterIdentification <- R6::R6Class(
costControl$scaling <- private$.outputMappings[[idx]]$scaling
ospsuite.utils::validateIsOption(
options = costControl,
validOptions = ObjectiveFunctionSpecs
validOptions = ObjectiveFunctionSpecs
)
costSummary <- calculateCostMetrics(
df = obsVsPredDf,
Expand Down
15 changes: 10 additions & 5 deletions R/messages.R
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,14 @@ messages$hessianEstimation <- function() {
"Post-hoc estimation of Hessian matrix."
}

messages$errorSimulationIdMissing <- function(id) {
paste0(
"Mismatch or missing ID detected: ", id,
". Ensure each Simulation ID matches with corresponding PIParameter and OutputMapping IDs."
)
messages$errorSimulationIdMissing <- function(simulationIds, piParamIds, outputMappingIds) {
message <- capture.output(cat(
"Mismatch or missing ID detected.\n",
"Ensure each Simulation ID matches with corresponding PIParameter and OutputMapping IDs.\n",
"Simulation IDs: ", paste(simulationIds, collapse = ", "), "\n",
"PIParameter IDs: ", paste(piParamIds, collapse = ", "), "\n",
"OutputMapping IDs: ", paste(outputMappingIds, collapse = ", ")
))

return(paste(message, collapse = "\n"))
}
31 changes: 23 additions & 8 deletions R/utilities-simulation.R
Original file line number Diff line number Diff line change
Expand Up @@ -31,19 +31,34 @@
#' the mismatch or absence of IDs.
#' @keywords internal
.validateSimulationIds <- function(simulationIds, piParameters, outputMappings) {
# Extract unique IDs from piParameters
piParamIds <- lapply(piParameters, function(param) .getSimulationContainer(param$parameters[[1]])$id)
# Extract unique IDs from piParameters assuming up to two levels of list depth
piParamIds <- lapply(piParameters, function(param) {
if (is.list(param$parameters)) {
return(lapply(param$parameters, function(sub_param) {
.getSimulationContainer(sub_param)$id
}))
} else {
return(.getSimulationContainer(param$parameters[[1]])$id)
}
})
piParamIds <- unique(unlist(piParamIds))

# Extract unique IDs from outputMappings
outputMappingIds <- lapply(outputMappings, function(mapping) .getSimulationContainer(mapping$quantity)$id)
outputMappingIds <- lapply(outputMappings, function(mapping) {
.getSimulationContainer(mapping$quantity)$id
})
outputMappingIds <- unique(unlist(outputMappingIds))

# Validate that each simulationId is present in both piParamIds and outputMappingIds
for (id in simulationIds) {
if (!(id %in% piParamIds && id %in% outputMappingIds)) {
stop(messages$errorSimulationIdMissing(id))
}
# sort IDs before comparison
simulationIds <- sort(unique(unlist(simulationIds)))
piParamIds <- sort(piParamIds)
outputMappingIds <- sort(outputMappingIds)

# Validate that simulationId is identical with piParamIds and outputMappingIds
if (!identical(simulationIds, piParamIds) || !identical(simulationIds, outputMappingIds)) {
stop(messages$errorSimulationIdMissing(
simulationIds, piParamIds, outputMappingIds
), call. = TRUE)
}

return()
Expand Down
24 changes: 12 additions & 12 deletions tests/testthat/_snaps/parameter-identification/after-estimation.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading

0 comments on commit 6d200d0

Please sign in to comment.