JGCRI / matilda

A probabilistic framework for the Hector simple climate model
Other
5 stars 3 forks source link

Bug Fix in tryCatch #86

Closed kdorheim closed 2 months ago

kdorheim commented 2 months ago

Hi @jk-brown I was trying to use matilda for a Hector related EPA project and kept on running into an error that caused the iterate_model function to break down, the error function implemented within the iterate run tryCatch was also throwing an error due to columns in the data.frame being different lengths.

In this PR made a change to the tryCatch statement and then wrote some unit tests to make sure iterate_model works as expected and even when don't expect it to work.

To recreate the error

# Define a set of parameters that we know should work.
param_values <- data.frame("BETA" = c(0.54, 0.48),
                           "Q10_RH" = c(1.4, 1.3),
                           "NPP_FLUX0" = c(59.2, 53),
                           "AERO_SCALE" = c(1.3, 0.93),
                           "DIFFUSIVITY" = c(2.1, 1),
                           "ECS" = c(2.4, 3.7))

# Set up the hector core.
ini <- system.file("input/hector_ssp245.ini", package = "hector")
hc <- newcore(ini)

# Intentionally pass a parameter value that will cause Hector to crash.
param_values$DIFFUSIVITY[2] <- -1
rslts <- iterate_model(core = hc, params = param_values)

Throws the following error message

An error occurred2
Error in data.frame(scenario = rep(core$name, each = length(save_years)),  : 
  arguments imply differing number of rows: 556, 0, 1
bpbond commented 2 months ago

Swear to God, tryCatch has the worst documentation page in all of R.

jk-brown commented 2 months ago

@bpbond I think its incorrect syntax I used to occupy a df with NAs if a run fails for nonviable parameters. Since save_years = NULL and save_vars = NULL are function defaults, I gunked it up by trying to print scenario names, years, and variable names, but this becomes problematic when save_years = NULL and save_vars = NULL.

At least that's how I am interpreting it. @kdorheim is that correct?

kdorheim commented 2 months ago

@jk-brown yea I think you are right!