Add Louvain community detection algorithm#453
Add Louvain community detection algorithm#453Becheler wants to merge 5 commits intoboostorg:developfrom
Conversation
|
Not sure why this error is occurring only for OSX 11.7 C++14. I assume this is not specific to your code. |
jeremy-murphy
left a comment
There was a problem hiding this comment.
Just a few comments, more later.
| // Revision History: | ||
| // |
There was a problem hiding this comment.
I know embedded revision histories were a thing in the past, but I would prefer to keep all this metadata in git.
There was a problem hiding this comment.
I did not like it either, I'm glad you don't ahah :)
| std::map<VertexDescriptor, WeightType> internal_weights; | ||
| std::map<VertexDescriptor, std::set<VertexDescriptor>> vertex_mapping; |
There was a problem hiding this comment.
We generally try to avoid hard-coding the choice of data structure, especially std::map and std::set, so instead of templating VertexDescriptor and WeightType we should template the whole map type, so users can use boost::unordered_map or some other kind of property map of their own choice.
There was a problem hiding this comment.
Mhhh I see. I felt it was not in the BGL spirit to do so, and Joaquin also mentioned it, but I was not sure how to solve it without making the API heavy. Can we default to a concrete type to simplify user's experience ? Also, is it ok to use boost::unordered_map if it adds the constraint of key_type being hashable ? That was my idea behind using std::map
| std::set<community_type> unique_communities; | ||
| std::map<community_type, vertex_descriptor> comm_to_vertex; | ||
| std::map<vertex_descriptor, std::set<vertex_descriptor>> vertex_to_originals; |
There was a problem hiding this comment.
These should almost certainly be input parameters taken by non-const reference so that the user a) decides their type and b) automatically gets their value at the end.
There was a problem hiding this comment.
So they gain access to the whole hierarchy. Ufff it's a lot of guts leaking out haha
Will do, and tell you in case of problemsm thanks again for your time !
There was a problem hiding this comment.
I could be wrong. But have a look at the astar API for some examples of prior art.
this was not supposed to be commited :)
this was not supposed to be commited :)
Implement #447
But: