giraph-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Herald Kllapi" <heral...@gmail.com>
Subject Re: Review Request 14025: Dense matrix aggregator implementation.
Date Mon, 09 Sep 2013 23:01:02 GMT


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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message