|Re: [eigen] inconsistent cwise() support|
[ Thread Index |
| More lists.tuxfamily.org/eigen Archives
- To: eigen@xxxxxxxxxxxxxxxxxxx
- Subject: Re: [eigen] inconsistent cwise() support
- From: Benoit Jacob <jacob.benoit.1@xxxxxxxxx>
- Date: Tue, 17 Nov 2009 08:47:37 -0500
- Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:in-reply-to:references :date:message-id:subject:from:to:content-type :content-transfer-encoding; bh=yeXBswsW5w4cMsErG20OFEOTfHXlakSbzQ8nlZbi2KY=; b=moSEjl+wZoqC+WAjaxCoWMXTqxidR5EjW2qevy0ldmp9jjxtY77+/xNo7cD3M+u1J0 VVQg7gBsIk4DGexlnstwTo2U+IzcQ/U8SMiILjQUliTyh389BMBbaDkYtsljguQxYjmT HfXZvUtV1KvgstDQmKOxIhTipbALvylBbBX5Q=
- Domainkey-signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :content-type:content-transfer-encoding; b=hZMD/BgkZUggAMZQZVKOJqytC1i6GHi+KnWy3EsvnhckPdxViOLn84N51VatOFtZkt JMjZ+Q3ZL2oYgx3lWN3HawNDjfYSMcvRDL8bEFY4RbaXOdvPsHzjFRypX3ugsKeOLPhJ TGrfzXiDkdENqgVzw0uDSBZc4Pt1T2KchLZiU=
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
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.
> 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;
>> that is redundant with the current:
>> 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:
>> 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;
>> 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
>> 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
>> (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,
>> 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.
>> 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>
>>> 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.
>>> > 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