agg_record and agg_capture advance global RNG state, leading to unintuitive downstream behavior

[gjhunt]

ragg::agg_record() and ragg:agg_capture() advance R’s global RNG state by calling sample() to generate a device name. This causes .Random.seed to change during graphics device initialization, etc.

This arguably breaks reproducibility guarantees when these functions are used as infrastructure, for example by IRkernel in Jupyter notebooks.

Minimal example

In a Jupyter / IRkernel context (but reproducible via direct calls):

set.seed(1)
s1 <- .Random.seed

# Device initialization triggers RNG use
ragg::agg_record()
dev.off()

s2 <- .Random.seed
identical(s1, s2)
# FALSE

The RNG advance occurs here:

paste0("agg_record_", sample(.Machine$integer.max, 1))

and here:

paste0('agg_capture_', sample(.Machine$integer.max, 1))

on lines 430 and 507 of agg_dev.R (and potentially elsewhere?)

Unintuitive behavior

• agg_record() is now used implicitly as a recording device (for example by IRkernel / evaluate in Jupyter notebooks)
• This causes .Random.seed to advance between notebook cells, even for deterministic code
• Running the same code via Rscript does not exhibit this behavior
• This creates unintuitive differences between running code in a notebook versus converting the notebook to a script and running it, due solely to differences in RNG state

It is unclear whether this is an intentional design decision, and whether users should expect code to behave differently in a Jupyter notebook versus a script. However, this makes the placement of cell breaks in a notebook affect RNG state, which is quite unintuitive.

For example, a single cell:

set.seed(1234)
runif(1)

behaves differently than two sequential cells:

set.seed(1234)

runif(1)

This is likely confusing for typical users.

Environment

• ragg: 1.5.0
• IRkernel: 1.3.2
• R: 4.5.2

[flying-sheep]

Wow I forgot how bad R’s random number infrastructure is.

.Random.seed is an integer vector, containing the random number generator (RNG) state for random number generation in R. It can be saved and restored, but should not be altered by the user.

OK, who wrote this and why should it not? If as I assume there is no actual reason, you can do something like the following mostrosity. Or just use a library that allows you to use an independent RNG.

# .Random.seed doesn't have to exist in .GlobalEnv
# So this function deletes it if it didn't exist and restores it otherwise
with_backed_up_state <- function(block) {
    backup_random_state <- .GlobalEnv$.Random.seed
    tryCatch({
        rv <- block  # evaluate block, which might modify .GlobalEnv$.Random.seed
        new_state <- .GlobalEnv$.Random.seed
    }, finally = {
        if (!is.null(backup_random_state)) {
            .GlobalEnv$.Random.seed <- backup_random_state
        } else if (!is.null(.GlobalEnv$.Random.seed)) {
            rm(".Random.seed", pos = .GlobalEnv)
        }
    })
    list(result = rv, new_state = new_state)
}

default_state_env <- new.env()

# All the above and below is required to use a different seed than GlobalEnv$.Random.seed
# Yikes R, you could do better.
with_state <- function(block, state_env = default_state_env) {
    # make sure the state is initialized
    if (is.null(state_env$random)) {
        state_env$random <- with_backed_up_state(runif(1L))$new_state
    }
    # modify random seed
    res_and_state <- with_backed_up_state({
        .GlobalEnv$.Random.seed <- state_env$random
        rv <- block  # evaluate block
        rv
    })
    state_env$random <- res_and_state$new_state
    res_and_state$result
}

And then replace both locations where you create a random state with this:

name <- with_state(paste0('agg_record_', sample(.Machine$integer.max, 1)))

ragg/R/agg_dev.R

Line 430 in f606c63

name <- paste0('agg_capture_', sample(.Machine$integer.max, 1))

and

ragg/R/agg_dev.R

Line 507 in f606c63

name <- paste0('agg_record_', sample(.Machine$integer.max, 1))

---

PS why “.Random.seed”? It’s clearly the random state, not the random seed. Anyway …

[gjhunt]

I can see a couple of solutions to this:

1. Isolate the PRNG by hand

Indeed, the isolated RNG stream is tricker than I had anticipated. I think your approach will generally work, however in an actual package the state needs to be mutable so you'd have to do something like:

# Private but mutable
.prng <- new.env(parent = emptyenv())

# Make sure `internal_random_state` is initialized
.prng$internal_random_state <- with_backed_up_state(runif(1L))$new_state

# All the above and below is required to use a different seed than GlobalEnv$.Random.seed
# Yikes R, you could do better.
with_internal_state <- function(block) {
    # modify random seed
    res_and_state <- with_backed_up_state({
        .GlobalEnv$.Random.seed <- .prng$internal_random_state
        rv <- block  # evaluate block
        rv
    })
    .prng$internal_random_state <- res_and_state$new_state
    res_and_state$result
}

This code seems to work for use in these very specific lines, although its probably quite brittle and might benefit from some error handling.

2. Use an isolated PRNG package

For example: dqrng has a nice drop-in subsitute dqsample and so the minimal changes would be to alter the lines to

name <- paste0('agg_capture_', dqrng::dqsample(.Machine$integer.max, 1))

etc. Of course, this requires changing package dependencies, which may be undesirable. Nonetheless, it seems to work.

3. Device Counter

Its unclear to me if these names need to be random at all or rather just non-colliding. Presumably one could just keep an internal state counter and do something like:

.agg_id <- new.env(parent = emptyenv())
.agg_id$n <- 0L

agg_name <- function(base_string) {
  .agg_id$n <- .agg_id$n + 1L
  paste0(base_string, "_", .agg_id$n)
}

and then update the lines to something like:

name <- agg_name('agg_capture')

4. tempfile hack

If for some reason we want some non-determinism and also want to avoid collisions one can use the old tempfile hack, and instead do something like:

agg_name <- function(base_string) {
  basename(tempfile(paste0(base_string, "_")))
}

A bit hacky, but works.

---

My assumption is that these identifiers do not need to be unpredictable, just non-colliding among active devices, so (3) seems ideal. If there’s any concern about collisions across processes, (4) may be acceptable.

I'm happy to try to put together a PR if people want to weigh-in on the best approach. This is currently causing (what I consider to be) strange and difficult-to-debug behavior in canoncial pipelines e.g. containerized notebooks like quay.io/jupyter/datascience-notebook:latest so I'd like to help get it rectified.

[flying-sheep]

I made my script more robust (edited above), but it still behaves weird when e.g. setting a different RNG using kind = ….

Your other options are less code to maintain and therefore better.

The tempfile hack is documented to use mkdtemp (or an equivalent implementation), which on linux uses {v,}getrandom so I think we could be reasonably sure it won’t ever set .Random.seed

[gjhunt]

Thanks. I try to work on a PR this week for option (3) above, since a sequential ID seems the least hacky.

Edit: Decided to go the tempfile approach since it is stateless and necessitated a very minimal modification to the code.

[thomasp85]

Fixed by #212