Re: [eigen] inconsistent cwise() support |

[ Thread Index | Date Index | More lists.tuxfamily.org/eigen Archives ]

*To*: eigen@xxxxxxxxxxxxxxxxxxx*Subject*: Re: [eigen] inconsistent cwise() support*From*: Benoit Jacob <jacob.benoit.1@xxxxxxxxx>*Date*: Wed, 18 Nov 2009 09:17:24 -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=QIgkBa/NVZPilhb5iRnrMk8rVvPb11vEtRth1XCGOYI=; b=Prf1PENux6SV8raWEKiKBc7nptOpZRdDwdPnpeyZY+Ttw93xFH/wQfyU7CatbHTX1L oQpVtcm5j3m/yZu29bBYkUmBfGDgWp0XIYiUd0n9DxFs28ymILfGsb2XU1aFVJt87IdU ah1hrKnHtlZdanVGa74iWffpCbzbbxDHWEfco=*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=R2IyH5bAZD8RBVZ++AQLYjavq3MIvINE08vR/WyIvm/ZBltFSPphnwaGmCFcekwuUl xCyjsVP/yrxByWoTzhyGHalNERTr8MgvxDo/OlgCJ+R4+hzYuy053WusBqB2+dytLDUo +/YWvs7ZX7PeusdBa9J06QTID+ahkvn2mQmAo=

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() Just that will be enough to cover 95% of cases and I don't even think that we need the += and *= operators (but it's of course a possibility to add a few more functions if you think it's needed). Notice how i prefer to name functions according to what they are returning, when they leave their arguments constant: Product instead of Add. On the other hand I don't think that we need such functions matrixOperation() in the Array class. Doing matrix operations on an Array is far, far less common, so I think that just having .matrix() will be good enough. Finally, I am not in favor of generalizing the use of global functions everywhere. I understand that it makes sense to have global std::sin and ei_sin for Array, to make that usable as any scalar type. And I understand that for some functions, it is pleasant to have a global version, such as dot(). But I don't think that it should become the new rule that every function has a global version. (Referring e.g. to above cwisemul(A,B)). Benoit > > Gael. > > >> >> With the addition of Array, that leads to the following API. The idea is >> that a module would use either the left or the right column, but not mix >> expressions. >> >> Matrix A,B Array A,B >> matrix multiply: A * B A.matrixmul(B) >> c-wise multiply: A.cwisemul(B) A * B >> matrix exponential: A.exp() A.matrixexp() >> c-wise exponential: A.cwiseexp() A.exp() >> >> The discussion on whether to introduce global functions instead of (or in >> parallel of) member functions seems to be orthogonal to this, but just for >> illustration, this is how it would work with global functions. >> >> Matrix A,B Array A,B >> matrix multiply: A * B matrixmul(A,B) >> c-wise multiply: cwisemul(A,B) A * B >> matrix exponential: exp(A) matrixexp(A) >> c-wise exponential: cwiseexp(A) exp(A) >> >> Cheers, >> Jitse >> >> > >

**Follow-Ups**:**Re: [eigen] inconsistent cwise() support***From:*Benoit Jacob

**References**:**[eigen] inconsistent cwise() support***From:*Hauke Heibel

**Re: [eigen] inconsistent cwise() support***From:*Benoit Jacob

**Re: [eigen] inconsistent cwise() support***From:*Hauke Heibel

**Re: [eigen] inconsistent cwise() support***From:*Benoit Jacob

**Re: [eigen] inconsistent cwise() support***From:*Gael Guennebaud

**Re: [eigen] inconsistent cwise() support***From:*Jitse Niesen

**Re: [eigen] inconsistent cwise() support***From:*Gael Guennebaud

**Messages sorted by:**[ date | thread ]- Prev by Date:
**Re: [eigen] inconsistent cwise() support** - Next by Date:
**Re: [eigen] inconsistent cwise() support** - Previous by thread:
**Re: [eigen] inconsistent cwise() support** - Next by thread:
**Re: [eigen] inconsistent cwise() support**

Mail converted by MHonArc 2.6.19+ | http://listengine.tuxfamily.org/ |