wlandau / crew

A distributed worker launcher
https://wlandau.github.io/crew/
Other
123 stars 4 forks source link

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

Closed shikokuchuo closed 1 year ago

shikokuchuo commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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.