> On Aug. 3, 2013, 12:03 a.m., Alessandro Presta wrote: > > giraph-core/src/main/java/org/apache/giraph/aggregators/matrix/Check.java, > > line 22 > > <https://reviews.apache.org/r/13248/diff/1/?file=334675#file334675line22> > > > > You can use Guava's Preconditions instead of this.
Removed it :-) > On Aug. 3, 2013, 12:03 a.m., Alessandro Presta wrote: > > giraph-core/src/main/java/org/apache/giraph/aggregators/matrix/DoubleMatrix.java, > > line 42 > > <https://reviews.apache.org/r/13248/diff/1/?file=334676#file334676line42> > > > > I would call this initialize(). done! > On Aug. 3, 2013, 12:03 a.m., Alessandro Presta wrote: > > giraph-core/src/main/java/org/apache/giraph/aggregators/matrix/DoubleVector.java, > > line 89 > > <https://reviews.apache.org/r/13248/diff/1/?file=334678#file334678line89> > > > > I think this example is overkill. Just say it does element-by-element > > addition. done! > On Aug. 3, 2013, 12:03 a.m., Alessandro Presta wrote: > > giraph-core/src/main/java/org/apache/giraph/aggregators/matrix/DoubleVector.java, > > line 96 > > <https://reviews.apache.org/r/13248/diff/1/?file=334678#file334678line96> > > > > kv -> entry done! > On Aug. 3, 2013, 12:03 a.m., Alessandro Presta wrote: > > giraph-core/src/test/java/org/apache/giraph/aggregators/matrix/InMemoryIO.java, > > line 32 > > <https://reviews.apache.org/r/13248/diff/1/?file=334693#file334693line32> > > > > I think you can use WritableUtils instead of this. Take a look at how > > TestVertexAndEdges uses it. done! > On Aug. 3, 2013, 12:03 a.m., Alessandro Presta wrote: > > giraph-core/src/main/java/org/apache/giraph/aggregators/matrix/DoubleVector.java, > > line 40 > > <https://reviews.apache.org/r/13248/diff/1/?file=334678#file334678line40> > > > > Why do we initialize to 0 length in absence of hints? This can lead to > > a suboptimal reallocation pattern. I would stick with Fastutil's default, > > by constructing the hash map with no arguments. I used Int2DoubleOpenHashMap.DEFAULT_INITIAL_SIZE instead of no arguments in order not to have an if statement inside initialize or copy the code in the constructor. If you want I can use the default constructor :-) - Herald ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13248/#review24595 ----------------------------------------------------------- On Aug. 2, 2013, 11:07 p.m., Herald Kllapi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/13248/ > ----------------------------------------------------------- > > (Updated Aug. 2, 2013, 11:07 p.m.) > > > Review request for giraph. > > > Repository: giraph-git > > > Description > ------- > > In applications where a matrix is needed, is not efficient to have an > aggregator per entry. This update provides the same functionality with an > aggregator per matrix row. > > > Diffs > ----- > > giraph-core/src/main/java/org/apache/giraph/aggregators/matrix/Check.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/aggregators/matrix/DoubleMatrix.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/aggregators/matrix/DoubleMatrixSumAggregator.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/aggregators/matrix/DoubleVector.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/aggregators/matrix/DoubleVectorSumAggregator.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/aggregators/matrix/FloatMatrix.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/aggregators/matrix/FloatMatrixSumAggregator.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/aggregators/matrix/FloatVector.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/aggregators/matrix/FloatVectorSumAggregator.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/aggregators/matrix/IntMatrix.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/aggregators/matrix/IntMatrixSumAggregator.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/aggregators/matrix/IntVector.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/aggregators/matrix/IntVectorSumAggregator.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/aggregators/matrix/LongMatrix.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/aggregators/matrix/LongMatrixSumAggregator.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/aggregators/matrix/LongVector.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/aggregators/matrix/LongVectorSumAggregator.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/aggregators/matrix/package-info.java > PRE-CREATION > > giraph-core/src/test/java/org/apache/giraph/aggregators/matrix/InMemoryIO.java > PRE-CREATION > > giraph-core/src/test/java/org/apache/giraph/aggregators/matrix/TestDoubleMatrix.java > PRE-CREATION > > giraph-core/src/test/java/org/apache/giraph/aggregators/matrix/TestFloatMatrix.java > PRE-CREATION > > giraph-core/src/test/java/org/apache/giraph/aggregators/matrix/TestIntMatrix.java > PRE-CREATION > > giraph-core/src/test/java/org/apache/giraph/aggregators/matrix/TestLongMatrix.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/13248/diff/ > > > Testing > ------- > > We provide test classes to test the functionality. > > > Thanks, > > Herald Kllapi > >
