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

Replace withr::local_seed() with set.seed() #67

Closed
2 of 6 tasks
shikokuchuo opened this issue Apr 27, 2023 · 2 comments
Closed
2 of 6 tasks

Replace withr::local_seed() with set.seed() #67

shikokuchuo opened this issue Apr 27, 2023 · 2 comments
Assignees

Comments

@shikokuchuo
Copy link
Contributor

Prework

Description

I mentioned before, but it probably got lost in other comments. There seems to be no benefit of using withr::local_seed() over set.seed(). I just looked over the code for withr::local_seed() to confirm my understanding, and in the sub function withr:::set_seed it just calls set.seed(). Everything else is just for reverting the seed on exit.

This reverting of the seed is irrelevant as (i) the seed is supplied by crew for all evaluatons anyway and (ii) if mirai is performing globals cleanup then the reverted seed is removed in any case.

I believe in your profiling work in #65, this had a measurable impact. It also reduces one dependency.

@wlandau
Copy link
Owner

wlandau commented Apr 27, 2023

Thanks for the nudge, @shikokuchuo. You are right, there is no point in reverting the seed because crew_eval() always sets a seed, and it never runs in the local R session outside of testing.

@shikokuchuo
Copy link
Contributor Author

Fantastic! Wasn't meant as a nudge at all. I just suddenly remembered in another context and thought I should post this for my own benefit as well - in case there was something subtle about seeds that I had not understood.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants