Skip to content

fix: handle random_state=0 correctly in leiden clustering#184

Open
Ekin-Kahraman wants to merge 2 commits intoscverse:mainfrom
Ekin-Kahraman:fix/random-state-zero-leiden
Open

fix: handle random_state=0 correctly in leiden clustering#184
Ekin-Kahraman wants to merge 2 commits intoscverse:mainfrom
Ekin-Kahraman:fix/random-state-zero-leiden

Conversation

@Ekin-Kahraman
Copy link
Copy Markdown

Summary

  • Fix falsy check on random_state in _cluster() that caused random_state=0 to skip setting the RNG seed
  • Changed if random_state: to if random_state is not None: so that 0 (the default) correctly calls optimiser.set_rng_seed(0)
  • Added regression tests verifying seed is set for random_state=0 and skipped for random_state=None

Root cause

In muon/_core/tools.py line 1005, the condition if random_state: evaluates to False when random_state=0 because 0 is falsy in Python. This meant the default value (random_state=0) and any explicit random_state=0 call never set the RNG seed, producing non-deterministic clustering results.

Closes #154

Test plan

  • test_leiden_random_state_zero_sets_seed — verifies set_rng_seed(0) is called when random_state=0
  • test_leiden_random_state_none_skips_seed — verifies set_rng_seed is not called when random_state=None

Ekin-Kahraman and others added 2 commits April 8, 2026 02:19
`random_state=0` was not setting the RNG seed because the check used
`if random_state:` which evaluates to False when random_state is 0.
Changed to `if random_state is not None:` so that any integer value
(including 0) correctly calls `optimiser.set_rng_seed()`.

Closes scverse#154

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…talled

The CI environment does not have leidenalg. The previous approach used
unittest.mock.patch which requires the target module to exist. Now we
inject a mock leidenalg module into sys.modules before calling
mu.tl.leiden, so the `import leidenalg` inside the function succeeds.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Ekin-Kahraman
Copy link
Copy Markdown
Author

CI status update:

I've pushed a fix for the test failures in tests/test_leiden_random_state.py — the tests were using unittest.mock.patch("leidenalg.Optimiser") which requires leidenalg to be installed, but CI doesn't have it. Now the tests mock leidenalg at the sys.modules level so they work without the package installed.

The remaining CI failures are pre-existing issues unrelated to this PR:

  • Lint (Black): muon/_atac/preproc.py and muon/_atac/tools.py need reformatting under black==26.3.0. These files are not touched by this PR.
  • Build (test_muon_preproc.py): 8 failures in TestInPlaceFiltering with TypeError: 'mappingproxy' object does not support item assignment at muon/_core/preproc.py:827. Also not touched by this PR.

Our change is a single-line fix in muon/_core/tools.py line 1005 (if random_state:if random_state is not None:). Happy to rebase if needed.

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

1 participant