Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

implement louvain algorithm #183

Merged
merged 18 commits into from
Mar 25, 2021
Merged

implement louvain algorithm #183

merged 18 commits into from
Mar 25, 2021

Conversation

acezen
Copy link
Collaborator

@acezen acezen commented Mar 10, 2021

What do these changes do?

implement a distributed louvain algorithm

Related issue number

Fixes #182

@acezen
Copy link
Collaborator Author

acezen commented Mar 10, 2021

community quality test

compare the modularity of result partition with https://github.com/taynaud/python-louvain

dataset: p2p (62586 nodes, 147892 edges)

graphscope louvain: 0.571986 find community num: 103
python-louvain: 0.578780 find community num: 110

com-youtube dataset (1134890 nodes, 2987624 edges), community num: 8,385

graphscope louvain: 0.717838, find community_num: 4980
python-louvain: 0.7204, find community num: 5340

@acezen acezen changed the title implement louvain algorithm [WIP] implement louvain algorithm Mar 10, 2021
@acezen
Copy link
Collaborator Author

acezen commented Mar 18, 2021

performance test

dataset: com-friendster ( 65,608,366 nodes, 1,806,067,135 edges)
tolerance=0, min_progress=1

parallel GraphScope louvain (8 worker)

time: 7 min (remove the atomic operation, but bring in a three-level vector buffer to process messages to prevent thread conflict.)

grape louvain( 8 worker, 128 frag)

time: 5 min

@acezen acezen changed the title [WIP] implement louvain algorithm implement louvain algorithm Mar 18, 2021
@acezen acezen marked this pull request as ready for review March 18, 2021 12:21
Copy link
Collaborator

@siyuan0322 siyuan0322 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it's will be open sourced, please revise the code style, such as function naming (Camel Style), class encapsulation, variable naming, comments, fixme, non-necessary header and so on.

Maybe you could add a python test case for louvain, mainly to demonstrate it's usage, and the format of result.
Reference: https://google.github.io/styleguide/cppguide.html

analytical_engine/apps/pregel/louvain/auxiliary.h Outdated Show resolved Hide resolved
analytical_engine/apps/pregel/louvain/louvain_app_base.h Outdated Show resolved Hide resolved
analytical_engine/core/app/pregel/pregel_vertex.h Outdated Show resolved Hide resolved
@acezen
Copy link
Collaborator Author

acezen commented Mar 19, 2021

Since it's will be open sourced, please revise the code style, such as function naming (Camel Style), class encapsulation, variable naming, comments, fixme, non-necessary header and so on.

Maybe you could add a python test case for louvain, mainly to demonstrate it's usage, and the format of result.
Reference: https://google.github.io/styleguide/cppguide.html

Hi, @siyuan0322, thanks for reviewing, could you quote some bad code examples as a hint, that would be much help.

@codecov-io
Copy link

codecov-io commented Mar 19, 2021

Codecov Report

Merging #183 (5a19fcc) into main (7a8f3f6) will decrease coverage by 0.55%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #183      +/-   ##
==========================================
- Coverage   78.91%   78.35%   -0.56%     
==========================================
  Files          52       53       +1     
  Lines        4767     4986     +219     
==========================================
+ Hits         3762     3907     +145     
- Misses       1005     1079      +74     
Impacted Files Coverage Δ
python/graphscope/analytical/app/__init__.py 100.00% <100.00%> (ø)
python/graphscope/analytical/app/louvain.py 100.00% <100.00%> (ø)
...n/graphscope/deploy/kubernetes/resource_builder.py 57.65% <0.00%> (-6.06%) ⬇️
python/graphscope/framework/graph_utils.py 84.05% <0.00%> (-4.79%) ⬇️
python/graphscope/framework/errors.py 92.64% <0.00%> (-1.48%) ⬇️
python/graphscope/client/session.py 72.30% <0.00%> (-1.41%) ⬇️
python/graphscope/interactive/query.py 89.28% <0.00%> (-0.72%) ⬇️
python/graphscope/config.py 100.00% <0.00%> (ø)
python/graphscope/client/rpc.py 83.70% <0.00%> (ø)
python/graphscope/dataset/ldbc.py 100.00% <0.00%> (ø)
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7a8f3f6...5a19fcc. Read the comment docs.

@acezen acezen force-pushed the zwb/louvain branch 2 times, most recently from 4684fc5 to 31aaf25 Compare March 22, 2021 01:49
acezen added 2 commits March 22, 2021 09:50
add louvain context and vertex

add louvain test

update

update

fix

fix

fix

update

update

update

compiled

update

runnable

runable

update

update

add louvain to client

update

adapt to ArrowProjectedFragment

sync community after comm ready

vd_t be oid_t

use SendToFragment

runnable of p2p n = 4

fix

use parallel worker

fix sendp2p and aggregate

fix pallell workers

update

update

cleanup

parallel processing messages

revise parallel process messages

clean up code

remove halted aggregator

edge weight as double

format

update

update

fix

update

revise code style

remove unnecessary virtual declare

update
Copy link
Collaborator

@lidongze0629 lidongze0629 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

analytical_engine/apps/pregel/louvain/louvain_vertex.h Outdated Show resolved Hide resolved
analytical_engine/apps/pregel/louvain/louvain_vertex.h Outdated Show resolved Hide resolved
analytical_engine/apps/pregel/louvain/louvain_vertex.h Outdated Show resolved Hide resolved
@@ -89,8 +98,7 @@ class PregelComputeContext {
message_manager_->SyncStateOnOuterVertex<fragment_t, MD_T>(*fragment_,
v, value);
} else {
messages_in_[v].emplace_back(value);
has_messages_ = true;
messages_out_[v].emplace_back(value);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lidongze0629 Any comment on this part?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Copy link
Collaborator

@sighingnow sighingnow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I'm also ccing @lidongze0629 for a quick review on the pregel app base related changes.

@sighingnow sighingnow requested a review from lidongze0629 March 25, 2021 02:16
@acezen acezen mentioned this pull request Mar 25, 2021
18 tasks
@sighingnow sighingnow merged commit 4f5db65 into alibaba:main Mar 25, 2021
@acezen acezen deleted the zwb/louvain branch September 3, 2021 10:52
acezen added a commit to acezen/GraphScope that referenced this pull request Jan 8, 2024
acezen added a commit to acezen/GraphScope that referenced this pull request Jan 15, 2024
acezen added a commit that referenced this pull request Jan 15, 2024
…sistent with #183 (#3472)

## What do these changes do?

This change try to fix the louvain algorithm that the result is not
consistent with #183

## Related issue number

Close #3457
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add louvain to built-in apps
5 participants