geco-bern / rsofun

Implements the Simulating Optimal FUNctioning framework for site-scale simulations of ecosystem processes, including model calibration. It contains Fortran 90 modules for the P-model, SPLASH, and BiomeE models.
https://geco-bern.github.io/rsofun/
GNU General Public License v3.0
25 stars 29 forks source link

Consistent calib_sofun output #98

Closed pepaaran closed 1 year ago

pepaaran commented 2 years ago

Depending on whether we use GenSA or BayesianTools as optimizers for the calibration, the output returned by calib_sofun changes.

With GenSA, the output is a list (shown below), which gives quite some useful information about the iterations of the optimization, the minimum value of the cost function and the values of the parameters at the minimum (ie, calibrated).

List of 4 $ value : num 1.3 $ par : Named num [1:3] 0.0568 0.2533 1.0233 ..- attr(, "names")= chr [1:3] "kphio" "soilm_par_a" "soilm_par_b" $ trace.mat: num [1:594, 1:4] 1 1 1 1 1 1 2 2 2 2 ... ..- attr(, "dimnames")=List of 2 .. ..$ : NULL .. ..$ : chr [1:4] "nb.steps" "temperature" "function.value" "current.minimum" $ counts : int 938

While with BayesianTools, we're only returning a list with the calibrated parameter values.

List of 1 $ par: Named num 0.0432 ..- attr(*, "names")= chr "kphio"

We should agree what information is useful and should be returned with the optimized values of the parameters, or if we return only the parameter values. Of course the output of the two optimizers will be different, so we need to find a way to structure the output in an analogous way (or document it such that people know what to expect). I think especially with BayesianTools we should provide more information, since it's almost more interesting to look at the posterior distribution of the parameters than the calibrated value and the user should be able to look at the convergence of the algorithm too. Furthermore, we would avoid having a blackbox calib_sofun function that makes it difficult to debug cost functions written by users.

stineb commented 2 years ago

We definitely should make it possible to return the full object returned by the respective library/function. I propose something like the following. Instead of the current

return(out_optim)

we should be doing something like

return(par = par, mod = mod)

where par is the named vector of best estimates of parameters and mod is the model object, returned here by either GenSA::GenSA() or BayesianTools::runMCMC(). Returning mod may be made optional.

pepaaran commented 2 years ago

I will go ahead with that, I like the idea. Thanks Beni

khufkens commented 2 years ago

I think this is fixed by: https://github.com/computationales/rsofun/pull/112 or earlier correct?

pepaaran commented 2 years ago

No, I actually corrected this yesterday but haven't done a pull request for it. Will do today.

khufkens commented 2 years ago

Ok, no worries. That's why I didn't close the issue yet.

khufkens commented 1 year ago

Closing, fixed per #132