You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@giraph.apache.org by Sebastian Schelter <ss...@apache.org> on 2013/03/11 08:40:38 UTC
Review Request: GIRAPH-480: Add convergence detection to
org.apache.giraph.examples.RandomWalkVertex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9850/
-----------------------------------------------------------
Review request for giraph.
Description
-------
Added convergence detection to the RandomWalkVertex, added a PageRank implementation based on the RandomWalkVertex, refactored the RandomWalkVertex to be indepedent of the edge type (PageRank with uniform transition probabilities doesn't need edge weights for example)
Diffs
-----
giraph-examples/pom.xml 7a18711
giraph-examples/src/main/java/org/apache/giraph/examples/LongDoubleNullDoubleTextInputFormat.java PRE-CREATION
giraph-examples/src/main/java/org/apache/giraph/examples/PageRankVertex.java PRE-CREATION
giraph-examples/src/main/java/org/apache/giraph/examples/RandomWalkVertex.java 8196523
giraph-examples/src/main/java/org/apache/giraph/examples/RandomWalkWithRestartVertex.java 8a689ed
giraph-examples/src/main/java/org/apache/giraph/examples/RandomWalkWorkerContext.java 5cff23f
giraph-examples/src/main/java/org/apache/giraph/examples/VertexWithDoubleValueNullEdgeTextOutputFormat.java PRE-CREATION
giraph-examples/src/test/java/org/apache/giraph/examples/PageRankVertexTest.java PRE-CREATION
giraph-examples/src/test/java/org/apache/giraph/examples/RandomWalkUtils.java PRE-CREATION
giraph-examples/src/test/java/org/apache/giraph/examples/RandomWalkWithRestartVertexTest.java 489b35a
Diff: https://reviews.apache.org/r/9850/diff/
Testing
-------
Ran the junit tests locally
Thanks,
Sebastian Schelter
Re: Review Request: GIRAPH-480: Add convergence detection to
org.apache.giraph.examples.RandomWalkVertex
Posted by Alessandro Presta <al...@fb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9850/#review18064
-----------------------------------------------------------
Very neat! I just have a few comments.
giraph-examples/src/main/java/org/apache/giraph/examples/RandomWalkVertex.java
<https://reviews.apache.org/r/9850/#comment38147>
Why are some aggregators prefixed with the class name, while some aren't?
Also, "dangling" and "convergence" don't really convey the meaning of those aggregators. Can you give them more explicit names?
giraph-examples/src/main/java/org/apache/giraph/examples/RandomWalkVertex.java
<https://reviews.apache.org/r/9850/#comment38150>
Why do you need to expose this as a protected member? It seems to me that it's only used in compute(), so it could be replaced by a local naked double value.
Also, I would call it "stateProbability" rather than "d".
giraph-examples/src/main/java/org/apache/giraph/examples/RandomWalkVertex.java
<https://reviews.apache.org/r/9850/#comment38148>
Missing a space.
giraph-examples/src/main/java/org/apache/giraph/examples/RandomWalkVertex.java
<https://reviews.apache.org/r/9850/#comment38149>
Cumulative?
giraph-examples/src/test/java/org/apache/giraph/examples/RandomWalkUtils.java
<https://reviews.apache.org/r/9850/#comment38152>
I'm being picky, but can you rename this to RandomWalkTestUtils, to make it clear it's not needed by the random walk implementation?
- Alessandro Presta
On March 11, 2013, 7:40 a.m., Sebastian Schelter wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9850/
> -----------------------------------------------------------
>
> (Updated March 11, 2013, 7:40 a.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> Added convergence detection to the RandomWalkVertex, added a PageRank implementation based on the RandomWalkVertex, refactored the RandomWalkVertex to be indepedent of the edge type (PageRank with uniform transition probabilities doesn't need edge weights for example)
>
>
> Diffs
> -----
>
> giraph-examples/pom.xml 7a18711
> giraph-examples/src/main/java/org/apache/giraph/examples/LongDoubleNullDoubleTextInputFormat.java PRE-CREATION
> giraph-examples/src/main/java/org/apache/giraph/examples/PageRankVertex.java PRE-CREATION
> giraph-examples/src/main/java/org/apache/giraph/examples/RandomWalkVertex.java 8196523
> giraph-examples/src/main/java/org/apache/giraph/examples/RandomWalkWithRestartVertex.java 8a689ed
> giraph-examples/src/main/java/org/apache/giraph/examples/RandomWalkWorkerContext.java 5cff23f
> giraph-examples/src/main/java/org/apache/giraph/examples/VertexWithDoubleValueNullEdgeTextOutputFormat.java PRE-CREATION
> giraph-examples/src/test/java/org/apache/giraph/examples/PageRankVertexTest.java PRE-CREATION
> giraph-examples/src/test/java/org/apache/giraph/examples/RandomWalkUtils.java PRE-CREATION
> giraph-examples/src/test/java/org/apache/giraph/examples/RandomWalkWithRestartVertexTest.java 489b35a
>
> Diff: https://reviews.apache.org/r/9850/diff/
>
>
> Testing
> -------
>
> Ran the junit tests locally
>
>
> Thanks,
>
> Sebastian Schelter
>
>
Re: Review Request: GIRAPH-480: Add convergence detection to
org.apache.giraph.examples.RandomWalkVertex
Posted by Alessandro Presta <al...@fb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9850/#review18078
-----------------------------------------------------------
I think you forgot to remove RandomWalkUtils. Other than that, +1.
- Alessandro Presta
On March 19, 2013, 8:03 a.m., Sebastian Schelter wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9850/
> -----------------------------------------------------------
>
> (Updated March 19, 2013, 8:03 a.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> Added convergence detection to the RandomWalkVertex, added a PageRank implementation based on the RandomWalkVertex, refactored the RandomWalkVertex to be indepedent of the edge type (PageRank with uniform transition probabilities doesn't need edge weights for example)
>
>
> Diffs
> -----
>
> giraph-examples/pom.xml 7a18711
> giraph-examples/src/main/java/org/apache/giraph/examples/LongDoubleNullDoubleTextInputFormat.java PRE-CREATION
> giraph-examples/src/main/java/org/apache/giraph/examples/PageRankVertex.java PRE-CREATION
> giraph-examples/src/main/java/org/apache/giraph/examples/RandomWalkVertex.java 8196523
> giraph-examples/src/main/java/org/apache/giraph/examples/RandomWalkWithRestartVertex.java 8a689ed
> giraph-examples/src/main/java/org/apache/giraph/examples/RandomWalkWorkerContext.java 5cff23f
> giraph-examples/src/main/java/org/apache/giraph/examples/VertexWithDoubleValueNullEdgeTextOutputFormat.java PRE-CREATION
> giraph-examples/src/test/java/org/apache/giraph/examples/PageRankVertexTest.java PRE-CREATION
> giraph-examples/src/test/java/org/apache/giraph/examples/RandomWalkTestUtils.java PRE-CREATION
> giraph-examples/src/test/java/org/apache/giraph/examples/RandomWalkWithRestartVertexTest.java 489b35a
>
> Diff: https://reviews.apache.org/r/9850/diff/
>
>
> Testing
> -------
>
> Ran the junit tests locally
>
>
> Thanks,
>
> Sebastian Schelter
>
>
Re: Review Request: GIRAPH-480: Add convergence detection to
org.apache.giraph.examples.RandomWalkVertex
Posted by Sebastian Schelter <ss...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9850/
-----------------------------------------------------------
(Updated March 19, 2013, 8:03 a.m.)
Review request for giraph.
Changes
-------
Reworked the patch to match Alessandro's comments
Description
-------
Added convergence detection to the RandomWalkVertex, added a PageRank implementation based on the RandomWalkVertex, refactored the RandomWalkVertex to be indepedent of the edge type (PageRank with uniform transition probabilities doesn't need edge weights for example)
Diffs (updated)
-----
giraph-examples/pom.xml 7a18711
giraph-examples/src/main/java/org/apache/giraph/examples/LongDoubleNullDoubleTextInputFormat.java PRE-CREATION
giraph-examples/src/main/java/org/apache/giraph/examples/PageRankVertex.java PRE-CREATION
giraph-examples/src/main/java/org/apache/giraph/examples/RandomWalkVertex.java 8196523
giraph-examples/src/main/java/org/apache/giraph/examples/RandomWalkWithRestartVertex.java 8a689ed
giraph-examples/src/main/java/org/apache/giraph/examples/RandomWalkWorkerContext.java 5cff23f
giraph-examples/src/main/java/org/apache/giraph/examples/VertexWithDoubleValueNullEdgeTextOutputFormat.java PRE-CREATION
giraph-examples/src/test/java/org/apache/giraph/examples/PageRankVertexTest.java PRE-CREATION
giraph-examples/src/test/java/org/apache/giraph/examples/RandomWalkTestUtils.java PRE-CREATION
giraph-examples/src/test/java/org/apache/giraph/examples/RandomWalkWithRestartVertexTest.java 489b35a
Diff: https://reviews.apache.org/r/9850/diff/
Testing
-------
Ran the junit tests locally
Thanks,
Sebastian Schelter