-
Notifications
You must be signed in to change notification settings - Fork 29k
[GraphX] initialmessage for pagerank should be 1.0 #1128
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
Conversation
|
Can one of the admins verify this patch? |
|
In my opinion, if the numIter is set to 0, then call Pregel, the pagerank value of each vertex should be 1, which is as the same as the traditional definition of pagerank. |
|
ok to test cc @jegonzal |
|
I don't think this will give the expected results. Consider the graph |
|
Hi ankurdave, In the graph you given( when the initalvalue is 0.0 a b when the initalvalue is 1.0 1 1 Actually, there is another problem maybe we should handle if there is 0-outdegree vertices in the graph(e.g. 2). One solution is to distribute their pagerank values to all the vertices uniformly, but the initialvalue should be one. For example, a b Omit some iterations. 0.701756 1.29824 In my understanding, if there is no 0-outdegree vertices in a graph, the sum of pagerank values from all vertices should remain the same. However, in the graphX implementation, this is not true. In a word, I still think the initialvalue should be 1.0, you could also refer to graphlab's implementation. 52th line of https://github.com/graphlab-code/graphlab/blob/master/toolkits/graph_analytics/pagerank.cpp |
|
@luyi0619 can you add |
|
sure, thanks for your advice |
|
Can one of the admins verify this patch? |
|
cc @jegonzal can you take a look at this? |
|
Is this a WontFix at this stage or needs another look from @ankurdave or @jegonzal ? |
|
I talked to @jegonzal and it seems this is the correct thing to do. I'll merge it pending Jenkins. |
|
Jenkins, test this please. |
|
Test build #27840 has started for PR 1128 at commit
|
|
Oh, this PR was against the old version of static PageRank that used Pregel, while we now use a standalone implementation. I think it still has the same problem and the fix seems to be to change the initially-assigned value from resetProb to 1. We can close this issue and I'll open a new one. |
|
Great! I agree with this proposal as well. I apologize for letting it sit
so long.
|
|
Test build #27840 has finished for PR 1128 at commit
|
|
Test FAILed. |
|
@luyi0619 mind closing this PR? It sounds like this can be pursued in a new PR. |
apache#1128) Co-authored-by: Egor Krivokon <>
In the first round, pagerank value for each vertex should be 1.0, so the initialmessage should be set to 1.0, right?