> On Sept. 9, 2013, 6:34 p.m., Alessandro Presta wrote: > > 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)?
It does make sense! However, is it possible to register aggregators in the worker? If not, than we cannot aggregate rows dynamically. Unless you are suggesting to not get all the rows at the beginning and do that lazily after getMatrix(...) call. :-) > On Sept. 9, 2013, 6:34 p.m., Alessandro Presta wrote: > > giraph-core/src/main/java/org/apache/giraph/aggregators/denseMatrix/DenseMatrixSumAggregator.java, > > line 28 > > <https://reviews.apache.org/r/14025/diff/1/?file=349145#file349145line28> > > > > Let's do this as part of this diff. done! > On Sept. 9, 2013, 6:34 p.m., Alessandro Presta wrote: > > giraph-core/src/main/java/org/apache/giraph/aggregators/denseMatrix/DoubleDenseVector.java, > > line 30 > > <https://reviews.apache.org/r/14025/diff/1/?file=349148#file349148line30> > > > > We should probably briefly explain the "singleton" bit here. done! > On Sept. 9, 2013, 6:34 p.m., Alessandro Presta wrote: > > giraph-core/src/main/java/org/apache/giraph/aggregators/denseMatrix/DenseMatrixSumAggregator.java, > > line 19 > > <https://reviews.apache.org/r/14025/diff/1/?file=349145#file349145line19> > > > > 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? I agree! Is better :-) I did it this way in order to leave the current code as is in the case that someone else is using it! - Herald ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14025/#review25995 ----------------------------------------------------------- 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 > >
