Skip to content
This repository has been archived by the owner on Oct 8, 2021. It is now read-only.

Function modularity should consider directed graphs #1066

Closed
vgauthier opened this issue Oct 26, 2018 · 8 comments
Closed

Function modularity should consider directed graphs #1066

vgauthier opened this issue Oct 26, 2018 · 8 comments

Comments

@vgauthier
Copy link
Contributor

The definition of the modularity could easily be extended to directed graphs (cf. ref. [1]) with minor tweak of the original algorithm.

[1] E. A. Leicht and M. E. J. Newman. Community structure in directed networks. Physical Review
Letter, 100:118703, 2008. (PDF)

@vgauthier vgauthier changed the title function modularity should consider directed graphs Function modularity should consider directed graphs Oct 26, 2018
@jpfairbanks
Copy link
Contributor

This would make a good hacktoberfest PR.

@simonschoelly
Copy link
Contributor

Would it be a good idea to also add optional weights to this algorithm?

@vgauthier
Copy link
Contributor Author

I would be nice addon indeed, but LightGraphs is not suppose to handle weighted graphs right ?

How do you propose to handle the weighted case: by the SimpleWeightedGraphs or by passing an array of weights to the function ?

@jpfairbanks
Copy link
Contributor

You would take a keyword argument with the weights which would default to weights(g). For SimpleGraph the weight(g) is a struct that represents the matrix ones(Int, nv(g), nv(g)).

@vgauthier
Copy link
Contributor Author

vgauthier commented Oct 29, 2018

@jpfairbanks Thank for your advice, I am almost done with the implementation of the modularity for directed and weighted graphs. However, for the weighted case I need to compute the sum of the weight of a graph. This operation is okay when I use SimpleWeightedGraph

julia> g = SimpleWeightedGraph(3);
julia> add_edge!(g, 1, 2, 1);
julia> add_edge!(g, 2, 3, 1);
julia> add_edge!(g, 3, 1, 1);
julia> barbell = blockdiag(g, g);
julia> add_edge!(barbell, 1, 4, 5);

julia> sum(weights(g))
22

However when I do the same operation with SimpleGraph I get a weird answer (the sum of a matrix where all elements are equals to 1) instead of getting a matrix with a one in all entries where there is an edge and zero elsewhere. Is it an expected behavior ?

julia> g = SimpleGraph(3);
julia> add_edge!(g, 1, 2);
julia> add_edge!(g, 2, 3);
julia> add_edge!(g, 3, 1);
julia> barbell = blockdiag(g, g);
julia> add_edge!(barbell, 1, 4);

julia> sum(weights(g))
36

@jpfairbanks
Copy link
Contributor

yeah that is the expected behavior. It is a little non-intuitive, but the idea was that you are only supposed to use weights(g)[u,v] for known edges in g. We might want to override the definition of sum for that struct.

@vgauthier
Copy link
Contributor Author

@jpfairbanks okay, thanks for the explanation. I finished the implementation of the modularity for directed and weighted case. Do you want me to open a new PR or update the PR I already opened about the modularity ?

@jpfairbanks
Copy link
Contributor

I just reviewed the other PR. I'll merge it into master and then you can base your new PR against master.

sbromberger pushed a commit that referenced this issue Oct 31, 2018
* Update the modularity doc and minor change  

Enhance the documentation with references and example. Add the resolution parameter in the during the computation of the modularity.

* spell check the docstring

* fix a test case

* update the test cases and function signature

update the test cases and the function signature according to @simonschoelly suggestion

* update the docstring

* Update modularity.jl

* modularity for both directed and undirected graphs

* remove clutter 

remove unnecessary print for debug purpose

* modularity should consider directed graphs (#1066)

* remove unnecessary line in docstring
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants