pik-piam / remind2

The remind2 package contains the REMIND-specific routines for data and model output manipulation.
0 stars 39 forks source link

Improve debugging and error messages for report functions #604

Closed orichters closed 1 month ago

orichters commented 1 month ago

Dear colleagues, I find it sometimes really annoying and difficult to find out where and why a remind2 function call fails exactly, because mostly somewhere a mbind files. Often, the report functions have the following structure:

cons <- gdx::readGDX(gdx,name=c("vm_cons"),field="l",restore_zeros=FALSE,format="first_found")
out <- NULL
out <- mbind(out, setNames(cons, "Consumption"))

Now, if I do some stupid stuff, I get this in the logs:

out <- mbind(out, setNames(cons["LAM",,], "Consumption LAM"))

Error in `mbind()`:
! Cannot handle objects! Data as well as spatial dimensions differ!
Run `rlang::last_trace()` to see where the error occurred.

And often, also rlang::last_trace() is not really helpful, in particular, if the var name was not typed in as here but rather composed of some sets.

I thought we could maybe replace it by something like that which would give a much more consistent and meaningful error message:

bindName <- function(out, data, name) {
  joined <- try(mbind(out, setNames(data, name)))
  if (inherits(joined, "try-error")) {
    stop(paste(c(paste0("Failing mbind for variable '", name, "':"),
                 "### out:",
                 capture.output(str(out)),
                 "### new data:",
                 capture.output(str(setNames(data, name)))
                ), collapse = "\n"))
  }
  return(joined)
}

And then use:

out <- bindName(out, cons["LAM",,], "Consumption LAM")

Error in mbind(out, setNames(data, name)) :
  Cannot handle objects! Data as well as spatial dimensions differ!
Error in `bindName()`:
! Failing mbind for variable 'Consumption LAM'
### out:
A magpie object (package: magclass)
 @ .Data:  num [1:228] 1.649 1.422 0.673 8.218 0.966 ...
 $ dimnames:List of 3
  ..$ all_regi: chr [1:12] "LAM" "OAS" "SSA" "EUR" ...
  ..$ ttot    : chr [1:19] "y2005" "y2010" "y2015" "y2020" ...
  ..$ data    : chr "Consumption"
### new data:
A magpie object (package: magclass)
 @ .Data:  num [1:19] 1.65 1.81 2.05 2.28 2.53 ...
 $ dimnames:List of 3
  ..$ all_regi: chr "LAM"
  ..$ ttot    : chr [1:19] "y2005" "y2010" "y2015" "y2020" ...
  ..$ data    : chr "Consumption LAM"
Run `rlang::last_trace()` to see where the error occurred.

Now I see directly which variable was affected and what was the issue (all_regi differs).

Maybe you have even better ideas, @0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q, @Renato-Rodrigues, @fbenke-pik.

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q commented 1 month ago

It is a bit hit and miss. Somebody would have to convert the ~500 usages of that idiom in the package,. Then users would have to stick to it. But the ~450 other uses of mbind() would not benefit.

The solution is obvious: proper error reporting by mbind() itself, instead of a cooked-up wrapper function.

fbenke-pik commented 1 month ago

What Michaja said. Have you tried creating a PR to magclass?

orichters commented 1 month ago

No, my only experience with "magclass" is: "replacing all occurrences by quitte".

fbenke-pik commented 1 month ago

https://github.com/pik-piam/magclass/blob/master/R/mbind.R#L79-L81

I guess we could make these error messages more meaningful?

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q commented 1 month ago

https://github.com/pik-piam/magclass/blob/master/R/mbind.R#L79-L81

I guess we could make these error messages more meaningful?

The problem is: to return really meaningful error messages, one needs to make some assumptions about the use mbind() is put to. Which might or might not jive with Jan. Specifically, one hast to assume that few regions/years/names are bound to many regions/years/names, that the latter are the first inputs element, and that only the differences to the first inputs element are significant. I have a draft, I just need to polish it a bit before submitting it to the court of RSE opinion.