Skip to content

Bug fix in sendMessage() of pregel implementation in PageRank.scala - #20899

Closed
mangolzy wants to merge 1 commit into
apache:masterfrom
mangolzy:patch-1
Closed

Bug fix in sendMessage() of pregel implementation in PageRank.scala#20899
mangolzy wants to merge 1 commit into
apache:masterfrom
mangolzy:patch-1

Conversation

@mangolzy

Copy link
Copy Markdown

What changes were proposed in this pull request?

Iterator((edge.dstId, edge.srcAttr._2 * edge.attr))
->
Iterator((edge.dstId, edge.srcAttr._1 * edge.attr))

How was this patch tested?

Since edge.srcAttr._2 is used to compare with tol, it should be the (newPR - oldPR), but in the sendMessage, the origin code send it as part of the message, instead it should be the newPR which is edge.srcAttr._1.

Please review http://spark.apache.org/contributing.html before opening a pull request.

Iterator((edge.dstId, edge.srcAttr._2 * edge.attr))
->
Iterator((edge.dstId, edge.srcAttr._1 * edge.attr))
Since edge.srcAttr._2 is used to compare with tol, it should be the (newPR - oldPR), but in the sendMessage, the origin code send it as part of the message, instead it should be the newPR which is edge.srcAttr._1.
@mangolzymangolzy changed the title fix bug in sendMessage() of pregel implementationfix bug in sendMessage() of pregel implementation in PageRank.scalaMar 24, 2018
@mangolzymangolzy changed the title fix bug in sendMessage() of pregel implementation in PageRank.scalaBug fix in sendMessage() of pregel implementation in PageRank.scalaMar 24, 2018
@WenqianZhao

Copy link
Copy Markdown

You can check the comment in pull request #2100.

@WenqianZhao

Copy link
Copy Markdown

By the way, @ankurdave , hi, I'm very curious about why you use delta instead of just newPR as the message. Is that for better performance or just at chance? Could you please answer this question when you are free? Thank you so much.

@ankurdave

ankurdave commented Oct 31, 2018

Copy link
Copy Markdown
Contributor

@WenqianZhao I think the point of sending deltas instead of absolute ranks was that, as parts of the graph converge, their deltas would go to zero. GraphX would then be able to compress those zero messages more efficiently.

@WenqianZhao

Copy link
Copy Markdown

@WenqianZhao I think the point of sending deltas instead of absolute ranks was that, as parts of the graph converge, their deltas would go to zero. GraphX would then be able to compress those zero messages more efficiently.

Got it! Thank you!

@AmplabJenkins

Copy link
Copy Markdown

Can one of the admins verify this patch?

@srowensrowen closed this Apr 8, 2019
Sign up for freeto 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.

5 participants

@mangolzy@WenqianZhao@ankurdave@AmplabJenkins@srowen