Conversation
|
It would be great to get this algorithm in! |
dschult
left a comment
There was a problem hiding this comment.
This looks good. I'm not sure how effective it will be because it is slow -- but the doc_string warns that it is slow. So I think that is up to the user to determine.
My comments are minor things. It will also need "black". And it doesn't test the normalized and weight optional arguments. (They are tested in the tests for betweenness_centrality, so maybe that's OK.) I also wondered if the normalized parameter makes any difference because the maximum is all that is used, and the max shouldn't depend on normalization... but that's probably OK too.
|
I've been looking at this due to Jarrod's comment above that we try to get it in soon. This overlaps slightly with the issue of whether the values should be normalized or not. Since we are going to have to compute the betweenness values separately for each component, and then compare them to find the highest betweenness edge. If they are normalized based on the component, they will have different normalization factors. So, I'm thinking that we should not use normalized scores. [Even in the case of edge rankings that don't require connected graphs, normalizing should not affect which edge is maximal. So there is no reason the user should need to specify whether we normalize the ranks or not. And that also reduces the input arguments by one -- Yay! :} I'm close to making these changes to this PR, but if you have an idea or perspective that would help please let me know. Also, I'm thinking I can push directly to the PR because it is fixing the test errors and this is a rewrite of a previous PR. If anyone would prefer that I make a PR to the branch that is this PR, let me know. |
649cca0 to
9e15b13
Compare
dschult
left a comment
There was a problem hiding this comment.
This looks ready to merge.
The tracking of partitions was implemented a year ago, but I never marked this as ready to review.
|
Just a quick ping here @MridulS - if there are no objections to making |
Co-authored-by: Ross Barnowski <rossbar@berkeley.edu>
dschult
left a comment
There was a problem hiding this comment.
This looks good to me!
:)
Thanks!
* Divisive community algorithms * Cleanup divisive community algos * Change number_of_partitions to number of sets, doc_string tweaks, examples, etc * Rewrite so CF Btwness is called on each component separately * importorskip numpy for edge_current_flow_betweenness * fix isort trouble * Apply suggestions from code review Co-authored-by: Ross Barnowski <rossbar@berkeley.edu> * FEAT: adding a PoC of creating an internal cache --------- Co-authored-by: Benjamin Edwards <BJEdwards@gmail.com> Co-authored-by: Dan Schult <dschult@colgate.edu> Co-authored-by: Ross Barnowski <rossbar@berkeley.edu>
This PR brings in the algorithms implemented in #764, cleaned up a bit.