vtraag / leidenalg

Implementation of the Leiden algorithm for various quality functions to be used with igraph in Python.
GNU General Public License v3.0
596 stars 78 forks source link

max_comm_size error #50

Closed PatrickPata closed 3 years ago

PatrickPata commented 3 years ago

Hi,

Thank you for continuously developing this algorithm! I previously used the louvain algorithm to analyze my thesis and now I am doing the reanalysis with the leiden algorithm and multiplex. I stumbled into this error in which the partitioning with multiplex requires a max_comm_size (first cell in photo) but when I add any max_comm_size value (second cell in photo) it does not recognize this as an argument. This is not an issue with the regular partitioning though.

I solved this for my purposes by removing the max_comm_size statements in lines 100 and 164. I'm sure there is a better way to address the bug.

Cheers, Patrick

max_comm_size_error

TomKellyGenetics commented 3 years ago

I get this error as well. When calling find_partition_multiplex only. Not when calling find_partition (which passes defaults of max_comm_size=0) in version 0.8.2. I'm calling it via reticulate in R but I think the issue is in the python side.

Error in py_call_impl(callable, dots$args, dots$keywords) : 
  NameError: name 'max_comm_size' is not defined

Detailed traceback: 
  File "/Users/tom/Library/r-miniconda/envs/r-reticulate/lib/python3.8/site-packages/leidenalg/functions.py", line 164, in find_partition_multiplex
    optimiser.max_comm_size = max_comm_size;

The "max_comm_size" parameter is defined by the optimiser but not passed to find_partition_multiplex`.

    library("reticulate")
    leidenalg <- import("leidenalg", delay_load = TRUE)
    self.optimiser = leidenalg$Optimiser()
    self.optimiser$max_comm_size
    #> [1] 0

Created on 2020-11-06 by the reprex package (v0.3.0)

TomKellyGenetics commented 3 years ago

I've managed to identify the issue. Default arguments are missing here:

src/functions.py:100

- def find_partition_multiplex(graphs, partition_type, n_iterations=2, seed=None, **kwargs):
+ def find_partition_multiplex(graphs, partition_type, n_iterations=2, max_comm_size=0, seed=None, **kwargs):
TomKellyGenetics commented 3 years ago

Correcting this and reinstalling from source fixes the issue for me:

python setup.py bdist_wheel 
python -m pip install dist/leidenalg-0.7.0.post2.dev207+g0734bff.d20201106-cp38-cp38-macosx_10_9_x86_64.whl
TomKellyGenetics commented 3 years ago

Here's a successful test job on the updated version.

python 3.8.6 | packaged by conda-forge | (default, Oct  7 2020, 18:42:56) 
[Clang 10.0.1 ] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import igraph as ig
>>> import leidenalg as la
>>> G1 = ig.Graph.Famous("Zachary")
>>> G2 = ig.Graph.Famous("Zachary")
>>> optimiser = la.Optimiser()
>>> membership, improv = la.find_partition_multiplex([G1, G2], la.ModularityVertexPartition);
>>> membership
[1, 1, 1, 1, 3, 3, 3, 1, 0, 0, 3, 1, 1, 1, 0, 0, 3, 1, 0, 1, 0, 1, 0, 2, 2, 2, 0, 2, 2, 0, 0, 2, 0, 0]