[general] data.table in-place modifications can lead to reproducibility failure in targets pipline

[MilesMcBain]

Help

• I understand and agree to https://books.ropensci.org/targets/help.html.

Description

I've been mixing {data.table} and {targets} and the flow on effects of in-place modifications in the context of targets pipeline have caught me out a couple of times.

Put simply: In-place modifications of a data.table target A within the context of another target B leads to changes that persist in target A so long as target A is held in memory. If the target A is later read from cache, the changes made during target B are not present. This can lead to inconsistent results in downstream computations.

There are workarounds:

• don't use in-place modification
• force a copy before using in-place modification

I am also wondering if {targets} could have some safeties that would detect or mitigate this situation. For example, targets could verify target dependency hashes before computation on a per-target basis. If a difference is detected, it could either warn and reload, or error.

Here's an example of the problem:

library(targets)
library(data.table)

tar_dir({ # tar_dir() runs code from a temporary directory.
  tar_script(
    list(
      tar_target(
        target_a,
        data.table::data.table(a = c(1, 2, 3)),
        format = "fst_dt"
      ),
      tar_target(
        target_b,
        rowSums(target_a[, b := c(1, 2, 3)]),
        format = "fst_dt"
      ),
      tar_target(
        target_c,
        colnames(target_a)
      )
    ),
    ask = FALSE
  )
  tar_make()
  first_result <- tar_read(target_c)
  tar_delete(target_c)
  tar_make()
  second_result <- tar_read(target_c)
  list(
    first_result = first_result,
    second_result = second_result
  )
})
#> • start target target_a
#> • built target target_a [0.002 seconds]
#> • start target target_b
#> • built target target_b [0.007 seconds]
#> • start target target_c
#> • built target target_c [0.001 seconds]
#> • end pipeline [0.182 seconds]
#> ✔ skip target target_a
#> ✔ skip target target_b
#> • start target target_c
#> • built target target_c [0.001 seconds]
#> • end pipeline [0.172 seconds]
#> $first_result
#> [1] "a" "b"
#> 
#> $second_result
#> [1] "a"

^{Created on 2023-03-29 with reprex v2.0.2}

[alexverse]

data.table is using external pointers but is an exception since it can restore these objects to a valid state even if shared across sessions.
Objects with external pointers are usually non-exportable and is recommended to not be shared across targets.

See also a similar discussion in the future package. https://cran.r-project.org/web/packages/future/vignettes/future-4-non-exportable-objects.html

[wlandau]

I see what you mean, Miles. At the very least, I would recommend memory = "transient" in tar_target() for target_a so every downstream reverse dependency loads a fresh copy from disk. In e198c99, I mentioned this workaround in the documentation.

[wlandau]

More generally, the store class could gain a store_copy_object() method. The default could just return the object, and the one for data tables could run data.table::copy().

[wlandau]

tar_format() could gain a copy argument to support the occasional edge case which needs deep copying.

[wlandau]

Almost done implementing this, c.f. #1042

[wlandau]

Now merged.

[MilesMcBain]

Thanks Will. Amazing turn-around on an improvement as always. 🥇

So to summarise, my options are now:

1. tar_target(<other_args>, memory = "transient") for data.table targets which will reload from disk before being used downstream
2. A custom format, format = tar_format(<other_args>, copy = data.table::copy) to ensure a deep copy is made of a possibly persistent target before the target is used downstream.

[MilesMcBain]

and actually, on closer reading:

3. tar_target(<other_args>, format = "fst_dt") which now deep copies as per 2. by default.

I quite like this solution. So the protection is on by default if you're using "fst_dt", but you can turn it off fairly easily by implementing your own custom format if performance is a concern. 👍