You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@giraph.apache.org by Crystal Ice <je...@gmail.com> on 2013/05/29 19:17:23 UTC
Review Request: aggregator at input super step
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11525/
-----------------------------------------------------------
Review request for giraph.
Description
-------
add aggregator for vertex reader and edge reader at input super step
Diffs
-----
giraph-core/src/main/java/org/apache/giraph/io/EdgeReader.java 363a5e6
giraph-core/src/main/java/org/apache/giraph/io/VertexReader.java 9695169
giraph-core/src/main/java/org/apache/giraph/io/internal/WrappedEdgeReader.java aae7a72
giraph-core/src/main/java/org/apache/giraph/io/internal/WrappedVertexReader.java 54adfec
giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java bd48116
giraph-core/src/main/java/org/apache/giraph/worker/BspServiceWorker.java 8b5e39a
giraph-core/src/main/java/org/apache/giraph/worker/EdgeInputSplitsCallable.java 78cdd8e
giraph-core/src/main/java/org/apache/giraph/worker/VertexInputSplitsCallable.java 977e100
giraph-core/src/test/java/org/apache/giraph/master/TestSwitchClasses.java 4069972
giraph-examples/src/main/java/org/apache/giraph/examples/AggregatorsTestComputation.java db527f2
giraph-examples/src/test/java/org/apache/giraph/aggregators/TestAggregatorsHandling.java 6d22800
Diff: https://reviews.apache.org/r/11525/diff/
Testing
-------
Thanks,
Crystal Ice
Re: Review Request: aggregator at input super step
Posted by Bingjing Zhang <je...@gmail.com>.
> On May 29, 2013, 10:41 p.m., Avery Ching wrote:
> > giraph-core/src/main/java/org/apache/giraph/worker/EdgeInputSplitsCallable.java, line 102
> > <https://reviews.apache.org/r/11525/diff/1/?file=298114#file298114line102>
> >
> > Same as VertexInputSplitsCallable
bspServiceWorker is also used in "PartitionOwner partitionOwner = bspServiceWorker.getVertexPartitionOwner(readerVertex.getId());" in L172-L173
> On May 29, 2013, 10:41 p.m., Avery Ching wrote:
> > giraph-core/src/main/java/org/apache/giraph/io/VertexReader.java, lines 60-70
> > <https://reviews.apache.org/r/11525/diff/1/?file=298109#file298109line60>
> >
> > Let's do the initialize() approach. It's cleaner.
lots of changes to initialize in subClasses
> On May 29, 2013, 10:41 p.m., Avery Ching wrote:
> > giraph-core/src/main/java/org/apache/giraph/io/EdgeReader.java, lines 59-68
> > <https://reviews.apache.org/r/11525/diff/1/?file=298108#file298108line59>
> >
> > Let's do the initialize() approach. It's cleaner.
lots of changes to initialize in subClasses
- Bingjing
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11525/#review21161
-----------------------------------------------------------
On May 30, 2013, 4:46 p.m., Bingjing Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11525/
> -----------------------------------------------------------
>
> (Updated May 30, 2013, 4:46 p.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> add aggregator for vertex reader and edge reader at input super step.
>
> Changes in code:
> add "aggregate" interface to VertexReader and EdgeReader
> initialize "aggregator" in "readInputSplit" in "VertexInputSplitsCallable" and "EdgeInputSplitsCallable"
> initialize master aggregator before data loading in "coordinateSuperStep" in "BspServiceMaster"
> seperate master aggregator initialization and its computation for Super Step 0 (originally they are mixed in one method "runMasterCompute")
> perpare aggregator in "setup" in "BspServiceWorker" before data loading
>
> Test:
> Add SimpleVertexReader and SimpleEdgeReader to AggregatorsTestComputation for genreating input data.
> Also add "aggregate" call in "getCurrentVertex" and "getCurrentEdge".
> Register persistent aggregator with name "INPUT_VERTEX_PERSISTENT_AGG" and "INPUT_EDGE_PERSISTENT_AGG"in AggregatorsTestMasterCompute.initialize
> Assert if aggregated values can be retrived in AggregatorsTestMasterCompute.compute when "superstep >= 0"
>
>
> This addresses bug GIRAPH-673.
> https://issues.apache.org/jira/browse/GIRAPH-673
>
>
> Diffs
> -----
>
> giraph-core/src/main/java/org/apache/giraph/io/EdgeReader.java 363a5e6
> giraph-core/src/main/java/org/apache/giraph/io/VertexReader.java 9695169
> giraph-core/src/main/java/org/apache/giraph/io/internal/WrappedEdgeReader.java aae7a72
> giraph-core/src/main/java/org/apache/giraph/io/internal/WrappedVertexReader.java 54adfec
> giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java bd48116
> giraph-core/src/main/java/org/apache/giraph/worker/BspServiceWorker.java 8b5e39a
> giraph-core/src/main/java/org/apache/giraph/worker/EdgeInputSplitsCallable.java 78cdd8e
> giraph-core/src/main/java/org/apache/giraph/worker/VertexInputSplitsCallable.java 977e100
> giraph-examples/src/main/java/org/apache/giraph/examples/AggregatorsTestComputation.java db527f2
> giraph-examples/src/test/java/org/apache/giraph/aggregators/TestAggregatorsHandling.java 6d22800
>
> Diff: https://reviews.apache.org/r/11525/diff/
>
>
> Testing
> -------
>
> Unit Test of persistent aggregator at input super step in modified "AggregatorsTestComputation" is done.
> "mvn clean verify" was done on giraph-core
> "mvn clean verfiy" was done on giraph-example
>
> rechecked "indent" problem. Hopefully they are all solved
>
>
> Thanks,
>
> Bingjing Zhang
>
>
Re: Review Request: aggregator at input super step
Posted by Avery Ching <av...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11525/#review21161
-----------------------------------------------------------
It's a great start, but you'll need to clean it up a bit before this is ready to be shipped. Can you also run 'mvn clean verify' in the parent directory to ensure nothing else broke. Thanks!
giraph-core/src/main/java/org/apache/giraph/io/EdgeReader.java
<https://reviews.apache.org/r/11525/#comment43969>
Extra space.
giraph-core/src/main/java/org/apache/giraph/io/EdgeReader.java
<https://reviews.apache.org/r/11525/#comment43954>
aggreagator -> Aggregator
for vertex -> for vertex
giraph-core/src/main/java/org/apache/giraph/io/EdgeReader.java
<https://reviews.apache.org/r/11525/#comment43972>
Let's do the initialize() approach. It's cleaner.
giraph-core/src/main/java/org/apache/giraph/io/EdgeReader.java
<https://reviews.apache.org/r/11525/#comment43958>
I don't think we should silently fail.
giraph-core/src/main/java/org/apache/giraph/io/EdgeReader.java
<https://reviews.apache.org/r/11525/#comment43960>
Same here.
giraph-core/src/main/java/org/apache/giraph/io/VertexReader.java
<https://reviews.apache.org/r/11525/#comment43973>
Let's do the initialize() approach. It's cleaner.
giraph-core/src/main/java/org/apache/giraph/io/VertexReader.java
<https://reviews.apache.org/r/11525/#comment43961>
same here
giraph-core/src/main/java/org/apache/giraph/io/VertexReader.java
<https://reviews.apache.org/r/11525/#comment43962>
same here
giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java
<https://reviews.apache.org/r/11525/#comment43986>
Given that this is checked before calling, we shouldn't silently fail.
giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java
<https://reviews.apache.org/r/11525/#comment43974>
Commented out lines should be removed.
giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java
<https://reviews.apache.org/r/11525/#comment43988>
A lot of duplicate code here, can you merge this and provide an argument perhaps?
giraph-core/src/main/java/org/apache/giraph/worker/EdgeInputSplitsCallable.java
<https://reviews.apache.org/r/11525/#comment43977>
You only need the aggregator handler right?
giraph-core/src/main/java/org/apache/giraph/worker/EdgeInputSplitsCallable.java
<https://reviews.apache.org/r/11525/#comment43985>
Same as VertexInputSplitsCallable
giraph-core/src/test/java/org/apache/giraph/master/TestSwitchClasses.java
<https://reviews.apache.org/r/11525/#comment43979>
Remove this.
giraph-examples/src/test/java/org/apache/giraph/aggregators/TestAggregatorsHandling.java
<https://reviews.apache.org/r/11525/#comment43963>
Bad indent.
giraph-examples/src/test/java/org/apache/giraph/aggregators/TestAggregatorsHandling.java
<https://reviews.apache.org/r/11525/#comment43964>
Bad indent.
giraph-examples/src/test/java/org/apache/giraph/aggregators/TestAggregatorsHandling.java
<https://reviews.apache.org/r/11525/#comment43965>
Bad indent.
giraph-examples/src/test/java/org/apache/giraph/aggregators/TestAggregatorsHandling.java
<https://reviews.apache.org/r/11525/#comment43966>
Bad indent.
giraph-examples/src/test/java/org/apache/giraph/aggregators/TestAggregatorsHandling.java
<https://reviews.apache.org/r/11525/#comment43967>
Bad indent.
giraph-examples/src/test/java/org/apache/giraph/aggregators/TestAggregatorsHandling.java
<https://reviews.apache.org/r/11525/#comment43968>
Bad indent.
- Avery Ching
On May 29, 2013, 5:41 p.m., Bingjing Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11525/
> -----------------------------------------------------------
>
> (Updated May 29, 2013, 5:41 p.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> add aggregator for vertex reader and edge reader at input super step.
>
> Changes in code:
> add "aggregate" interface to VertexReader and EdgeReader
> initialize "aggregator" in "readInputSplit" in "VertexInputSplitsCallable" and "EdgeInputSplitsCallable"
> initialize master aggregator before data loading in "coordinateSuperStep" in "BspServiceMaster"
> seperate master aggregator initialization and its computation for Super Step 0 (originally they are mixed in one method "runMasterCompute")
> perpare aggregator in "setup" in "BspServiceWorker" before data loading
>
> Test:
> Add SimpleVertexReader and SimpleEdgeReader to AggregatorsTestComputation for genreating input data.
> Also add "aggregate" call in "getCurrentVertex" and "getCurrentEdge".
> Register persistent aggregator with name "INPUT_VERTEX_PERSISTENT_AGG" and "INPUT_EDGE_PERSISTENT_AGG"in AggregatorsTestMasterCompute.initialize
> Assert if aggregated values can be retrived in AggregatorsTestMasterCompute.compute when "superstep >= 0"
>
>
> This addresses bug GIRAPH-673.
> https://issues.apache.org/jira/browse/GIRAPH-673
>
>
> Diffs
> -----
>
> giraph-core/src/main/java/org/apache/giraph/io/EdgeReader.java 363a5e6
> giraph-core/src/main/java/org/apache/giraph/io/VertexReader.java 9695169
> giraph-core/src/main/java/org/apache/giraph/io/internal/WrappedEdgeReader.java aae7a72
> giraph-core/src/main/java/org/apache/giraph/io/internal/WrappedVertexReader.java 54adfec
> giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java bd48116
> giraph-core/src/main/java/org/apache/giraph/worker/BspServiceWorker.java 8b5e39a
> giraph-core/src/main/java/org/apache/giraph/worker/EdgeInputSplitsCallable.java 78cdd8e
> giraph-core/src/main/java/org/apache/giraph/worker/VertexInputSplitsCallable.java 977e100
> giraph-core/src/test/java/org/apache/giraph/master/TestSwitchClasses.java 4069972
> giraph-examples/src/main/java/org/apache/giraph/examples/AggregatorsTestComputation.java db527f2
> giraph-examples/src/test/java/org/apache/giraph/aggregators/TestAggregatorsHandling.java 6d22800
>
> Diff: https://reviews.apache.org/r/11525/diff/
>
>
> Testing
> -------
>
> Unit Test of persistent aggregator at input super step in modified "AggregatorsTestComputation" is done.
>
>
> Thanks,
>
> Bingjing Zhang
>
>
Re: Review Request: aggregator at input super step
Posted by Avery Ching <av...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11525/#review21287
-----------------------------------------------------------
Getting better...Please take a look at the grammar and make sure everything looks polished.
giraph-core/src/main/java/org/apache/giraph/io/EdgeReader.java
<https://reviews.apache.org/r/11525/#comment44164>
This comment is a bit messy. Perhaps something like
"Sets the aggreagator usage. This method is only for use by the infrastructure."
By the way, should we call it setWorkerAggregatorUse instead?
giraph-core/src/main/java/org/apache/giraph/io/VertexReader.java
<https://reviews.apache.org/r/11525/#comment44165>
extra space.
"for vertex" -> "for vertex"
giraph-core/src/main/java/org/apache/giraph/io/VertexReader.java
<https://reviews.apache.org/r/11525/#comment44166>
Please consider a cleaner comment (i.e. the same suggestion above).
giraph-core/src/main/java/org/apache/giraph/io/internal/WrappedEdgeReader.java
<https://reviews.apache.org/r/11525/#comment44167>
// set -> // Set
giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java
<https://reviews.apache.org/r/11525/#comment44337>
Please reword this comment.
giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java
<https://reviews.apache.org/r/11525/#comment44331>
If you don't think it's necessary, feel free to take it out.
giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java
<https://reviews.apache.org/r/11525/#comment44338>
bfore -> before
giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java
<https://reviews.apache.org/r/11525/#comment44332>
Maybe change to something like "This method is called only after masterCompute is initialized."
giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java
<https://reviews.apache.org/r/11525/#comment44333>
Should this method be moved closer to where its called? - appears only once I think.
giraph-core/src/main/java/org/apache/giraph/worker/BspServiceWorker.java
<https://reviews.apache.org/r/11525/#comment44334>
Given this is a one line method called once, does it make more sense to inline it?
giraph-core/src/main/java/org/apache/giraph/worker/VertexInputSplitsCallable.java
<https://reviews.apache.org/r/11525/#comment44335>
// Similar...
giraph-core/src/main/java/org/apache/giraph/worker/VertexInputSplitsCallable.java
<https://reviews.apache.org/r/11525/#comment44336>
// Set...
- Avery Ching
On May 30, 2013, 4:46 p.m., Bingjing Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11525/
> -----------------------------------------------------------
>
> (Updated May 30, 2013, 4:46 p.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> add aggregator for vertex reader and edge reader at input super step.
>
> Changes in code:
> add "aggregate" interface to VertexReader and EdgeReader
> initialize "aggregator" in "readInputSplit" in "VertexInputSplitsCallable" and "EdgeInputSplitsCallable"
> initialize master aggregator before data loading in "coordinateSuperStep" in "BspServiceMaster"
> seperate master aggregator initialization and its computation for Super Step 0 (originally they are mixed in one method "runMasterCompute")
> perpare aggregator in "setup" in "BspServiceWorker" before data loading
>
> Test:
> Add SimpleVertexReader and SimpleEdgeReader to AggregatorsTestComputation for genreating input data.
> Also add "aggregate" call in "getCurrentVertex" and "getCurrentEdge".
> Register persistent aggregator with name "INPUT_VERTEX_PERSISTENT_AGG" and "INPUT_EDGE_PERSISTENT_AGG"in AggregatorsTestMasterCompute.initialize
> Assert if aggregated values can be retrived in AggregatorsTestMasterCompute.compute when "superstep >= 0"
>
>
> This addresses bug GIRAPH-673.
> https://issues.apache.org/jira/browse/GIRAPH-673
>
>
> Diffs
> -----
>
> giraph-core/src/main/java/org/apache/giraph/io/EdgeReader.java 363a5e6
> giraph-core/src/main/java/org/apache/giraph/io/VertexReader.java 9695169
> giraph-core/src/main/java/org/apache/giraph/io/internal/WrappedEdgeReader.java aae7a72
> giraph-core/src/main/java/org/apache/giraph/io/internal/WrappedVertexReader.java 54adfec
> giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java bd48116
> giraph-core/src/main/java/org/apache/giraph/worker/BspServiceWorker.java 8b5e39a
> giraph-core/src/main/java/org/apache/giraph/worker/EdgeInputSplitsCallable.java 78cdd8e
> giraph-core/src/main/java/org/apache/giraph/worker/VertexInputSplitsCallable.java 977e100
> giraph-examples/src/main/java/org/apache/giraph/examples/AggregatorsTestComputation.java db527f2
> giraph-examples/src/test/java/org/apache/giraph/aggregators/TestAggregatorsHandling.java 6d22800
>
> Diff: https://reviews.apache.org/r/11525/diff/
>
>
> Testing
> -------
>
> Unit Test of persistent aggregator at input super step in modified "AggregatorsTestComputation" is done.
> "mvn clean verify" was done on giraph-core
> "mvn clean verfiy" was done on giraph-example
>
> rechecked "indent" problem. Hopefully they are all solved
>
>
> Thanks,
>
> Bingjing Zhang
>
>
Re: Review Request: aggregator at input super step
Posted by Avery Ching <av...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11525/#review22090
-----------------------------------------------------------
Minor issues below. I was able to run 'mvn clean verify' with your code and it passed. Let's fix these minor things and ship it!
giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java
<https://reviews.apache.org/r/11525/#comment45478>
cooperates with other code
giraph-core/src/main/java/org/apache/giraph/worker/EdgeInputSplitsCallable.java
<https://reviews.apache.org/r/11525/#comment45479>
Aggregator handler
giraph-examples/src/main/java/org/apache/giraph/examples/GeneratedEdgeReader.java
<https://reviews.apache.org/r/11525/#comment45477>
Can you please convert this to LongConfOption? See GiraphConf for an example.
- Avery Ching
On June 17, 2013, 4:48 p.m., Bingjing Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11525/
> -----------------------------------------------------------
>
> (Updated June 17, 2013, 4:48 p.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> add aggregator for vertex reader and edge reader at input super step.
>
> Changes in code:
> add "aggregate" interface to VertexReader and EdgeReader
> initialize "aggregator" in "readInputSplit" in "VertexInputSplitsCallable" and "EdgeInputSplitsCallable"
> initialize master aggregator before data loading in "coordinateSuperStep" in "BspServiceMaster"
> seperate master aggregator initialization and its computation for Super Step 0 (originally they are mixed in one method "runMasterCompute")
> perpare aggregator in "setup" in "BspServiceWorker" before data loading
>
> Test:
> Add SimpleVertexReader and SimpleEdgeReader to AggregatorsTestComputation for genreating input data.
> Also add "aggregate" call in "getCurrentVertex" and "getCurrentEdge".
> Register persistent aggregator with name "INPUT_VERTEX_PERSISTENT_AGG" and "INPUT_EDGE_PERSISTENT_AGG"in AggregatorsTestMasterCompute.initialize
> Assert if aggregated values can be retrived in AggregatorsTestMasterCompute.compute when "superstep >= 0"
>
>
> This addresses bug GIRAPH-673.
> https://issues.apache.org/jira/browse/GIRAPH-673
>
>
> Diffs
> -----
>
> giraph-core/src/main/java/org/apache/giraph/io/EdgeReader.java 363a5e6
> giraph-core/src/main/java/org/apache/giraph/io/VertexReader.java b8b82af
> giraph-core/src/main/java/org/apache/giraph/io/internal/WrappedEdgeReader.java aae7a72
> giraph-core/src/main/java/org/apache/giraph/io/internal/WrappedVertexReader.java 54adfec
> giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java bd48116
> giraph-core/src/main/java/org/apache/giraph/worker/BspServiceWorker.java eb6d30d
> giraph-core/src/main/java/org/apache/giraph/worker/EdgeInputSplitsCallable.java 78cdd8e
> giraph-core/src/main/java/org/apache/giraph/worker/VertexInputSplitsCallable.java 977e100
> giraph-examples/src/main/java/org/apache/giraph/examples/AggregatorsTestComputation.java db527f2
> giraph-examples/src/main/java/org/apache/giraph/examples/GeneratedEdgeReader.java PRE-CREATION
> giraph-examples/src/test/java/org/apache/giraph/aggregators/TestAggregatorsHandling.java 6d22800
>
> Diff: https://reviews.apache.org/r/11525/diff/
>
>
> Testing
> -------
>
> Unit Test of persistent aggregator at input super step in modified "AggregatorsTestComputation" is done.
> "mvn clean verify" was done on giraph-core
> "mvn clean verfiy" was done on giraph-example
>
> rechecked "indent" problem. Hopefully they are all solved
>
>
> Thanks,
>
> Bingjing Zhang
>
>
Re: Review Request: aggregator at input super step
Posted by Avery Ching <av...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11525/#review22122
-----------------------------------------------------------
Ship it!
+1, I'm committing this. Thanks Bingjing!
- Avery Ching
On June 19, 2013, 4:29 p.m., Bingjing Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11525/
> -----------------------------------------------------------
>
> (Updated June 19, 2013, 4:29 p.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> add aggregator for vertex reader and edge reader at input super step.
>
> Changes in code:
> add "aggregate" interface to VertexReader and EdgeReader
> initialize "aggregator" in "readInputSplit" in "VertexInputSplitsCallable" and "EdgeInputSplitsCallable"
> initialize master aggregator before data loading in "coordinateSuperStep" in "BspServiceMaster"
> seperate master aggregator initialization and its computation for Super Step 0 (originally they are mixed in one method "runMasterCompute")
> perpare aggregator in "setup" in "BspServiceWorker" before data loading
>
> Test:
> Add SimpleVertexReader and SimpleEdgeReader to AggregatorsTestComputation for genreating input data.
> Also add "aggregate" call in "getCurrentVertex" and "getCurrentEdge".
> Register persistent aggregator with name "INPUT_VERTEX_PERSISTENT_AGG" and "INPUT_EDGE_PERSISTENT_AGG"in AggregatorsTestMasterCompute.initialize
> Assert if aggregated values can be retrived in AggregatorsTestMasterCompute.compute when "superstep >= 0"
>
>
> This addresses bug GIRAPH-673.
> https://issues.apache.org/jira/browse/GIRAPH-673
>
>
> Diffs
> -----
>
> giraph-core/src/main/java/org/apache/giraph/io/EdgeReader.java 363a5e6
> giraph-core/src/main/java/org/apache/giraph/io/VertexReader.java b8b82af
> giraph-core/src/main/java/org/apache/giraph/io/internal/WrappedEdgeReader.java aae7a72
> giraph-core/src/main/java/org/apache/giraph/io/internal/WrappedVertexReader.java 54adfec
> giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java bd48116
> giraph-core/src/main/java/org/apache/giraph/worker/BspServiceWorker.java eb6d30d
> giraph-core/src/main/java/org/apache/giraph/worker/EdgeInputSplitsCallable.java 78cdd8e
> giraph-core/src/main/java/org/apache/giraph/worker/VertexInputSplitsCallable.java 977e100
> giraph-examples/src/main/java/org/apache/giraph/examples/AggregatorsTestComputation.java db527f2
> giraph-examples/src/main/java/org/apache/giraph/examples/GeneratedEdgeReader.java PRE-CREATION
> giraph-examples/src/test/java/org/apache/giraph/aggregators/TestAggregatorsHandling.java 6d22800
>
> Diff: https://reviews.apache.org/r/11525/diff/
>
>
> Testing
> -------
>
> Unit Test of persistent aggregator at input super step in modified "AggregatorsTestComputation" is done.
> "mvn clean verify" was done on giraph-core
> "mvn clean verfiy" was done on giraph-example
>
> rechecked "indent" problem. Hopefully they are all solved
>
>
> Thanks,
>
> Bingjing Zhang
>
>
Re: Review Request: aggregator at input super step
Posted by Bingjing Zhang <je...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11525/
-----------------------------------------------------------
(Updated June 19, 2013, 4:29 p.m.)
Review request for giraph.
Description
-------
add aggregator for vertex reader and edge reader at input super step.
Changes in code:
add "aggregate" interface to VertexReader and EdgeReader
initialize "aggregator" in "readInputSplit" in "VertexInputSplitsCallable" and "EdgeInputSplitsCallable"
initialize master aggregator before data loading in "coordinateSuperStep" in "BspServiceMaster"
seperate master aggregator initialization and its computation for Super Step 0 (originally they are mixed in one method "runMasterCompute")
perpare aggregator in "setup" in "BspServiceWorker" before data loading
Test:
Add SimpleVertexReader and SimpleEdgeReader to AggregatorsTestComputation for genreating input data.
Also add "aggregate" call in "getCurrentVertex" and "getCurrentEdge".
Register persistent aggregator with name "INPUT_VERTEX_PERSISTENT_AGG" and "INPUT_EDGE_PERSISTENT_AGG"in AggregatorsTestMasterCompute.initialize
Assert if aggregated values can be retrived in AggregatorsTestMasterCompute.compute when "superstep >= 0"
This addresses bug GIRAPH-673.
https://issues.apache.org/jira/browse/GIRAPH-673
Diffs (updated)
-----
giraph-core/src/main/java/org/apache/giraph/io/EdgeReader.java 363a5e6
giraph-core/src/main/java/org/apache/giraph/io/VertexReader.java b8b82af
giraph-core/src/main/java/org/apache/giraph/io/internal/WrappedEdgeReader.java aae7a72
giraph-core/src/main/java/org/apache/giraph/io/internal/WrappedVertexReader.java 54adfec
giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java bd48116
giraph-core/src/main/java/org/apache/giraph/worker/BspServiceWorker.java eb6d30d
giraph-core/src/main/java/org/apache/giraph/worker/EdgeInputSplitsCallable.java 78cdd8e
giraph-core/src/main/java/org/apache/giraph/worker/VertexInputSplitsCallable.java 977e100
giraph-examples/src/main/java/org/apache/giraph/examples/AggregatorsTestComputation.java db527f2
giraph-examples/src/main/java/org/apache/giraph/examples/GeneratedEdgeReader.java PRE-CREATION
giraph-examples/src/test/java/org/apache/giraph/aggregators/TestAggregatorsHandling.java 6d22800
Diff: https://reviews.apache.org/r/11525/diff/
Testing
-------
Unit Test of persistent aggregator at input super step in modified "AggregatorsTestComputation" is done.
"mvn clean verify" was done on giraph-core
"mvn clean verfiy" was done on giraph-example
rechecked "indent" problem. Hopefully they are all solved
Thanks,
Bingjing Zhang
Re: Review Request: aggregator at input super step
Posted by Bingjing Zhang <je...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11525/
-----------------------------------------------------------
(Updated June 17, 2013, 4:48 p.m.)
Review request for giraph.
Changes
-------
fix some naming and typing issues
Description
-------
add aggregator for vertex reader and edge reader at input super step.
Changes in code:
add "aggregate" interface to VertexReader and EdgeReader
initialize "aggregator" in "readInputSplit" in "VertexInputSplitsCallable" and "EdgeInputSplitsCallable"
initialize master aggregator before data loading in "coordinateSuperStep" in "BspServiceMaster"
seperate master aggregator initialization and its computation for Super Step 0 (originally they are mixed in one method "runMasterCompute")
perpare aggregator in "setup" in "BspServiceWorker" before data loading
Test:
Add SimpleVertexReader and SimpleEdgeReader to AggregatorsTestComputation for genreating input data.
Also add "aggregate" call in "getCurrentVertex" and "getCurrentEdge".
Register persistent aggregator with name "INPUT_VERTEX_PERSISTENT_AGG" and "INPUT_EDGE_PERSISTENT_AGG"in AggregatorsTestMasterCompute.initialize
Assert if aggregated values can be retrived in AggregatorsTestMasterCompute.compute when "superstep >= 0"
This addresses bug GIRAPH-673.
https://issues.apache.org/jira/browse/GIRAPH-673
Diffs (updated)
-----
giraph-core/src/main/java/org/apache/giraph/io/EdgeReader.java 363a5e6
giraph-core/src/main/java/org/apache/giraph/io/VertexReader.java b8b82af
giraph-core/src/main/java/org/apache/giraph/io/internal/WrappedEdgeReader.java aae7a72
giraph-core/src/main/java/org/apache/giraph/io/internal/WrappedVertexReader.java 54adfec
giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java bd48116
giraph-core/src/main/java/org/apache/giraph/worker/BspServiceWorker.java eb6d30d
giraph-core/src/main/java/org/apache/giraph/worker/EdgeInputSplitsCallable.java 78cdd8e
giraph-core/src/main/java/org/apache/giraph/worker/VertexInputSplitsCallable.java 977e100
giraph-examples/src/main/java/org/apache/giraph/examples/AggregatorsTestComputation.java db527f2
giraph-examples/src/main/java/org/apache/giraph/examples/GeneratedEdgeReader.java PRE-CREATION
giraph-examples/src/test/java/org/apache/giraph/aggregators/TestAggregatorsHandling.java 6d22800
Diff: https://reviews.apache.org/r/11525/diff/
Testing
-------
Unit Test of persistent aggregator at input super step in modified "AggregatorsTestComputation" is done.
"mvn clean verify" was done on giraph-core
"mvn clean verfiy" was done on giraph-example
rechecked "indent" problem. Hopefully they are all solved
Thanks,
Bingjing Zhang
Re: Review Request: aggregator at input super step
Posted by Avery Ching <av...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11525/#review21844
-----------------------------------------------------------
Getting closer! =)
I tried to run mvn clean verify with your patch, but ran into the following build issues:
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.0:compile (default-compile) on project giraph-examples: Compilation failure: Compilation failure:
[ERROR] /Users/aching/git/giraph_git/giraph-examples/target/munged/main/org/apache/giraph/examples/AggregatorsTestComputation.java:[231,5] cannot find symbol
[ERROR] symbol : class GeneratedEdgeReader
[ERROR] location: class org.apache.giraph.examples.AggregatorsTestComputation
[ERROR] /Users/aching/git/giraph_git/giraph-examples/target/munged/main/org/apache/giraph/examples/AggregatorsTestComputation.java:[237,14] cannot find symbol
[ERROR] symbol : variable totalRecords
[ERROR] location: class org.apache.giraph.examples.AggregatorsTestComputation.SimpleEdgeReader
[ERROR] /Users/aching/git/giraph_git/giraph-examples/target/munged/main/org/apache/giraph/examples/AggregatorsTestComputation.java:[237,29] cannot find symbol
[ERROR] symbol : variable recordsRead
[ERROR] location: class org.apache.giraph.examples.AggregatorsTestComputation.SimpleEdgeReader
I think you are missing GeneratedEdgeReader in this patch?
giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java
<https://reviews.apache.org/r/11525/#comment45130>
Called 2x?
giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java
<https://reviews.apache.org/r/11525/#comment45129>
is called -> is called
giraph-core/src/main/java/org/apache/giraph/io/VertexReader.java
<https://reviews.apache.org/r/11525/#comment45132>
This extra line is not needed, see other classes.
giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java
<https://reviews.apache.org/r/11525/#comment45133>
INPUT Super Step -> input superstep or INPUT_SUPERSTEP, your choice =)
giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java
<https://reviews.apache.org/r/11525/#comment45134>
can we make this initializeAggregatorInputSuperstep? In other parts of the code, superstep is a single word.
giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java
<https://reviews.apache.org/r/11525/#comment45135>
initializeMasterCompute -> initializeAggregatorInputSuperstep
giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java
<https://reviews.apache.org/r/11525/#comment45136>
initializeMasterCompute -> initializeAggregatorInputSuperstep
giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java
<https://reviews.apache.org/r/11525/#comment45137>
initializeMasterCompute -> initializeAggregatorInputSuperstep
giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java
<https://reviews.apache.org/r/11525/#comment45138>
runMasterCompute -> initializeAggregatorInputSuperstep
giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java
<https://reviews.apache.org/r/11525/#comment45139>
Please remove this line, it only describes what you did in this diff.
- Avery Ching
On June 12, 2013, 3:25 a.m., Bingjing Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11525/
> -----------------------------------------------------------
>
> (Updated June 12, 2013, 3:25 a.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> add aggregator for vertex reader and edge reader at input super step.
>
> Changes in code:
> add "aggregate" interface to VertexReader and EdgeReader
> initialize "aggregator" in "readInputSplit" in "VertexInputSplitsCallable" and "EdgeInputSplitsCallable"
> initialize master aggregator before data loading in "coordinateSuperStep" in "BspServiceMaster"
> seperate master aggregator initialization and its computation for Super Step 0 (originally they are mixed in one method "runMasterCompute")
> perpare aggregator in "setup" in "BspServiceWorker" before data loading
>
> Test:
> Add SimpleVertexReader and SimpleEdgeReader to AggregatorsTestComputation for genreating input data.
> Also add "aggregate" call in "getCurrentVertex" and "getCurrentEdge".
> Register persistent aggregator with name "INPUT_VERTEX_PERSISTENT_AGG" and "INPUT_EDGE_PERSISTENT_AGG"in AggregatorsTestMasterCompute.initialize
> Assert if aggregated values can be retrived in AggregatorsTestMasterCompute.compute when "superstep >= 0"
>
>
> This addresses bug GIRAPH-673.
> https://issues.apache.org/jira/browse/GIRAPH-673
>
>
> Diffs
> -----
>
> giraph-core/src/main/java/org/apache/giraph/io/EdgeReader.java 363a5e6
> giraph-core/src/main/java/org/apache/giraph/io/VertexReader.java 9695169
> giraph-core/src/main/java/org/apache/giraph/io/internal/WrappedEdgeReader.java aae7a72
> giraph-core/src/main/java/org/apache/giraph/io/internal/WrappedVertexReader.java 54adfec
> giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java bd48116
> giraph-core/src/main/java/org/apache/giraph/worker/BspServiceWorker.java 8b5e39a
> giraph-core/src/main/java/org/apache/giraph/worker/EdgeInputSplitsCallable.java 78cdd8e
> giraph-core/src/main/java/org/apache/giraph/worker/VertexInputSplitsCallable.java 977e100
> giraph-examples/src/main/java/org/apache/giraph/examples/AggregatorsTestComputation.java db527f2
> giraph-examples/src/test/java/org/apache/giraph/aggregators/TestAggregatorsHandling.java 6d22800
>
> Diff: https://reviews.apache.org/r/11525/diff/
>
>
> Testing
> -------
>
> Unit Test of persistent aggregator at input super step in modified "AggregatorsTestComputation" is done.
> "mvn clean verify" was done on giraph-core
> "mvn clean verfiy" was done on giraph-example
>
> rechecked "indent" problem. Hopefully they are all solved
>
>
> Thanks,
>
> Bingjing Zhang
>
>
Re: Review Request: aggregator at input super step
Posted by Bingjing Zhang <je...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11525/
-----------------------------------------------------------
(Updated June 12, 2013, 3:25 a.m.)
Review request for giraph.
Changes
-------
update comments, remove unnecessary functions.
Description
-------
add aggregator for vertex reader and edge reader at input super step.
Changes in code:
add "aggregate" interface to VertexReader and EdgeReader
initialize "aggregator" in "readInputSplit" in "VertexInputSplitsCallable" and "EdgeInputSplitsCallable"
initialize master aggregator before data loading in "coordinateSuperStep" in "BspServiceMaster"
seperate master aggregator initialization and its computation for Super Step 0 (originally they are mixed in one method "runMasterCompute")
perpare aggregator in "setup" in "BspServiceWorker" before data loading
Test:
Add SimpleVertexReader and SimpleEdgeReader to AggregatorsTestComputation for genreating input data.
Also add "aggregate" call in "getCurrentVertex" and "getCurrentEdge".
Register persistent aggregator with name "INPUT_VERTEX_PERSISTENT_AGG" and "INPUT_EDGE_PERSISTENT_AGG"in AggregatorsTestMasterCompute.initialize
Assert if aggregated values can be retrived in AggregatorsTestMasterCompute.compute when "superstep >= 0"
This addresses bug GIRAPH-673.
https://issues.apache.org/jira/browse/GIRAPH-673
Diffs (updated)
-----
giraph-core/src/main/java/org/apache/giraph/io/EdgeReader.java 363a5e6
giraph-core/src/main/java/org/apache/giraph/io/VertexReader.java 9695169
giraph-core/src/main/java/org/apache/giraph/io/internal/WrappedEdgeReader.java aae7a72
giraph-core/src/main/java/org/apache/giraph/io/internal/WrappedVertexReader.java 54adfec
giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java bd48116
giraph-core/src/main/java/org/apache/giraph/worker/BspServiceWorker.java 8b5e39a
giraph-core/src/main/java/org/apache/giraph/worker/EdgeInputSplitsCallable.java 78cdd8e
giraph-core/src/main/java/org/apache/giraph/worker/VertexInputSplitsCallable.java 977e100
giraph-examples/src/main/java/org/apache/giraph/examples/AggregatorsTestComputation.java db527f2
giraph-examples/src/test/java/org/apache/giraph/aggregators/TestAggregatorsHandling.java 6d22800
Diff: https://reviews.apache.org/r/11525/diff/
Testing
-------
Unit Test of persistent aggregator at input super step in modified "AggregatorsTestComputation" is done.
"mvn clean verify" was done on giraph-core
"mvn clean verfiy" was done on giraph-example
rechecked "indent" problem. Hopefully they are all solved
Thanks,
Bingjing Zhang
Re: Review Request: aggregator at input super step
Posted by Avery Ching <av...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11525/#review21656
-----------------------------------------------------------
I have a few style comments as well as code reuse questions.
giraph-core/src/main/java/org/apache/giraph/io/EdgeReader.java
<https://reviews.apache.org/r/11525/#comment44750>
Missing a space r*/ -> r */
giraph-core/src/main/java/org/apache/giraph/io/VertexReader.java
<https://reviews.apache.org/r/11525/#comment44749>
missing a space r*/ -> r */
giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java
<https://reviews.apache.org/r/11525/#comment44756>
at -> at the
giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java
<https://reviews.apache.org/r/11525/#comment44782>
Doesn't this code do exactly the same thing as runMasterCompute? By the way, I think there is a bug as MasterCompute#initialize will be called 2x right? Here and also in runMasterCompute?
giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java
<https://reviews.apache.org/r/11525/#comment44757>
If you are typing masterCompute, we should probably just use MasterCompute everything (the actual class name).
giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java
<https://reviews.apache.org/r/11525/#comment44758>
Given that this method only is basically one line, it shouldn't be its own method.
giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java
<https://reviews.apache.org/r/11525/#comment44751>
Extra space
after masterCompute -> after masterCompute
Should probably be something like
"This method is called only after masterCompute is initialized"
giraph-core/src/main/java/org/apache/giraph/worker/BspServiceWorker.java
<https://reviews.apache.org/r/11525/#comment44752>
edge loading -> edge loading
giraph-core/src/main/java/org/apache/giraph/worker/EdgeInputSplitsCallable.java
<https://reviews.apache.org/r/11525/#comment44753>
//I -> // I
giraph-core/src/main/java/org/apache/giraph/worker/EdgeInputSplitsCallable.java
<https://reviews.apache.org/r/11525/#comment44754>
//S -> // S
Actually, this comment is not necessary. There is no such comment in ComputeCallable.call.
giraph-core/src/main/java/org/apache/giraph/worker/VertexInputSplitsCallable.java
<https://reviews.apache.org/r/11525/#comment44755>
//S -> // S
Actually, this comment is not necessary. There is no such comment in ComputeCallable.call.
- Avery Ching
On June 5, 2013, 1:29 a.m., Bingjing Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11525/
> -----------------------------------------------------------
>
> (Updated June 5, 2013, 1:29 a.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> add aggregator for vertex reader and edge reader at input super step.
>
> Changes in code:
> add "aggregate" interface to VertexReader and EdgeReader
> initialize "aggregator" in "readInputSplit" in "VertexInputSplitsCallable" and "EdgeInputSplitsCallable"
> initialize master aggregator before data loading in "coordinateSuperStep" in "BspServiceMaster"
> seperate master aggregator initialization and its computation for Super Step 0 (originally they are mixed in one method "runMasterCompute")
> perpare aggregator in "setup" in "BspServiceWorker" before data loading
>
> Test:
> Add SimpleVertexReader and SimpleEdgeReader to AggregatorsTestComputation for genreating input data.
> Also add "aggregate" call in "getCurrentVertex" and "getCurrentEdge".
> Register persistent aggregator with name "INPUT_VERTEX_PERSISTENT_AGG" and "INPUT_EDGE_PERSISTENT_AGG"in AggregatorsTestMasterCompute.initialize
> Assert if aggregated values can be retrived in AggregatorsTestMasterCompute.compute when "superstep >= 0"
>
>
> This addresses bug GIRAPH-673.
> https://issues.apache.org/jira/browse/GIRAPH-673
>
>
> Diffs
> -----
>
> giraph-core/src/main/java/org/apache/giraph/io/EdgeReader.java 363a5e6
> giraph-core/src/main/java/org/apache/giraph/io/VertexReader.java 9695169
> giraph-core/src/main/java/org/apache/giraph/io/internal/WrappedEdgeReader.java aae7a72
> giraph-core/src/main/java/org/apache/giraph/io/internal/WrappedVertexReader.java 54adfec
> giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java bd48116
> giraph-core/src/main/java/org/apache/giraph/worker/BspServiceWorker.java 8b5e39a
> giraph-core/src/main/java/org/apache/giraph/worker/EdgeInputSplitsCallable.java 78cdd8e
> giraph-core/src/main/java/org/apache/giraph/worker/VertexInputSplitsCallable.java 977e100
> giraph-examples/src/main/java/org/apache/giraph/examples/AggregatorsTestComputation.java db527f2
> giraph-examples/src/test/java/org/apache/giraph/aggregators/TestAggregatorsHandling.java 6d22800
>
> Diff: https://reviews.apache.org/r/11525/diff/
>
>
> Testing
> -------
>
> Unit Test of persistent aggregator at input super step in modified "AggregatorsTestComputation" is done.
> "mvn clean verify" was done on giraph-core
> "mvn clean verfiy" was done on giraph-example
>
> rechecked "indent" problem. Hopefully they are all solved
>
>
> Thanks,
>
> Bingjing Zhang
>
>
Re: Review Request: aggregator at input super step
Posted by Bingjing Zhang <je...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11525/
-----------------------------------------------------------
(Updated June 5, 2013, 1:29 a.m.)
Review request for giraph.
Changes
-------
resloved issues
Description
-------
add aggregator for vertex reader and edge reader at input super step.
Changes in code:
add "aggregate" interface to VertexReader and EdgeReader
initialize "aggregator" in "readInputSplit" in "VertexInputSplitsCallable" and "EdgeInputSplitsCallable"
initialize master aggregator before data loading in "coordinateSuperStep" in "BspServiceMaster"
seperate master aggregator initialization and its computation for Super Step 0 (originally they are mixed in one method "runMasterCompute")
perpare aggregator in "setup" in "BspServiceWorker" before data loading
Test:
Add SimpleVertexReader and SimpleEdgeReader to AggregatorsTestComputation for genreating input data.
Also add "aggregate" call in "getCurrentVertex" and "getCurrentEdge".
Register persistent aggregator with name "INPUT_VERTEX_PERSISTENT_AGG" and "INPUT_EDGE_PERSISTENT_AGG"in AggregatorsTestMasterCompute.initialize
Assert if aggregated values can be retrived in AggregatorsTestMasterCompute.compute when "superstep >= 0"
This addresses bug GIRAPH-673.
https://issues.apache.org/jira/browse/GIRAPH-673
Diffs (updated)
-----
giraph-core/src/main/java/org/apache/giraph/io/EdgeReader.java 363a5e6
giraph-core/src/main/java/org/apache/giraph/io/VertexReader.java 9695169
giraph-core/src/main/java/org/apache/giraph/io/internal/WrappedEdgeReader.java aae7a72
giraph-core/src/main/java/org/apache/giraph/io/internal/WrappedVertexReader.java 54adfec
giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java bd48116
giraph-core/src/main/java/org/apache/giraph/worker/BspServiceWorker.java 8b5e39a
giraph-core/src/main/java/org/apache/giraph/worker/EdgeInputSplitsCallable.java 78cdd8e
giraph-core/src/main/java/org/apache/giraph/worker/VertexInputSplitsCallable.java 977e100
giraph-examples/src/main/java/org/apache/giraph/examples/AggregatorsTestComputation.java db527f2
giraph-examples/src/test/java/org/apache/giraph/aggregators/TestAggregatorsHandling.java 6d22800
Diff: https://reviews.apache.org/r/11525/diff/
Testing
-------
Unit Test of persistent aggregator at input super step in modified "AggregatorsTestComputation" is done.
"mvn clean verify" was done on giraph-core
"mvn clean verfiy" was done on giraph-example
rechecked "indent" problem. Hopefully they are all solved
Thanks,
Bingjing Zhang
Re: Review Request: aggregator at input super step
Posted by Bingjing Zhang <je...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11525/
-----------------------------------------------------------
(Updated May 30, 2013, 4:46 p.m.)
Review request for giraph.
Description
-------
add aggregator for vertex reader and edge reader at input super step.
Changes in code:
add "aggregate" interface to VertexReader and EdgeReader
initialize "aggregator" in "readInputSplit" in "VertexInputSplitsCallable" and "EdgeInputSplitsCallable"
initialize master aggregator before data loading in "coordinateSuperStep" in "BspServiceMaster"
seperate master aggregator initialization and its computation for Super Step 0 (originally they are mixed in one method "runMasterCompute")
perpare aggregator in "setup" in "BspServiceWorker" before data loading
Test:
Add SimpleVertexReader and SimpleEdgeReader to AggregatorsTestComputation for genreating input data.
Also add "aggregate" call in "getCurrentVertex" and "getCurrentEdge".
Register persistent aggregator with name "INPUT_VERTEX_PERSISTENT_AGG" and "INPUT_EDGE_PERSISTENT_AGG"in AggregatorsTestMasterCompute.initialize
Assert if aggregated values can be retrived in AggregatorsTestMasterCompute.compute when "superstep >= 0"
This addresses bug GIRAPH-673.
https://issues.apache.org/jira/browse/GIRAPH-673
Diffs (updated)
-----
giraph-core/src/main/java/org/apache/giraph/io/EdgeReader.java 363a5e6
giraph-core/src/main/java/org/apache/giraph/io/VertexReader.java 9695169
giraph-core/src/main/java/org/apache/giraph/io/internal/WrappedEdgeReader.java aae7a72
giraph-core/src/main/java/org/apache/giraph/io/internal/WrappedVertexReader.java 54adfec
giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java bd48116
giraph-core/src/main/java/org/apache/giraph/worker/BspServiceWorker.java 8b5e39a
giraph-core/src/main/java/org/apache/giraph/worker/EdgeInputSplitsCallable.java 78cdd8e
giraph-core/src/main/java/org/apache/giraph/worker/VertexInputSplitsCallable.java 977e100
giraph-examples/src/main/java/org/apache/giraph/examples/AggregatorsTestComputation.java db527f2
giraph-examples/src/test/java/org/apache/giraph/aggregators/TestAggregatorsHandling.java 6d22800
Diff: https://reviews.apache.org/r/11525/diff/
Testing (updated)
-------
Unit Test of persistent aggregator at input super step in modified "AggregatorsTestComputation" is done.
"mvn clean verify" was done on giraph-core
"mvn clean verfiy" was done on giraph-example
rechecked "indent" problem. Hopefully they are all solved
Thanks,
Bingjing Zhang
Re: Review Request: aggregator at input super step
Posted by Bingjing Zhang <je...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11525/
-----------------------------------------------------------
(Updated May 29, 2013, 11:38 p.m.)
Review request for giraph.
Description
-------
add aggregator for vertex reader and edge reader at input super step.
Changes in code:
add "aggregate" interface to VertexReader and EdgeReader
initialize "aggregator" in "readInputSplit" in "VertexInputSplitsCallable" and "EdgeInputSplitsCallable"
initialize master aggregator before data loading in "coordinateSuperStep" in "BspServiceMaster"
seperate master aggregator initialization and its computation for Super Step 0 (originally they are mixed in one method "runMasterCompute")
perpare aggregator in "setup" in "BspServiceWorker" before data loading
Test:
Add SimpleVertexReader and SimpleEdgeReader to AggregatorsTestComputation for genreating input data.
Also add "aggregate" call in "getCurrentVertex" and "getCurrentEdge".
Register persistent aggregator with name "INPUT_VERTEX_PERSISTENT_AGG" and "INPUT_EDGE_PERSISTENT_AGG"in AggregatorsTestMasterCompute.initialize
Assert if aggregated values can be retrived in AggregatorsTestMasterCompute.compute when "superstep >= 0"
This addresses bug GIRAPH-673.
https://issues.apache.org/jira/browse/GIRAPH-673
Diffs
-----
giraph-core/src/main/java/org/apache/giraph/io/EdgeReader.java 363a5e6
giraph-core/src/main/java/org/apache/giraph/io/VertexReader.java 9695169
giraph-core/src/main/java/org/apache/giraph/io/internal/WrappedEdgeReader.java aae7a72
giraph-core/src/main/java/org/apache/giraph/io/internal/WrappedVertexReader.java 54adfec
giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java bd48116
giraph-core/src/main/java/org/apache/giraph/worker/BspServiceWorker.java 8b5e39a
giraph-core/src/main/java/org/apache/giraph/worker/EdgeInputSplitsCallable.java 78cdd8e
giraph-core/src/main/java/org/apache/giraph/worker/VertexInputSplitsCallable.java 977e100
giraph-core/src/test/java/org/apache/giraph/master/TestSwitchClasses.java 4069972
giraph-examples/src/main/java/org/apache/giraph/examples/AggregatorsTestComputation.java db527f2
giraph-examples/src/test/java/org/apache/giraph/aggregators/TestAggregatorsHandling.java 6d22800
Diff: https://reviews.apache.org/r/11525/diff/
Testing (updated)
-------
Unit Test of persistent aggregator at input super step in modified "AggregatorsTestComputation" is done.
"mvn clean verify" was done on giraph-core
"mvn clean verfiy" was done on giraph-example
Thanks,
Bingjing Zhang
Re: Review Request: aggregator at input super step
Posted by Bingjing Zhang <je...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11525/
-----------------------------------------------------------
(Updated May 29, 2013, 5:41 p.m.)
Review request for giraph.
Description (updated)
-------
add aggregator for vertex reader and edge reader at input super step.
Changes in code:
add "aggregate" interface to VertexReader and EdgeReader
initialize "aggregator" in "readInputSplit" in "VertexInputSplitsCallable" and "EdgeInputSplitsCallable"
initialize master aggregator before data loading in "coordinateSuperStep" in "BspServiceMaster"
seperate master aggregator initialization and its computation for Super Step 0 (originally they are mixed in one method "runMasterCompute")
perpare aggregator in "setup" in "BspServiceWorker" before data loading
Test:
Add SimpleVertexReader and SimpleEdgeReader to AggregatorsTestComputation for genreating input data.
Also add "aggregate" call in "getCurrentVertex" and "getCurrentEdge".
Register persistent aggregator with name "INPUT_VERTEX_PERSISTENT_AGG" and "INPUT_EDGE_PERSISTENT_AGG"in AggregatorsTestMasterCompute.initialize
Assert if aggregated values can be retrived in AggregatorsTestMasterCompute.compute when "superstep >= 0"
This addresses bug GIRAPH-673.
https://issues.apache.org/jira/browse/GIRAPH-673
Diffs
-----
giraph-core/src/main/java/org/apache/giraph/io/EdgeReader.java 363a5e6
giraph-core/src/main/java/org/apache/giraph/io/VertexReader.java 9695169
giraph-core/src/main/java/org/apache/giraph/io/internal/WrappedEdgeReader.java aae7a72
giraph-core/src/main/java/org/apache/giraph/io/internal/WrappedVertexReader.java 54adfec
giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java bd48116
giraph-core/src/main/java/org/apache/giraph/worker/BspServiceWorker.java 8b5e39a
giraph-core/src/main/java/org/apache/giraph/worker/EdgeInputSplitsCallable.java 78cdd8e
giraph-core/src/main/java/org/apache/giraph/worker/VertexInputSplitsCallable.java 977e100
giraph-core/src/test/java/org/apache/giraph/master/TestSwitchClasses.java 4069972
giraph-examples/src/main/java/org/apache/giraph/examples/AggregatorsTestComputation.java db527f2
giraph-examples/src/test/java/org/apache/giraph/aggregators/TestAggregatorsHandling.java 6d22800
Diff: https://reviews.apache.org/r/11525/diff/
Testing (updated)
-------
Unit Test of persistent aggregator at input super step in modified "AggregatorsTestComputation" is done.
Thanks,
Bingjing Zhang