----------------------------------------------------------- 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 > >
