----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14025/#review25995 -----------------------------------------------------------
One comment on sparse vs dense: the current version uses sparse vectors for rows, but still explicitly creates all rows. Now that we're going to have this dense option, do you think it makes sense/is doable to make the current one fully sparse (i.e., create rows on demand)? giraph-core/src/main/java/org/apache/giraph/aggregators/denseMatrix/DenseMatrixSumAggregator.java <https://reviews.apache.org/r/14025/#comment50747> I think we use underscores in this codebase. Anyway, I suggest leaving a common matrix package and adding subpackages matrix.dense, matrix.sparse. What do you think? giraph-core/src/main/java/org/apache/giraph/aggregators/denseMatrix/DenseMatrixSumAggregator.java <https://reviews.apache.org/r/14025/#comment50746> Let's do this as part of this diff. giraph-core/src/main/java/org/apache/giraph/aggregators/denseMatrix/DoubleDenseVector.java <https://reviews.apache.org/r/14025/#comment50745> We should probably briefly explain the "singleton" bit here. - Alessandro Presta On Sept. 6, 2013, 11:01 p.m., Herald Kllapi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/14025/ > ----------------------------------------------------------- > > (Updated Sept. 6, 2013, 11:01 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. This implementation uses an array per row and is > efficient when the matrices are dense. > > > Diffs > ----- > > > giraph-core/src/main/java/org/apache/giraph/aggregators/denseMatrix/DenseMatrixSumAggregator.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/aggregators/denseMatrix/DoubleDenseMatrix.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/aggregators/denseMatrix/DoubleDenseMatrixSumAggregator.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/aggregators/denseMatrix/DoubleDenseVector.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/aggregators/denseMatrix/DoubleDenseVectorSumAggregator.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/aggregators/denseMatrix/FloatDenseMatrix.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/aggregators/denseMatrix/FloatDenseMatrixSumAggregator.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/aggregators/denseMatrix/FloatDenseVector.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/aggregators/denseMatrix/FloatDenseVectorSumAggregator.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/aggregators/denseMatrix/IntDenseMatrix.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/aggregators/denseMatrix/IntDenseMatrixSumAggregator.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/aggregators/denseMatrix/IntDenseVector.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/aggregators/denseMatrix/IntDenseVectorSumAggregator.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/aggregators/denseMatrix/LongDenseMatrix.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/aggregators/denseMatrix/LongDenseMatrixSumAggregator.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/aggregators/denseMatrix/LongDenseVector.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/aggregators/denseMatrix/LongDenseVectorSumAggregator.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/aggregators/denseMatrix/package-info.java > PRE-CREATION > > giraph-core/src/test/java/org/apache/giraph/aggregators/denseMatrix/TestDoubleDenseMatrix.java > PRE-CREATION > > giraph-core/src/test/java/org/apache/giraph/aggregators/denseMatrix/TestFloatDenseMatrix.java > PRE-CREATION > > giraph-core/src/test/java/org/apache/giraph/aggregators/denseMatrix/TestIntDenseMatrix.java > PRE-CREATION > > giraph-core/src/test/java/org/apache/giraph/aggregators/denseMatrix/TestLongDenseMatrix.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/14025/diff/ > > > Testing > ------- > > > Thanks, > > Herald Kllapi > >
