Re: [eigen] inconsistent cwise() support

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

or you could just move the Array class to Core. This isn't quite like
merging the whole Array module into Core, e.g. the partial reductions
stay out; this is more about redefining what the Array module is. At
the moment it is a bit unfortunate that the word Array is used for 2
different notions, the current Array module and the future Array
class.

Benoit

2009/11/17 Gael Guennebaud <gael.guennebaud@xxxxxxxxx>:
>
> There is one little difficulty: currently a few methods of Cwise are defined
> in Core (abs, min/max) because they appeared to be really fundamental. With
> my proposal I don't see how this could be done because the class Array
> should really be declared in the Array module ! That means the Array module
> will have to be included more often. Again, there is a good counter part:
> this splitting of Cwise across two modules is really prone to annoying
> compilation errors which are really hard to understand for the users.
>
> gael.
>
> On Tue, Nov 17, 2009 at 12:48 PM, Gael Guennebaud
> <gael.guennebaud@xxxxxxxxx> wrote:
>>
>> I also agree with that proposal, but I'm thinking that once we'll have a
>> true array support, all this array/cwise stuff is going to be messy. Here
>> I'll assume that a true Array class supporting the standard global math
>> function (e.g., std::sin) is really needed. If that's not the case, then we
>> don't implement it => problem solved. So once we'll have this Array class, I
>> guess we'll also want an easy way to view a Matrix as an Array, so one will
>> be able to write:
>>
>> MatrixXf m;
>> abs(m.array());
>>
>> that is redundant with the current:
>>
>> m.cwise().abs();
>>
>> Since we are currently in favor of member functions rather than global
>> functions, we might also want to add, e.g.,  an abs() method to Array
>> yielding a third option for the user:
>>
>> m.array().abs();
>>
>> For consistency reason, perhaps we would also support the cwise() API for
>> Array. After all it would be weird that this cwise() API works for Matrix
>> only. So now we would also have three options for Array objects:
>>
>> ArrayXf a;
>> abs(a);
>> a.abs();
>> a.cwise().abs();
>>
>> So the addition of such an Array class is perhaps the occasion to
>> reconsider coefficient wise operations. In order to make a decision, it is
>> also interesting to have a look at some binary operations:
>>
>> MatrixXf m1, m2;
>> ArrayXf a1, a2;
>>
>> m1.cwise() + m2;    // current API
>> m1.array() + m2.array(); // more verbose, but symmetric
>>
>> a1 + a2;
>> a1.cwise() + a2;   // very stupid
>>
>> m1.cwise().max(m2); // current API
>> max(m1.array(), m2.array()); // more verbose, but symmetric
>> m1.array().max(m2.array());  // more verbose, not really symmetric
>>
>> max(a1,a2);
>> a1.max(a2);  // not really symmetric
>> a1.cwise().max(a2);   // very stupid
>>
>> I think it is also important to acknowledge that the current cwise() API
>> is odd enough to make it hard to learn for new comers. They often have some
>> difficulties to determine where .cwise() is needed (e.g., m1.cwise() *
>> m2.cwise()), and how it propagates (though it does not, of course). For
>> instance, if you look at "m1.cwise() * m2" and you know, or did not
>> understand the prefix concept of .cwise(), then it seems that .cwise
>> propagate to m2.
>>
>> Ok I think you got me now. When we designed this cwise() API, I liked it a
>> lot because it was fun: it allows us to overload a couple of operators
>> (*,+,-,<,>,!=, etc.) but now, it looks more odd than fun, and this is
>> something that annoys me in Eigen.
>>
>> Said that, I still don't have an ideal solution to propose, but what about
>> removing the .cwise() prefix in favor of only the .array() style that is
>> illustrated above ? To help you make a decision here are the four main
>> consequences of such a change with their own pro and cons:
>>
>> 1 - It makes the API simpler once Array is implemented (+1)
>>
>> 2 - For binary operations you need to put Array on both sides, so it is,
>> at a first glance, a bit more verbose (-0.5). On the other hand it is
>> perhaps less cumbersome (+0.5).
>>
>> 3 - When you do: (m1.array() * m2.array()) you get an Array expression,
>> not a Matrix. So you might need to convert it back explicitly to a Matrix
>> (-0.5):
>>
>> (m1.array() * m2.array()).matrix() * m3;
>>
>> On the other hand this avoid the need to repeat .cwise() all the time when
>> all your operations are coefficient wise, e.g.:
>>
>> (m1.array() * m2.array()).abs().pow(alpha);
>>
>> // or:
>>
>> pow(abs(m1.array() * m2.array()), alpha);
>>
>> Since it seems to me that you very rarely have to mix pure Array
>> operations with pure Matrix ones in a single expression, this seems to be a
>> nice advantage (+1).
>>
>> 4 - Of course the last consequence is a significant API break, and here I
>> only see a strong drawback, I'm thinking hard, but I cannot find any
>> advantage for this consequence (-1).
>>
>> For the rest it is pretty much the same than for the current cwise()
>> solution (operator overloading, for unary ops it is just a text replacement,
>> etc.).
>>
>> At the end the sum of the scores is positive, +0.5, so that's a proof this
>> is a good proposal ;) Seriously, it seems to me that this proposal has some
>> real benefits for practical uses, but of course the major blocking point is
>> the API break.
>>
>>
>>
>> Finally, we can also discuss the choice to have both global functions and
>> member methods for Array. Here global functions are really useful, that's
>> why I'm in favor to provide both.
>>
>>
>> Cheers,
>>
>> Gael, who likes to put on the table tough API discussions, over and over
>> again ;)
>>
>>
>> On Mon, Nov 16, 2009 at 9:40 PM, Benoit Jacob <jacob.benoit.1@xxxxxxxxx>
>> wrote:
>>>
>>> 2009/11/16 Hauke Heibel <hauke.heibel@xxxxxxxxxxxxxx>:
>>> > On Mon, Nov 16, 2009 at 7:22 PM, Benoit Jacob
>>> > <jacob.benoit.1@xxxxxxxxx>
>>> > wrote:
>>> >>
>>> >> It seems like what you want is Gael's proposal for "true array
>>> >> support".
>>> >
>>> > Not really, since I am really required to work on matrices - actually I
>>> > was
>>> > working on spectral clustering involving eigenvalue decompositions (so
>>> > real
>>> > linear algebra stuff).
>>> >
>>> >>
>>> >> vector - scalar is not a standard operation; it doesn't have a
>>> >> geometric meaning. Actually matrix - scalar is sometimes used as
>>> >> matrix - scalar*Identity, so that's another reason not to let it mean
>>> >> coeff-wise substraction.
>>> >
>>> > I totally agree, I was just thinking about consistingly supporting (I
>>> > do
>>> > even volunteer to implement it)
>>> >
>>> > .cwise() += | -= | /= | *=
>>>
>>> oh, i see.
>>>
>>> Yes, I agree with your proposal, it would be cool if you implement it.
>>> It's true that there's no reason not to let .cwise syntax always work.
>>>
>>> Benoit
>>>
>>>
>>> >
>>> > and not in general
>>> >
>>> > MatrixBase::operator += etc.
>>> >
>>> > I just think that some problems (formulated in an algebraic framework)
>>> > do
>>> > sometimes need individual rescaling and sometimes even shifting when
>>> > you
>>> > need to work e.g. on zero-mean input data.
>>> >
>>> > - Hauke
>>> >
>>>
>>>
>>
>
>
>
>