JGCRI/hector

Spurious errors showing up in GA tests, failing pkgdown

bpbond opened this issue · 5 comments

Summarizing what I know.

Error in pkgdown GA

The v3 pkgdown GitHub Action is failing:

! path[1]="tables/ssp245_emiss-constraints_rf.csv": No such file or directory

The source of the 'error'

This isn't an error; it's a message originating from the intended functionality of ini_to_core_reader.cpp:

            #ifdef USE_RCPP
            // ANS: This is the algorithm used if Hector is compiled
	    // as an R package.
            //
	    // If the csvFileName normalizes to a real path, use that.
            // Otherwise, assume that it is pointing to a file in the
            // same directory as the INI file.
            try {
                // Second argument is "winslash", which is only used
                // on Windows machines and is ignored for Unix-based
                // systems. "\\" (single backward slash) is the
                // default. Need it here to access the third argument
                // -- mustWork -- which throws the error to be caught
                // if the path doesn't exist
                csvFileName = Rcpp::as<string>(normalizePath(csvFileName, "\\", true));
            } catch (...) {
                // Get the full path of the INI file with
                // normalizePath. Then, get just the directory using dirname.
                Rcpp::String parentPath = dirname(normalizePath(reader->iniFilePath));
                csvFileName = Rcpp::as<string>(filepath(parentPath, csvFileName));
            }

Summarizing: we use normalizePath to try to find the csv: file, and if that doesn't work, catch the error and, assuming that it's a relative path, add on the path of the source INI file. This is old code; functionality has not changed in years.

The other GAs are seeing this error too, but ignoring it

From the same PR, the R-cmd-check test:

Screenshot 2023-02-04 at 12 00 48 PM

@kdorheim THIS IS THE ISSUE YOU FLAGGED and I said, pfft, let's ignore it for now. 🤦

So we have two problems:

  1. The 'error message' generated in the try-catch block of ini_to_core_reader.cpp is being printed (I think?) and then 'seen' by R, even though the error was caught in the C++ code. This didn't use to occur
  2. In the pkgdown GA, this causes failure because knitr sees the vignette 'failing' and errors out

Note: the code is still running OK!

Even though an 'error message' is generated, the R code still runs fine:

> ini <- system.file("input/hector_ssp245.ini", package = "hector")
> hc <- newcore(ini)
Error in (function (path, winslash = "\\", mustWork = NA) :
path[1]="tables/ssp245_emiss-constraints_rf.csv": No such file or directory

[many identical messages, one for each `csv:` reference in the INI]

> run(hc)
Hector core:	Unnamed Hector core
Start date:	1745
End date:	2300
Current date:	2300
Input file:	/Users/bpbond/Dropbox/Documents/Work/Code/hector/inst/input/hector_ssp245.ini

[fetchvars and plot, everything looks good]

Previously good and merged PRs fail

I have tried branching from a number of different commits in the v3_dev history and then opening a PR, and the same thing happens (e.g. #690 ).

Not a pkgdown.yaml problem

Sort of a corollary of the point above. I've tried lots of different tweaks to pkgdown.yaml, making sure it has the same setup as R-CMD-check.yaml, trying older (e.g. from 2022) versions, etc.

This implies it's an R/environment/software stack problem?

Local replication

I have been able to replicate this locally, so successfully that I've borked my local installation!

Currently for me with v3_dev HEAD:

  • Rstudio "Check" button, which should be using R CMD CHECK, succeeds and runs all tests
  • RStudio "Build->Install" succeeds
  • RStudio "Build->Test" fails
  • devtools::check() on console fails: [ FAIL 4885 | WARN 1 | SKIP 0 | PASS 330 ]

So currently any call to the ini_to_core.cpp code now produces 'error messages' on my system. This occurred after I tried to follow pkgdown::build_site() and ended up running

pkg <- as_pkgdown(".", override = list())
utils::install.packages(pkg$src_path, repos = NULL, type = "source", quiet = FALSE)

(which it does in its code)

I've tried uninstalling the hector package, restarting R... 😕

Strictly an R issue

There's no behavior change when running the C++ code.

Summary

This is confusing. I don't think it's a YAML file issue, but rather an R issue and/or behavior difference between the devtools toolchain and R CMD ..., something that changed fairly recently. What's suspicious, of course, is that this is right when @kdorheim and I were upgrading the GA YAML files.

@pralitp @crvernon

I'm not sure what to try next here...any thoughts? Does either of you have bandwidth to look at this next week? It's holding up the entire Hector v3 release. edit: never mind!

Possible workarounds:

  • Don't use the try-catch logic in ini_to_core_reader.cpp -- either (1) always use the C++17 filesystem approach, or (2) do file.path and then file.exists to determine whether the csv file exists at the absolute path or not
  • Maybe there's a way to force build_site / knitr to ignore seeming error messages? ugh

Well, this was certainly a pain in the rear, but:

I have merged #693 that is not a "workaround" but rather a fix of the root issue: the janky try/catch approach taken in ini_to_core_reader.cpp. The new code uses file.exists instead and is shorter and simpler. 🎉

Even simpler would be to rip out Rcpp usage entirely in this file, and just depend on std::filesystem! I couldn't quite get this to work — see #694 — but enough already, closing that PR but will open an issue for the future.