fix(parquet): caching bug due to different partitions (#128)

* fix(parquet): caching bug due to different partitions

Previously, the parquet files were first downloaded to a temporary
directory and then a hard link was created. This failed when
the directories were on different partitions and the file
is now immediately downloaded to its final destination

* also add caching to the CI

Author: sebffischer

.github/workflows/r-cmd-check.yml

@@ -44,6 +44,16 @@ jobs:
         with:
           r-version: ${{ matrix.config.r }}
 
+
+      - name: Cache OpenML
+        uses: actions/cache@v3
+        id: openml-cache
+        with:
+          path: ~/openml
+            # we will just (almost) always get the cache from the restore keys
+          key: ${{ runner.os }}-openml-${{ hashFiles('.') }}
+          restore-keys: ${{ runner.os }}-openml-
+
       - uses: r-lib/actions/setup-r-dependencies@v2
         with:
           extra-packages: any::rcmdcheck

DESCRIPTION

@@ -46,5 +46,5 @@ Config/testthat/edition: 3
 Encoding: UTF-8
 NeedsCompilation: yes
 Roxygen: list(markdown = TRUE)
-RoxygenNote: 7.2.3
+RoxygenNote: 7.2.3.9000
 Config/Needs/website: rmarkdown

NEWS.md

@@ -1,5 +1,7 @@
 # mlr3oml 0.9.0-9000
 
+* Bugfix: Caching of parquet files failed when the temporary directory was on a
+different partition as the cache directory
 * feat: Add option `mlr3oml.retries` to control number of retries when
 downloading data from OpenML. The default is 3.
 

R/OMLData.R

@@ -184,8 +184,6 @@ OMLData = R6Class("OMLData",
       }
       if (is.null(private$.parquet_path)) {
         loadNamespace("mlr3db")
-        # this function is already cached, it works a little different than the cached(f, ...)
-        # because we cache it as .parquet and not as .qs
         private$.parquet_path = cached(
           download_parquet, "data_parquet", self$id, desc = self$desc, cache_dir = self$cache_dir,
           server = self$server, test_server = self$test_server, parquet = TRUE

R/OMLRun.R

@@ -245,7 +245,9 @@ as_resample_result.OMLRun = function(x, store_backends = TRUE, ...) {
     resampling = list(resampling),
     iteration = seq_len(resampling$iters),
     prediction = predictions,
-    uhash = uuid::UUIDgenerate()
+    uhash = uuid::UUIDgenerate(),
+    param_values  = replicate(list(), n = 2),
+    learner_hash = learner$hash
   )
 
   ResampleResult$new(ResultData$new(data, store_backends = store_backends))

R/cache.R

@@ -117,7 +117,7 @@ cached = function(fun, type, id, test_server, parquet = FALSE, ..., cache_dir =
     return(fun(id, ...))
   }
 
-  path = file.path(cache_dir, ifelse(test_server, "test", "public"), type)
+  path = file.path(cache_dir, if (test_server) "test" else "public", type)
   file = file.path(path, sprintf("%i.%s", id, if (parquet) "parquet" else "qs"))
 
   if (file.exists(file)) {
@@ -138,12 +138,11 @@ cached = function(fun, type, id, test_server, parquet = FALSE, ..., cache_dir =
     dir.create(path, recursive = TRUE)
   }
 
-  obj = fun(id, ...)
   if (parquet) {
-    lg$debug("Moving parquet data from tempfile to cache.", type = type, id = id, file = file)
-    file.rename(obj, file)
+    obj = fun(id, ..., file = file)
     return(file)
   }
+  obj = fun(id, ...)
 
   lg$debug("Storing compressed object in cache", type = type, id = id, file = file)
   qs::qsave(obj, file = file, nthreads = getOption("Ncpus", 1L))

R/download_parquet.R

@@ -1,19 +1,19 @@
-download_parquet = function(data_id, server, desc = download_desc_data(data_id, server)) {
-  get_parquet(desc$minio_url, api_key = get_api_key(server))
+download_parquet = function(data_id, server, desc = download_desc_data(data_id, server), file = NULL) {
+  get_parquet(desc$minio_url, api_key = get_api_key(server), file = file)
 }
 
-get_parquet = function(url, ..., server, api_key = get_api_key(server), retries = getOption("mlr3oml.retries", 3L)) {
-  path = tempfile(fileext = ".parquet")
+get_parquet = function(url, ..., server, api_key = get_api_key(server), retries = getOption("mlr3oml.retries", 3L), file = NULL) {
+  file = file %??% tempfile(fileext = ".parquet")
   url = sprintf(url, ...)
 
   lg$info("Retrieving parquet.", url = url, authenticated = !is.na(api_key))
 
   for (retry in seq_len(retries)) {
-    response = download_file(url, path, api_key = api_key)
+    response = download_file(url, file, api_key = api_key)
 
     if (response$ok) {
-      lg$debug("Downloaded parquet file.", path = path)
-      return(path)
+      lg$debug("Downloaded parquet file.", path = file)
+      return(file)
     } else if (retry < retries && response$http_code >= 500L) {
       delay = max(rnorm(1L, mean = 10), 0)
       lg$debug("Server busy, retrying in %.2f seconds", delay, try = retry)

tests/testthat/setup.R

@@ -9,4 +9,8 @@ lg = lgr::get_logger("mlr3oml")
 old_threshold = lg$threshold
 lg$set_threshold("warn")
 
-..old_opts = options(mlr3oml.verbose = FALSE, mlr3oml.retries = 20L)
+..old_opts = options(
+  mlr3oml.verbose = FALSE,
+  mlr3oml.retries = 20L,
+  mlr3oml.cache = if (identical(Sys.getenv("GITHUB_ACTIONS"), "true")) "~/openml" else FALSE
+)