Need to error (or at least warn) with older readr!

[bpbond]

Per @pkyle experience on Slack:

Just FYI, as this is the first I've seen/heard of this--in my home directory on PIC, working off of a recent (post-readr-1.3.1) branch off of master, I built XMLs and tried to run the model, but failed calibration/solution in 1990 (in all markets I think) and crashed out shortly thereafter. Worked a while with @pralitp on it...basically, all of my XML files built without warnings or errors, but most had calibration quantities that were incorrect. Sometimes only ~1e-5 off, but in a lot of the ag sector the values were an order of magnitude off. No rhyme or reason that I could determine for which quantities were correct, but the cause of the problem ended up being the readr package on PIC, which for my home directory was 1.1.1. And the solution was to update readr.

But yeah, it's hard to say what must have happened, that gcamdata cranked out a bunch of wrong numbers with no error messages or warnings, and somehow those bad numbers themselves didn't crash anything. :thinking_face: My hat is off to the readr developers!

[ssmithClimate]

Perhaps add some tests for calibration consistency? Could we test if global supplies and demands are equal for some specified global goods?

[bpbond]

@ssmithClimate thanks for the suggestion. I think that's a separate (and more complicated) step, so will open another issue for it.

[ashiklom]

Perhaps we can just add something like this to driver.R?

readr_version <- packageVersion("readr")
if (!(readr_version >= "1.3")) {
  stop("Need readr >= v1.3, but installed version is ", readr_version)
}

[bpbond]

Yes, pretty much, thanks. My only two questions are:

• Whether this should be a stop() or a warning() (there might be a legitimate use case for running out-of-date readr...?)
• Whether it should be in driver() or onLoad()

[pkyle]

Re: legitimate cases, I'd imagine developers with code that works for readr < 1.3.1 choose to stay in that state rather than update both their package and code. But inside the GCAM community, everyone will likely just have to update...I'd still like to see if the errors I found can be replicated on any different computer or PIC user account. Note that I wasn't on the master branch when it happened.

Also do the following lines of the gcamdata DESCRIPTION not require that this package be 1.3.1?

Imports:
    readr (>= 1.3.1),

[bpbond]

@pkyle That really surprised me. I thought an error message would be generated with a sub-1.3.1 readr and that R would refuse to load the package.

[pkyle]

Just to follow up here--Tom Wild (@feralflows) had this exact error earlier tonight, also on PIC. He was working on a different branch from me, and I believe the PIC gods assigned him readr 1.2.1. Same symptoms: no errors or warnings from running the driver and building XML files, but when running the model, pretty much every market blows up in 1990. And, he said that his Colombian collaborators also couldn't get it to run, presumably from this same issue. So yeah, I guess at this point I'm on board with a stop() if readr is less than 1.3.1...while I am curious as to exactly what is going wrong, actually debugging this might be challenging, and is certainly beyond my current project-related mandate! If any of you need compatibility here (e.g., updating would really mess things up for your situation) just let me know off-line.

[mbins]

Giving this one a bump. Nazar just ran into this issue today (readr v1.1.1 on Windows). Same story as above - no errors or warnings when running the driver and building the XMLs, but major calibration failures starting in 1990.

[bpbond]

Thanks @mbins . I'm opening a PR for this.