Re: [eigen] Commenting issue #110, disabling operator[] for matrices

[Benoit Jacob <jacob.benoit.1@xxxxxxxxx>]

There is a real, very important use case for linear index-based access
of matrices, i.e. matrix(index).

Take an operation like:

   matrix3 = matrix1 + matrix2;

Naively one can implement it like

for(i...)
  for(j...)
    matrix3(i,j) = matrix1(i,j) + matrix2(i,j);

But one can do much better:
for(i...)
  matrix3(i) = matrix1(i) + matrix2(i);

This is much better because:
1) it's just faster (if only because it avoids nontrivial offset computations)
2) it's even faster as it can vectorize a larger part of the
computation, since we don't have to deal with unaligned boundaries in
every column (or row).
3) it generates smaller binary code

Eigen is able to understand automatically when an operation like that
can be performed using such a linear index-based traversal. The
accessor it uses internally is the coeff(int index) method.

So in any case, we keep coeff(int index).

Then, it would be slightly (no big deal) inconsistent to remove
operator()(int index)    (the difference being that operator()() does
assertions, while coeff() doesn't). If we allow the unsafe coeff()
version, why not allow the safe operator()() version? And the fact
that we are using it shows that there is a use case.

Another argument is that "the user mistakenly believes the matrix to
be a vector" is a rather uncommon kind of mistake, no?

Benoit


2010/4/22 Márton Danóczy <marton78@xxxxxxxxx>:
> In my opinion matrix(i) should be disallowed as well, I'd guess in
> most of the cases matrix(i) means that the user mistakenly believes
> the matrix to be a vector. I'd go for the more verbose but
> self-documenting .asVector(i).
>
> Just my two cents.
>
> Marton
>
> On 22 April 2010 14:34, Benoit Jacob <jacob.benoit.1@xxxxxxxxx> wrote:
>> Hi,
>>
>> https://bitbucket.org/eigen/eigen/issue/110/disabling-operator-for-a-matrix
>>
>> here we are discussing disabling operator[] on matrices.
>>
>> We can perfectly disable it on matrices while keeping it on vectors
>> (e.g. with a static assertion).
>>
>> The motivation for disabling it on matrices is to guard against this
>> dreadful c++ code:
>>
>>   matrix[i,j]
>>
>> which in c++ compiles as
>>
>>   matrix[i]
>>
>> which addresses the i-th entry in the storage order of the matrix.
>>
>> If you want that functionality, there still is the synonym
>>
>>  matrix(i)
>>
>> And the method coeff(int) if you want to avoid the assertion.
>>
>> People have already proposed adding an expression to view a matrix as
>> a vector, following its storage order. But you can already do that
>> with Map. If you really want, we can add a conveniene method
>> asVector() to return such a Map object. But given the possibility of
>> doing matrix(index), i don't see too much motivation for that...
>>
>> Benoit
>>
>>
>>
>
>
>