futureverse / futureverse/future

Make seed = I(FALSE) escalate to an error if RNG was used

Open
#831 1 comment 0 reactions 0 assignees View on GitHub
RNG
Dominant language
R
Stars
1k
Forks
92
PR merge metrics
No merged PRs in 30d

Description

Currently, the default behavior of declaring use of the random number generator (RNG) is `seed = FALSE`, which triggers a warning if it detects that the future did indeed use the RNG despite not declaring so;

```r
library(future)
f <- future(sample.int(3)) ## defaults to seed = FALSE
v <- value(f)
v
#> [1] 3 2 1
#> Warning message:
#> UNRELIABLE VALUE: Future () unexpectedly generated random numbers without specifying argument 'seed'. There is a risk that those random numbers are not statistically sound and the overall results might be invalid. To fix this, specify 'seed=TRUE'. This ensures that proper, parallel-safe random numbers are produced. To disable this check, use 'seed=NULL', or set option 'future.rng.onMisuse' to "ignore". [future (54b70f088e2c65b835aea5f0cd100eff-1); on 54b70f088e2c65b835aea5f0cd100eff@hb-x1-2023<867157>]
```

We can escalate these type of warnings to errors globally by setting R option `future.rng.onMisuse` as in:

```r
library(future)
options(future.rng.onMisuse = "error")
f <- future(sample.int(3))
v <- value(f)
#> Error: UNRELIABLE VALUE: Future () unexpectedly generated random numbers without specifying argument 'seed'. There is a risk that those random numbers are not statistically sound and the overall results might be invalid. To fix this, specify 'seed=TRUE'. This ensures that proper, parallel-safe random numbers are produced. To disable this check, use 'seed=NULL', or set option 'future.rng.onMisuse' to "ignore". [future (54b70f088e2c65b835aea5f0cd100eff-2); on 54b70f088e2c65b835aea5f0cd100eff@hb-x1-2023<867157>]
```

The problem with this approach is that options are global and affects all codes, even where we don't want a strict check. Also,none of the future R options should be set developers - only end-users for troubleshooting etc. One can of course set options temporarily to control this, but it adds clutter and is error prone.

A better way to give the developer control of the severity of RNG misuse is via the `seed` argument itself. Here are a few approaches:

* `seed = structure(FALSE, onMisuse = "error")`
* `seed = I(FALSE)` - "as-is"

We could also move towards making `seed = FALSE` be strict and give an error, and `seed = NA` to be the current soft warning. However, such a migration is a much bigger task given that we'll break some existing code.

Contributor guide

Open the contributing guide

Research direction

No files or tests are named. Start by tracing the future() seed argument and the existing future.rng.onMisuse handling; compare the proposed per-call severity forms and define compatibility with seed=FALSE and global options. Done means the selected behavior is specified and verified by tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
r
Domain
distributed-systems
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.