Re: [eigen] inconsistent cwise() support

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

ok, nevermind, your solution is better: just force more code to #include Array.

(now replying to your other e-mail).

Benoit

2009/11/17 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
>>>> >
>>>>
>>>>
>>>
>>
>>
>>
>>
>