-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/exe 1884 make community detection deterministic #176
Feature/exe 1884 make community detection deterministic #176
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #176 +/- ##
=======================================
Coverage 81.78% 81.78%
=======================================
Files 121 121
Lines 6795 6795
=======================================
Hits 5557 5557
Misses 1238 1238 ☔ View full report in Codecov by Sentry. |
51b4d5b
to
0c964e5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me. Great work!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of comments but nothing major. Please, read my comments on the ticket.
@@ -221,7 +221,7 @@ def annotate_components( | |||
# components clustering | |||
if adata.n_obs > MINIMUM_NBR_OF_CELLS_FOR_ANNOTATION: | |||
# perform the unsupervised clustering | |||
cluster_components(adata=adata, inplace=True) | |||
cluster_components(adata=adata, inplace=True, random_seed=1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why the different random_seed here and in _networkx?
@@ -305,6 +305,7 @@ def community_leiden( | |||
graph, | |||
use_modularity=True, | |||
randomness=beta, | |||
random_seed=42, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we use the same see everywhere? i.e. 1477
:)
Superseded by PR 188. |
Setting a random seed for leiden algorithm so that component IDs remain constant through multiple runs of pipeline.
Fixes: #EXE-1884
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
PR checklist: