vtraag / louvain-igraph

Implementation of the Louvain algorithm for community detection with various methods for use with igraph in python.
GNU General Public License v3.0
246 stars 46 forks source link

Optimiser.consider_empty_communities should be initialized #36

Closed ragibson closed 6 years ago

ragibson commented 6 years ago

Optimiser::consider_empty_communities is never initialized (unless Optimiser.consider_empty_community is explicitly set from Python).

Hence, the if statement on line 402 of Optimiser.cpp is undefined behavior.

In fact, while Optimiser.py states that this value defaults to True, it is often 0.

I've also seen consider_empty_communities evaluate to 32732 and 1081192448 (among many other "True" values), depending on whatever happened to reside in memory at the time.

A fix is to simply set this->consider_empty_community = true; in the Optimiser constructor (if defaulting to True is indeed intended).

For what it's worth, the valgrind error that initially alerted us to this problem (while writing #34) is as follows:

Conditional jump or move depends on uninitialised value(s)
   at: Optimiser::move_nodes(std::vector<MutableVertexPartition*, std::allocator<MutableVertexPartition*> >, std::vector<double, std::allocator<double> >, int, int) (Optimiser.cpp:402)
   by: Optimiser::move_nodes(std::vector<MutableVertexPartition*, std::allocator<MutableVertexPartition*> >, std::vector<double, std::allocator<double> >) (Optimiser.cpp:273)
   by: Optimiser::optimise_partition(std::vector<MutableVertexPartition*, std::allocator<MutableVertexPartition*> >, std::vector<double, std::allocator<double> >) (Optimiser.cpp:116)

Conditional jump or move depends on uninitialised value(s)
   at: Optimiser::move_nodes(std::vector<MutableVertexPartition*, std::allocator<MutableVertexPartition*> >, std::vector<double, std::allocator<double> >, int, int) (Optimiser.cpp:402)
   by: Optimiser::move_nodes(std::vector<MutableVertexPartition*, std::allocator<MutableVertexPartition*> >, std::vector<double, std::allocator<double> >) (Optimiser.cpp:273)
   by: Optimiser::optimise_partition(std::vector<MutableVertexPartition*, std::allocator<MutableVertexPartition*> >, std::vector<double, std::allocator<double> >) (Optimiser.cpp:116)
   by: Optimiser::optimise_partition(MutableVertexPartition*) (Optimiser.cpp:46)
   by: _Optimiser_optimise_partition (python_optimiser_interface.cpp:76)