Skip to content

Conversation

@ilia-kats
Copy link
Collaborator

@ilia-kats ilia-kats requested a review from gtca August 15, 2025 14:36
@ilia-kats ilia-kats requested a review from ilan-gold October 9, 2025 16:10
Copy link
Contributor

@ilan-gold ilan-gold left a 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)
@ilia-kats
Copy link
Collaborator Author

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
Copy link
Contributor

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

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.

random_state=0 does not set seed in muon.tl.leiden, either a bug or unclear documentation

2 participants