Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure reproducible if using model cache #470

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

gowerc
Copy link
Collaborator

@gowerc gowerc commented Jan 8, 2025

Closes #469

FYI @luwidmer

@gowerc gowerc requested a review from gravesti January 8, 2025 16:50
@luwidmer
Copy link

luwidmer commented Jan 9, 2025

@gowerc doesn't .Random.seed always exist? At least I get

Restarting R session...

> exists(".Random.seed")
[1] TRUE
> ls()
character(0)
> .Random.seed <<- .Random.seed
> ls()
character(0)
> exists(".Random.seed")
[1] TRUE

@gravesti
Copy link
Collaborator

gravesti commented Jan 9, 2025

I think it doesn't necessarily exist. eg if I start a new R session from the terminal

gravesti$ R

R version 4.4.1 (2024-06-14) -- "Race for Your Life"
Copyright (C) 2024 The R Foundation for Statistical Computing
Platform: aarch64-apple-darwin20
... snip ...

> .Random.seed
Error: object '.Random.seed' not found
> rnorm(1)
[1] -0.6881252
> .Random.seed
  [1]       10403           2 -1831660992  -462780898 -1349857047   996995611
  [7]   676991530   594638156 -1127753969  1257593413  1702644732  1213026898
 [13]  -333959219  -261150329 -1072259986   635315272  1119354443    93778857
 [19]   864664312   193269286  -542546111 -1416680237  1130410114   418352820
 [25]   -98325961  -999378643 -1509490108  1850580842   640379861   544927087

R/utilities.R Outdated Show resolved Hide resolved
R/utilities.R Outdated Show resolved Hide resolved
@luwidmer
Copy link

luwidmer commented Jan 9, 2025

Yup, turns out I didn't have a completely new R session even after restarting! Thank you for testing @gowerc & @gravesti ! IMO the on.exit() solution (when .Random.seed exists) would be a pretty robust one.

@gowerc
Copy link
Collaborator Author

gowerc commented Jan 9, 2025

Yup, turns out I didn't have a completely new R session even after restarting! Thank you for testing @gowerc !

In fairness I had no idea it wasn't created on new R sessions until all the CICD tests failed unexpectedly 😓

@gowerc
Copy link
Collaborator Author

gowerc commented Jan 10, 2025

@gravesti - Ok should be good for review now. I adapted the code you shared a little bit as I realised when the processx update goes on CRAN the rm statement might throw an error as the seed may not have ever been created or updated so we need to conditionally check if it exists to handle both the old and the new/upcoming behaviours.

@gowerc gowerc requested a review from gravesti January 10, 2025 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rbmi 1.3.1: first fit after compiling model not reproducible with subsequent fits
3 participants