-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-11432][GraphX] Personalized PageRank shouldn't use uniform initialization #9386
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
|
Jenkins, add to whitelist. |
|
Jenkins, ok to test. |
|
Test build #44711 has finished for PR 9386 at commit
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.mapVertices { (id, attr) =>
if (personalized) {
if (id == srcId.get) resetProb else 0.0
} else {
resetProb
}
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With src, let's do
.mapVertices { (id, attr) =>
if (!(id != src && personalized)) resetProb else 0.0
}to avoid extra unboxing of Option
|
Seems that the implementation of personalized page rank doesn't follow the twitter's paper entirely when initializing the initial page rank. In the paper, only the source node should be activated. This PR addresses this issue. +cc @dwmclary @ankurdave @rxin @jegonzal for more feedback. Thanks. |
|
If I recall, we specifically decided against a conditional in the BSP function at that point because the branching might causes hotspots. If that's still a concern, maybe @jegonzal can comment. Otherwise, this looks good to me -- nice catch. |
|
I think the branching should be fine since it's just initialization. |
|
Yes, I agree, it shouldn't add overhead. Sent from my iPhone
|
|
This is actually a pretty serious error since it could lead to mass being accumulated on unreachable sub-graphs. The performance implications of the above branch should be negligible. |
|
Jenkins, ok to test. |
|
Test build #44875 has finished for PR 9386 at commit
|
|
LGTM. Thanks, merged into master. |
|
Many thanks @dbtsai |
…tialization Changes the personalized pagerank initialization to be non-uniform. Author: Yves Raimond <[email protected]> Closes #9386 from moustaki/personalized-pagerank-init. (cherry picked from commit efaa472) Signed-off-by: Xiangrui Meng <[email protected]>
…tialization Changes the personalized pagerank initialization to be non-uniform. Author: Yves Raimond <[email protected]> Closes #9386 from moustaki/personalized-pagerank-init. (cherry picked from commit efaa472) Signed-off-by: Xiangrui Meng <[email protected]>
|
I backported this patch to branch-1.5 and 1.4. |
Changes the personalized pagerank initialization to be non-uniform.