istl_assign_to_fmatrix is broken
I'm trying to break up issue #5 (closed) into manageable pieces here.
It's currently apparently meant to be possible to construct a DenseMatrix
from a C-style matrix through the function istl_assign_to_fmatrix
defined in densematrix.hh.
One can use the function directly or through DenseMatrix::operator=
and thus also in the FieldMatrix
copy constructor. All of this will compile and is apparently supposed to:
double sourceMatrix23[2][3] = { { 0, 1, 2 }, { 3, 4, 5 } };
{
Dune::FieldMatrix<double, 2, 3> const targetMatrix = sourceMatrix23;
}
{
Dune::FieldMatrix<double, 2, 3> targetMatrix;
targetMatrix = sourceMatrix23;
}
{
Dune::FieldMatrix<double, 2, 3> targetMatrix;
istl_assign_to_fmatrix(targetMatrix, sourceMatrix23);
}
{
Dune::DynamicMatrix<double> targetMatrix(2, 3);
targetMatrix = sourceMatrix23;
}
{
Dune::DynamicMatrix<double> targetMatrix(2, 3);
istl_assign_to_fmatrix(targetMatrix, sourceMatrix23);
}
None of this does anything reasonable at runtime, though. If you look at the implementation, i.e.
template< class DenseMatrix, class K, int N, int M >
void istl_assign_to_fmatrix ( DenseMatrix &denseMatrix, const K (&values)[ M ][ N ] )
{
for( int i = 0; i < N; ++i )
for( int j = 0; j < M; ++j )
denseMatrix[ i ][ j ] = values[ i ][ j ];
}
you will see that when the signature matches a double[2][3]
, it is actually treated as a double[3][2]
in the body of the loop. With assertions enabled, the program will fail to run because it'll try to write to the entry [2][0]
of the target matrix, which does not exist.
For a square matrix, the implementation is fine as-is. The fix here is obvious. The question is: Since it seems nobody out there is using this functionality (and it might have made more sense in a time before initializer lists): Is it maybe appropriate to remove the function entirely?