fix: use weights to calculate modularity, fix umap tests#4045
fix: use weights to calculate modularity, fix umap tests#4045flying-sheep merged 5 commits intomainfrom
Conversation
flying-sheep
changed the title
flying-sheep
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4045 +/- ##
=======================================
Coverage ? 78.50%
=======================================
Files ? 117
Lines ? 12750
Branches ? 0
=======================================
Hits ? 10009
Misses ? 2741
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more.
|
flying-sheep
changed the title
| "patsy", | ||
| "pynndescent>=0.5.13", | ||
| "scikit-learn>=1.4.2", | ||
| "scikit-learn>=1.6", |
There was a problem hiding this comment.
required by umap 0.5.12
| igraph_mode: str = ig.ADJ_DIRECTED if is_directed else ig.ADJ_UNDIRECTED | ||
| graph: ig.Graph = ig.Graph.Weighted_Adjacency(connectivities, mode=igraph_mode) | ||
| return graph.modularity(_codes(labels)) | ||
| return graph.modularity(_codes(labels), "weight") |
There was a problem hiding this comment.
Can you explain this a bit? Why was this broken before?
There was a problem hiding this comment.
the Graph.Weighted_Adjacency() class method creates a graph with a "weights" edge attribute (parameter default attr='weight'), using the values of the passed sparse matrix as edge weights.
the Graph.modularity() instance method has a weights: str | array | None = None parameter that specify which edge weights to take into account when calculating the modularity.
In #3613 we forgot to specify that.
flying-sheep
changed the title
2 participants
This gets rid of two numba related test skips:
fastmath=Trueis used in incompatible functions (breaks in numba 0.62.0rc1+) lmcinnes/umap#1216 (fixed in numba 0.5.12)SET_FUNCTION_ATTRIBUTEargument 16 (“annotate”) numba/numba#10319 (broken in numba 0.62–0.63rc1 or so, so not present in any of our jobs)I’ll check if we can backport this or if it pushes the minimum versions too far up