grunwaldlab / poppr

🌶 An R package for genetic analysis of populations with mixed (clonal/sexual) reproduction
https://grunwaldlab.github.io/poppr
68 stars 26 forks source link

Preserve state of random number generator in .onAttach() #259

Closed TimTaylor closed 8 months ago

TimTaylor commented 8 months ago

Please place an "x" in all the boxes that apply


Currently .onAttach() will change the state of a users random number generator.

https://github.com/grunwaldlab/poppr/blob/2ae5741bb73b4034993dc6c58ec264c379fbd6f5/R/zzz.r#L57

Not a bug per se but would you consider a change to preserve it?

For reference I believe {ggplot2} was updated to use withr::with_preserve_seed() for this reason.

Motivate by this thread https://fosstodon.org/@henrikbengtsson@mastodon.social/112079393660379008


zkamvar commented 8 months ago

Thank you for opening this, Tim. I think a change should definitely happen. I'll probably use Ben's millisecond trick to avoid calling the rng all together.

zkamvar commented 8 months ago

Alas, the RNG is still modified, which means that one of the packages being loaded is modifying it. I tried debugging for set.seed(), but that gave me shiny, which should safely modify its own internal seed.

zkamvar commented 8 months ago

Actually, shiny might be the culprit here.

exists(".Random.seed")
#> [1] FALSE
library(shiny)
exists(".Random.seed")
#> [1] TRUE

Created on 2024-03-13 with reprex v2.0.2

TimTaylor commented 8 months ago

@zkamvar interesting 🤔. Away from pc to take a proper look but seems like they've tried to handle this but it's not quite working. Will have a proper gander when I get a moment.

TimTaylor commented 8 months ago

@zkamvar - Actually I think it's ok. In a vanilla session there is no seed to preserve.

$ R -s -e "runif(1); withr::with_preserve_seed(runif(1)); library(shiny); runif(1)"
[1] 0.455732
[1] 0.5350188
[1] 0.5350188

but even without shiny, you cannot preserve a seed that doesn't exist

$ R -s -e "exists('.Random.seed'); withr::with_preserve_seed(runif(1)); runif(1); exists('.Random.seed')"
[1] FALSE
[1] 0.2837904
[1] 0.02483963
[1] TRUE

So I think shiny is ok. Does that make sense?

zkamvar commented 8 months ago

Ah yeah that does make sense! Thank you!