ECG Community Detection implementation#502
ECG Community Detection implementation#502ryandewolfe33 wants to merge 8 commits intoJuliaGraphs:masterfrom
Conversation
Benchmark Results (Julia v1)Time benchmarks
Memory benchmarks
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #502 +/- ##
==========================================
+ Coverage 97.31% 97.46% +0.15%
==========================================
Files 126 127 +1
Lines 7739 7765 +26
==========================================
+ Hits 7531 7568 +37
+ Misses 208 197 -11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
LoveLow-Global
left a comment
LoveLow-Global
left a comment
There was a problem hiding this comment.
Thanks for working on this! Adding ECG is a great feature for the community detection.
I've left a few comments. The two main things to check on are ensuring the ensemble weighting only applies to the 2-core of the graph and a small bug with undirected self-loops.
As this is my first review after being a reviewer here, I may have not given the best review possible. I apologize in advance for this issue, please let me know if there is anything to be improved from my side.
| move_tol=move_tol, | ||
| rng=rng, | ||
| ) | ||
| weights = |
There was a problem hiding this comment.
I noticed an algorithmic detail issue from the weight calculation. According to Equation (1) in the Poulin & Théberge paper (which I found on line 23 of this file), the ensemble co-association adjustment must only be applied to edges within the 2-core of the graph. Any edge outside the 2-core should default to
Currently, if I am not mistaken, it applies the interpolation globally to every edge.
I believe you have to conditionally apply this, using core_number(g).
There was a problem hiding this comment.
Thanks for catching this. Without this step, the implementation (and intuition) works nicely for weighted (parallel edges considered as weighted) and directed graphs, but the 2-core is not so obvious in these cases. Also the core_number(g) function currently does not support self loops.
My proposed (and now implemented) solution is to have a min_weight_outside_2core parameter that calls core_number(g) and decreases all edge weights with at least of vertex with core number < 2 to the minimum weight. This way it can be bypassed for any of the edge cases above, and will throw an error if the core_number function is not supported for the graph type.
However I'm not super happy with this solution, and would be open to more discussion about the edge cases.
2 participants
Another community detection algorithm, see #231
Ensemble Clustering for Graphs (ECG) uses many runs of Louvain (see #488 ) to improve performance and stability.