-
Notifications
You must be signed in to change notification settings - Fork 33
make random state argument handling more consistent#166
make random state argument handling more consistent #166ilia-kats wants to merge 1 commit intoscverse:main from
Conversation
@ilia-kats
ilia-kats
commented
Aug 14, 2025
- always default to non-None random_state (consistent within muon and with scanpy)
- fix condition before setting random state in .tl.leiden (closes random_state=0 does not set seed in muon.tl.leiden, either a bug or unclear documentation #154 )
@ilan-gold
ilan-gold
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is this not a breaking change for reproducibility given that you're changing the default? If you didn't set the random seed by default now in each of these, you're setting it every time now to 0. I think handling random state this way is a separate issue
- always default to non-None random_state (consistent within muon and with scanpy) - fix condition before setting random state in .tl.leiden (closes scverse#154)
d4e91a1 to
e06e875
Compare
ilia-kats
commented
Oct 10, 2025
We already weren't reproducible before, unless someone set their global random seed at the very beginning of the analysis, which very few people do. I would think this improves reproducibility. But I guess we can defer that change to muon 0.2 or something, with an explicit warning in the release notes.
ilan-gold
commented
Oct 10, 2025
Up to you! If the goal is to just "fix the issue" I think checking is not None is sufficient but otherwise this change seems reasonable