feat: add networkit flavor for parallel Leiden clustering#4170
feat: add networkit flavor for parallel Leiden clustering#4170amalia-k510 wants to merge 11 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4170 +/- ##
==========================================
- Coverage 79.91% 79.74% -0.17%
==========================================
Files 121 121
Lines 12949 12989 +40
==========================================
+ Hits 10348 10358 +10
- Misses 2601 2631 +30
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
| 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?
There was a problem hiding this comment.
I checked and turns out that iterations=3 is already the default on the Python side for ParallelLeiden. I could drop it, but I left it explicit so the value is obvious at the call site and clearly maps to scanpy's n_iterations.
|
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.
There was a problem hiding this comment.
Also, I tried to create gist, and it is erroring out on the notebook. I think the file is too large with the embedded outputs. Want me to push it to the PR branch, strip the outputs and re-upload to a gist, or just share it another way?
for more information, see https://pre-commit.ci
…into networkit_gve
amalia-k510
changed the title
for more information, see https://pre-commit.ci
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.