Re: [eigen] Sparse Module Patch

["Gael Guennebaud" <gael.guennebaud@xxxxxxxxx>]

Hi Daniel,

sorry I was not very helpful to guide you with that stuff... actually
now I remember what was the big deal  with m.row(i) += xpr; etc....

In fact, m.row(i) += xpr; should not be allowed in the general case
since this might create new non zero entries... so here we need to
first create a setter/wrapper which allows such expressions. If we
assume m is row-major, then we have a case of "outer coherent update"
since the inner vector m.row(i) is updated randomly. Moreover it is
very likely that the current vector m.row(i) is updated several time
before we process the next row m.row(i+1).  A typical example is in
SparseProduct.h where the lines 161-248 could be implemented as:

{
- get an outer coherent wrapper w of the matrix m
  for (int j=0; j<cols; ++j)
  {
     w.processInner(j);
     for (typename Rhs::InnerIterator rhsIt(rhs, j); rhsIt; ++rhsIt)
        w.currentInner() += rhsIt.value() * lhs.col(rhsIt.index());
  }
}  // delete the wrapper

w.processInner(j) would estimate the ratio of non zero entries and
according to this ratio it will automatically use a temporary dense
vector (lines 169-186) or a temporary custom linked list...(lines
190-248)  Perhaps, w.processInner() could take an optional argument to
specify this ratio manually. (there are some details on the wiki) Then
w.currentInner() would returns a proxy object with an efficient
InnerIterator implementation. If it happens the default implementation
of operator + and operator = using InnerIterators is too slow, then
you might overwrite the operator =, (and perhaps += and -= as well) ,
I don't know !

For the API of this "Outer-Coherent-Setter", do as you want...  maybe
a inner(int i) is sufficient (internally it can detect a change of
current row or column and does the initialization/updates if needed)

That let me thought that SparseSetter could take an additional
template (or non template ?) parameter to specify if we want to "set
only" the matrix or update it as well.


BTW, if you only have small trivial changes in the Core module feel
free to commit your current modif so that we can give you some
feedback, the Sparse module is currently pretty messy anyway, so don't
worry !

feel free to ask if some parts are not very clear or if you think
there is a better way to achieve the same goal !

gael.

On Thu, Aug 21, 2008 at 6:12 PM, Daniel Gómez <dgomezferro@xxxxxxxxx> wrote:
> Hi,
>
> I've added a simple test for the Sparse Module. Right now it just initializes
> and accesses a Sparse Matrix, but I found some bugs in
> SparseMatrix::coeff/coeffRef thanks to it.
>
> I attach the diff. Please review it carefully as I'm not an experienced
> programmer.
>
> Thank you :)
>
> Daniel
>