Adding "networkit" flavour to scanpy.tl.leiden for parallel community detection#4170
Adding "networkit" flavour to scanpy.tl.leiden for parallel community detection#4170amalia-k510 wants to merge 3 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4170 +/- ##
==========================================
+ Coverage 79.61% 79.70% +0.08%
==========================================
Files 120 121 +1
Lines 12786 12972 +186
==========================================
+ Hits 10180 10339 +159
- Misses 2606 2633 +27
Flags with carried forward coverage won't be shown. Click here to find out more.
|
ilan-gold
left a comment
ilan-gold
left a comment
There was a problem hiding this comment.
Nice! Would be best to run the installation check :)
Also please add an extra to our pyproject.toml.
Let's try to use a heuristic about warning users about large inputs to igraph and the faster alternative
| raise ImportError(msg) | ||
|
|
||
|
|
||
| def ensure_network() -> None: |
There was a problem hiding this comment.
Where is this used?
| def ensure_network() -> None: | |
| def ensure_networkit() -> None: |
|
|
||
|
|
||
| def ensure_network() -> None: | ||
| if importlib.util.find_spec("netowrkit"): |
There was a problem hiding this comment.
| if importlib.util.find_spec("netowrkit"): | |
| if importlib.util.find_spec("networkit"): |
| seed = int(rng.integers(np.iinfo(np.int64).max)) | ||
| networkit.setSeed(seed, useThreadId=True) |
There was a problem hiding this comment.
Despite this, things aren't deterministic, right? Just making sure, but if that's the case, please comment why we do this (helps with determinism even if it's not perfect maybe?)
| networkit.setSeed(seed, useThreadId=True) | ||
| # only undirected for Parallel Leiden | ||
| g = _utils.get_networkit_from_adjacency(adjacency, weighted=use_weights) | ||
| iterations = n_iterations if n_iterations > 0 else 3 |
There was a problem hiding this comment.
Where did the number 3 come from?
There was a problem hiding this comment.
I checked the the NetworKit's header file for the ParallelLeiden and found that default for ParallelLeiden.iterations is set to three; hence why I decided to use it here as well.
explicit ParallelLeiden(const Graph &graph, int iterations = 3, bool randomize = true, double gamma = 1);
There was a problem hiding this comment.
Can we document that? Also, why isn't it the default then for python?
|
Also can you point this branch at networkit/networkit#1422 (and then |
| if importlib.util.find_spec("networkit") is None: | ||
| return |
There was a problem hiding this comment.
Would still warn even if there is no networkit installed :)
Do you have a benchmarking notebook as a gist somewhere?
There was a problem hiding this comment.
I think this is already handled. It returns early if find_spec("networkit") is None, so the hint only fires when networkit's actually installed. Might've been from before I pushed the guard? Let me know if you're still seeing it warn without networkit and I'll take another look.
2 participants
This PR adds NetworKit's ParallelLeiden as a new flavor option for
scanpy.tl.leiden, enabling multithreaded Leiden community detection viascanpy.tl.leiden(adata, flavor="networkit"). The motivation behind is that scanpy's current Leiden backends (igraph, leidenalg) are single-threaded. For atlas-scale datasets (500k+ cells), clustering becomes a bottleneck. NetworKit's ParallelLeiden is a C++ parallel implementation available via pip with no additional compilation, making it the lowest-friction path to parallel Leiden in scanpy.