markov_clustering, possible error in instructions for computing of modularity Q

[paolotgithub]

In calculating modularity Q I get the following error in my script that uses the markov_clustering package:

Traceback (most recent call last):
  File "mcl_3_inflat_test.py", line 48, in <module>
    Q = mc.modularity(matrix=result, clusters=clusters)
  File "/usr/local/lib/python3.7/site-packages/markov_clustering/modularity.py", line 74, in modularity
    matrix = convert_to_adjacency_matrix(matrix)
  File "/usr/local/lib/python3.7/site-packages/markov_clustering/modularity.py", line 39, in convert_to_adjacency_matrix
    coeff = max( Fraction(c).limit_denominator().denominator for c in col )
TypeError: 'float' object is not iterable

Shouldn't

Q = mc.modularity(matrix=result, clusters=clusters)

as it reads in the instructions be instead:

Q = mc.modularity(matrix, clusters=clusters) ?

Or otherwise...?

Thanks a lot.

[grgmiller]

I also experienced this issue when running

for inflation in [i / 10 for i in range(15, 26)]:
    result = mc.run_mcl(matrix, inflation=inflation)
    clusters = mc.get_clusters(result)
    Q = mc.modularity(matrix=result, clusters=clusters)
    print("inflation:", inflation, "modularity:", Q)

If you remove the [0] from the following line, the code seems to work, but I'm not sure if that is functionally correct (i.e. if you're supposed to be plugging int the result matrix or the original matrix as @plttvmt suggested)

markov_clustering/markov_clustering/modularity.py

Line 37 in 28787cf

col = matrix[:,i].T.tolist()[0]

[BerkeAltiparmak]

Problem

I've noticed that this issue still exists when matrix is a numpy array, so I proceeded to confirm whether @grgmiller 's approach is functionally correct, as he shared his concerns. Here is my procedure to confirm it.

Procedure

1. First, create a new file called modularity.py (or whatever name you want to give to it).
2. Copy-paste the contents of the modularity.py file from https://github.com/GuyAllard/markov_clustering/blob/28787cf64ef06bf024ff915246008c767ea830cf/markov_clustering/modularity.py
3. Make the changes as proposed by @grgmiller, which is removing [0] from the line col = matrix[:,i].T.tolist()[0]
4. Create a new file to test if this approach actually works.
5. You can use this code to test the approach:

6. You can compare the outputs and notice that the one that uses the original approach as a sparse matrix gives approximately equal result to this approach that works with numpy arrays.

Conclusion

The solution provided by @grgmiller works for similarity matrices that are of numpy arrays type.

[cpouchon]

Dear all,
how can you import modularity from your modularity.py file ?
I give this error:
7 from itertools import permutations
9 from scipy.sparse import isspmatrix, dok_matrix, find
---> 10 from .mcl import sparse_allclose
12 def is_undirected(matrix):
13 """
14 Determine if the matrix reprensents a directed graph
15
16 :param matrix: The matrix to tested
17 :returns: boolean
18 """

ImportError: attempted relative import with no known parent package

Thank you,
best regards
Charles