Skip to content
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

Integration of Ben Edwards' GSoC project 2011 on community detection #764

Closed
wants to merge 2 commits into from

Conversation

Midnighter
Copy link

Incomplete Pull Request

This pull request is incomplete and it's meant to discuss pulling @bjedwards'
GSoC project on community finding and other related code into the main networkx repository.

General Remark

There have been several discussions on the mailing list showing interest in
replacing the basic Graph data structure with custom ones, e.g., using a
database instead of the dict of dict model. Should we rely on particular class
interfaces, for example, Graph.adjacency_iter or DiGraph.predecessors_iter,
instead of accessing the dictionary structures directly in order to facilitate
other data models?

Direct access of the dictionary structure is extremely
widespread throughout networkx; one could argue that any customized data
structure would have to expose a dictionary-like interface anyway because a
rewrite of networkx is not worth the time and effort.

Observations

  • spectrum.py and laplacian_spectral.py are disorganized and seem to serve
    similar purposes. Can we reorganize it?
  • similarly quality.py and community_quality.py isn't all the code community
    related?

Related Issues

The GSoC project issue #549 (used to be Trac 557), other related issues are an algorithm based on Newman(2006) PNAS #160 (Trac 158),
the Louvain method #239 (same on Trac), and a fast detection algorithm #245
(same on Trac).

Moritz Emanuel Beber added 2 commits September 8, 2012 19:24
Added files from @bjedwards' GSoC repository over on bitbucket
(https://bitbucket.org/bedwards/networkx-community) that have tests. Further
files remain but testing needs to be added.
* fixed a few function references and other minor points
* still many references are incomplete or wrong
@ghost ghost assigned bjedwards Sep 10, 2012
@bjedwards
Copy link
Member

Thanks for adding this Moritz. Sorry I've been slow getting this integrated.

  • spectrum.py and laplacian_spectral.py are disorganized and seem to serve similar purposes. Can we reorganize it?

Yes we should. Adding spectral graph theory type algorithms to NetworkX turned out to be a bit more of a big deal than we anticipated. There was also some question about requiring numpy for this vs. providing iterative methods. I think the consensus we came to was that if one wants to do spectral graph theory, then it is a reasonable assumption that numpy is available. We can always raise an exception otherwise. The other alternative was to use scipy_sparse methods if possible, and reasonable.

  • similarly quality.py and community_quality.py isn't all the code community related?

Yes, these both contain community quality measures. I think there was a little bit of a fork between Aric and I's efforts, but I don't think there is overlap. We can simply move all the functions in community_quality.py to quality.py. Could probably do some renaming as well, e.g. community_performance could just be performance

@aweinstein
Copy link
Contributor

Note that pull request #741 adds a function to compute the directed laplacian. This PR is based on @bjedwards code.

@Midnighter
Copy link
Author

I suppose it is only sensible to put as much of the code from algorithms/community/spectrum.py and algorithms/community/laplacian_spectral.py into the fitting linalg modules. Any estimate on when #741 will be merged?

@hagberg
Copy link
Member

hagberg commented Sep 14, 2012

On Fri, Sep 14, 2012 at 2:21 PM, Moritz Emanuel Beber
notifications@github.com wrote:

I suppose it is only sensible to put as much of the code from
algorithms/community/spectrum.py and
algorithms/community/laplacian_spectral.py into the fitting linalg modules.
Any estimate on when #741 will be merged?

It looks like #741 needs some more tests and docs. I'll try to find
time to review that in the next week or so.

@aweinstein
Copy link
Contributor

It looks like #741 needs some more tests and docs. I'll try to find
time to review that in the next week or so.

@hagberg Let me know if can help with that.

@ghost ghost assigned hagberg Dec 29, 2012
@hagberg
Copy link
Member

hagberg commented Mar 6, 2014

Another addition could be "nodal roles" in #1060

@MridulS
Copy link
Member

MridulS commented Jun 29, 2015

@hagberg @Midnighter @bjedwards What is the current status of this project?

@harrymvr
Copy link
Contributor

Just a heads up (particularly for spectrum.py) . The matrices should be handled as scipy.sparse, as in #1021 . Also, the way that linear algebra is coded will be super extra slow.

@bjedwards
Copy link
Member

@MridulS, About the same as it was 2.5 years ago?

#1035 added some of the community graph generators in December of 2013, but I haven't had much time to work on the other stuff.

@harrymvr initially we were avoiding the scipy.sparse dependency, or at least tried to implement it in such a way that we could use either. I am not sure what the current thoughts requiring scipy.sparse. My thought is if you want to do spectral graph theory, it's probably not unreasonable to assume that you have scipy.sparse installed.

@Midnighter
Copy link
Author

I don't have a good overview right now but I know I stopped the merging effort back then because there were some spectral functions being written. So those could probably be used.

@MridulS
Copy link
Member

MridulS commented May 18, 2022

Went through this PR to see the current status, a lot of code has been merged into NetworkX by new PRs.

Some bits of code that is left over:
networkx/algorithms/community/community_quality.py
- community_performance implemented in partition_quality
- community_coverage implemented in partition_quality

networkx/algorithms/community/divisive.py -> #5830
- divisive_partition
- edge_betweenness_partition
- edge_current_flow_betweenness_partition

networkx/algorithms/community/laplacian_spectral.py

networkx/algorithms/community/partition.py

networkx/algorithms/community/random_community.py

networkx/algorithms/community/spectrum.py All of these seem just variations of pagerank, and I'm not sure if we need to add them to networkx.
- stationary_distribution
- lazy_stationary_distribution
- pagerank_stationary_distribution

We don't necessarily have to add all of the other bits of code. I'll go through the remaining bits and open a new PR so we an review it again and get this in NetworkX :)

@dschult
Copy link
Member

dschult commented May 18, 2022

Nice!! There are a lot of pieces there -- but at least we can make a list now. :}

I believe the simple/lazy/pagerank_directed_laplacian are all available now in directed_laplacian_matrix with a keyword argument for which version you might want. (looked at that earlier today while reviewing a different PR). It is not in the community subpackage, but I think is where it should be especially since the proposed laplacian centrality would use it also.

@MridulS
Copy link
Member

MridulS commented Feb 2, 2023

I have updated the above comment with the current state and some opinions about some bits that I don't think we should add to networkx today. I am tagging this with a close ?. We can continue reviews in other open PRs.

@MridulS MridulS added the close ? label Feb 2, 2023
@MridulS MridulS removed this from the networkx-future milestone Feb 2, 2023
@Midnighter
Copy link
Author

Blast from the past 😄

@rossbar
Copy link
Contributor

rossbar commented Feb 13, 2023

+1 for focusing on the separate PRs. For anything that isn't already pulled out, it might be easier to cherry-pick and resubmit that try to wrangle them in this PR. Thanks all!

@rossbar rossbar closed this Feb 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

8 participants