> 2009/11/18 Gael Guennebaud <
gael.guennebaud@xxxxxxxxx>:
>>
>>
>> On Wed, Nov 18, 2009 at 1:03 PM, Jitse Niesen <
jitse@xxxxxxxxxxxxxxxxx>
>> wrote:
>>>
>>> On Tue, 17 Nov 2009, Gael Guennebaud wrote:
>>>
>>>> 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.
>>>
>>> I agree that we need an easy way to apply functions like sin
>>> coefficient-wise. However, the Array class has the disadvantage that A*B has
>>> different meaning depending on whether A and B are Matrix or Array.
>>>
>>> To me, that seems a serious issue, but perhaps I'm alone in that opinion.
>>
>> Actually I think that's one of the main goals of the class Array.
>>
>>> Nevertheless, let me discuss how to mitigate this. When the Array class is
>>> proposed, I thought that it was to support the different languages used in
>>> different domains (e.g., and rather crudely, theoretical linear algebra for
>>> Matrix, data manipulation for Array). A program in the first domain would
>>> use exclusively Matrix, and a program in the second domain would use
>>> exclusively Array. In that case, confusion is minimal, so I think that would
>>> be good programming style.
>>>
>>> However, now it's envisaged that Matrix and Array are freely mixed, and if
>>> cwise() is abolished it's even necessary to do so. So, the next best thing
>>> is to make sure that all your variables are Matrix (supposing you're in the
>>> theoretical linear algebra domain). As Gael said, this will lead to code
>>> like the following for componentwise multiplication = Hadamard product:
>>>
>>> MatrixXd A = ...
>>> MatrixXd B = ...
>>> MatrixXd prod = (A.array() * B.array()).matrix();
>>
>> Well, to be precise, the .matrix() here would be useless because Matrix
>> would have a ctor from Array and vice versa. So:
>>
>> MatrixXd prod = A.array() * B.array();
>>
>> will work. Note that because Array::operator* would expect a template
>> ArrayBase object, in spite of this Matrix from Array ctor, the following
>> still won't work:
>>
>> MatrixXd prod = A.array() * B;
>>
>> That let me think such a ctor is safe.
>>
>>
>>> Not only is that cumbersome, but I think that it's more difficult to
>>> understand than A.cwise() * B.
>>>
>>>> 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.
>>>
>>> I agree with this, cwise() is difficult, because of what you call the
>>> prefix concept: in A.cwise().max(B), cwise() modifies max, while C++
>>> programmers will think it modifies A. Perhaps a resolution would be to add a
>>> cwisemax() function to MatrixBase, so that we would replace A.cwise().max(B)
>>> to A.cwisemax(B). Now it's clear that the cwise prefix modifies max.
>>>
>>> With componentwise multiplication, that would mean that we replace
>>> A.cwise() * B by A.cwisemul(B). It's a pity that we lose the multiplication
>>> sign, but I think that we win on clarity: it's hard to misinterpret what
>>> A.cwisemul(B) would mean.
>>
>> he he, this is exactly what we had at the very beginning of Eigen2.. If I
>> remember well the major reason, not to say the unique one, that motivated us
>> to move to the cwise() prefix was to be able to overload operators.. Indeed,
>> if we list all the cwise operators we currently have together with their
>> possible respective method names, one can see that some are very hard to
>> translate (especially the +=, -=, and *= operators):
>>
>> + scalar <=> cwiseAdd(scalar)
>> - scalar <=> cwiseSub(scalar)
>> * mat <=> cwiseMul(mat)
>> < mat <=> cwiseLessThan(mat) or cwiseLT
>> <= mat <=> cwiseLessEqual(mat) or cwiseLE
>> < scalar <=> cwiseLessThan(scalar) or cwiseLT
>> <= scalar <=> cwiseLessEqual(scalar) or cwiseLE
>> // same for >, >=, ==, !=
>>
>> += scalar <=> cwiseAddInPlace(scalar)
>> -= scalar <=> cwiseSubInPlace(scalar)
>> *= mat <=> cwiseMulInPlace(scalar)
>>
>> And we could also imagine to support all kind of bitwise operations: |, &,
>> ~, ^, ! that we could name bitwiseXxx.
>>
>> Now, I also acknowledge this drawback is mitigated by the introduction of an
>> Array class, because then we can expect that the need to apply such
>> operators on matrices will be significantly reduced.
>>
>> Nevertheless, I think it is still important to be able to view matrix as
>> array, and vice versa. Two options:
>>
>> 1 - We introduce easy to use .array() and .matrix() methods, and then we'll
>> have both my proposal and your proposal, so again an infinite ways to do the
>> same thing that is not very nice IMO.
>>
>> 2 - We assume such a conversion is worth it only if you use the
>> view-as-array object multiple times, and so we enforce to use named wrapper
>> objects, e.g.:
>> MatrixXf m = ...;
>> ArrayWrapper<MatrixXf> a(m);
>> // take care that "a" and "m" reference the same data.
>> a += 2;
>>
>>
>> Finally, there is another issue that I forgot to mention in my previous
>> email and that go in favor of your proposal. All those cwise
>> operators/functions are not only useful for dense objects but also for
>> sparse matrices and certainly all other special storage objects (especially
>> abs(), abs2(), etc.). Since we want to preserve a consistent API as well as
>> to reduce code duplication, this means that with my proposal the Array class
>> should be able to potentially wrap any kind of matrices.... No need to say
>> that significantly complicate the implementation of the Array class (e.g.,
>> some will support only unary ops, some require to extend Array with other
>> overloads, etc.). The implementation of your proposal is far much simpler
>> because 1) Array would be reserved to dense storage, and 2) the cwise*
>> functions won't be hidden into wrapper classes. (sorry if these last
>> sentences does not make much sense to you, but believe me it's much simpler
>> ;) )
>
> Yes, it's much better. I am 100% for letting Array be plainly for dense storage.
>
>>
>> So after all, I'm going to like your proposal, and will wait for other
>> opinions.
>>
>> As a side note, if we go for your proposal, we can also think about
>> implementing in MatrixBase only a subset of the cwise ops available in
>> Array.
>
> Yes that's what I was going to reply: I like Jitse's proposal too but
> we really don't have to provide .cwiseEverything(), only the most
> common functions,
> matrix.cwiseProduct(matrix2)
> matrix.cwiseSum(scalar)
> matrix.cwiseAbs()
> matrix.cwiseAbs2()