[Dune] RFC 01/2011: replace row_type & by row_reference in DenseMatrix

[Oliver Sander]

Let me follow up on this.  In the wake of Martin's change
Carsten and I have been discussing the semantics of the
DenseMatrix class.  Is it a container, and if yes, of what?

Many features of the current implementation make you think
that a matrix is a container for its rows.  For example, begin()
and end() methods provide you with iterators over the rows,
and operator[] gives a row, too.  These are all container
methods.  On the other hand, the value_type typedef does
_not_ return the type of the row, but the type of a matrix
entry.  One may call this inconsistent.

Now if a matrix really is a container of its rows, then we may
want to consider if it makes sense to have it more standard-
conforming.  In particular this would mean that the new
typedef row_reference should better be called 'reference'.

cheers,
Oliver


Am 17.10.2011 13:46, schrieb Carsten Gräser:
> Hi Martin,
> I really like this change. It allows to reflect what
> is already implemented in ScaledIdentityMatrix and
> DiagonalMatrix. However I used a typedef named reference
> there as it's done in std.:vector<bool>.
>
> In fact I'd prefer to call it 'reference' because that's
> conforming with the stl. But DenseMatrix differs from stl
> containers anyway because value_type is not the type
> of the values the container contains.
>
> We should clearly state that one should use row_reference
> instead of row_type&  from now on, in order to write
> portable code.
>
> Best,
> Carsten
>
>
> Am 14.10.2011 15:03, schrieb Martin Nolte:
>    
>> Dear all,
>>
>> I have replaced the return value of the operator[] of a DenseMatrix
>> (which was row_type&) by a row_reference, which is exported from the
>> MatVecTraits. For the corresponding matrix implementations, i.e.,
>> FieldMatrix and DynamicMatrix, I added the corresponding typedefs
>> (row_reference and const_row_reference) to their MatVecTraits.
>>
>> This change has very little impact on your current code, as no types are
>> actually changed, but allows you to build a DenseMatrix that has a
>> single FieldVector as container for its values.
>>
>> Despite the heavy use of FieldMatrix in the core modules, few lines
>> actually obtain the row_type from the FieldMatrix and would have to be
>> changed. Actually, the word row_type does not occur in any header of
>> dune-grid or dune-localfunctions, but only in dune-common and dune-istl.
>> In dune-common, the only place I did not change is exprtmpl.hh, but I
>> guess this is not a big problem. In dune-istl, there are only two lines
>> actually using row_type&, namely:
>>
>> diagonalmatrix.hh:575:
>> row_ = row_type(&(mat_->diagonal(i)), i);
>>
>> io.hh:441:
>> const typename MatrixType::row_type&  row = matrix[rowIdx];
>>
>> So nearly all current code makes use of explicit knowledge of what
>> row_type and, hence, row_type&, actually is. Therefore, I think the
>> only files that might need to be changed are those mentioned. However,
>> for dune-istl, this might break code for other kinds of matrices that do
>> not, yet, support a row_reference. Maybe we have to add this to
>> dune-istl, too?
>>
>> Since there has already been some discussion on this, I would like to
>> hear your comments, opinions, and suggestions. It would also be great to
>> know whether someone objects.
>>
>> As the change is not very intrusive to existing codes, I will leave the
>> commit in place until the discussion is over. I hope there are no
>> objections to this.
>>
>> Best,
>>
>> Martin
>>      
> _______________________________________________
> Dune mailing list
> Dune at dune-project.org
> http://lists.dune-project.org/mailman/listinfo/dune
>    


-- 
************************************************************************
* Oliver Sander                ** email: sander at mi.fu-berlin.de        *
* Freie Universität Berlin     ** phone: + 49 (30) 838 75348           *
* Institut für Mathematik      ** URL  : page.mi.fu-berlin.de/~sander  *
* Arnimallee 6                 ** -------------------------------------*
* 14195 Berlin, Germany        ** Member of MATHEON (www.matheon.de)   *
************************************************************************