Re: [eigen] Transform products |

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

*To*: eigen@xxxxxxxxxxxxxxxxxxx*Subject*: Re: [eigen] Transform products*From*: Benoit Jacob <jacob.benoit.1@xxxxxxxxx>*Date*: Thu, 19 Aug 2010 08:47:44 -0400*Dkim-signature*: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:received:in-reply-to :references:date:message-id:subject:from:to:content-type; bh=hvYHvea/k1c1JD8RRM4QEbOsAlbe/ddVnx98IHESISs=; b=Sw+K3kAcV7aumioUQ7934+9mmYk2L/PwfkI/j3Eszy0g0ptCjOrenoYptETwodIpKb YkPA1+q9pFqs7kYqxn9bMyLcHqVmpW5b8yeELIoQoTWFqnf3eE2Hf9S9kiSsdWJaDL/Y WsGXiMZLmUF2RCjNspu6nNI77Non68Xv8SRlI=*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; b=fESIdRYBbftVsOAZQ8D1O9TnvD0yzRER1aeZzGeySVbO7ZxOFspgDisaNcrGS9fzob CNSOTbqwKeGFxvQO1CnyiFwKFkHhgXjPCmUwHqCALSOLQUf4+GtLIdQ6ZlheBTRu997l Z5i7OEmGfLgCqr2UIp95d/f17feuBPn/fk6SA=

2010/8/19 Gael Guennebaud <gael.guennebaud@xxxxxxxxx>

Also, the old (current) code in Transform is taking any EigenBase, i.e. either a dense matrix or another kind of special matrix, in particular a DiagonalMatrix.

That was nice when products were interpreted in the old way (interpreting the EigenBase as a Transform) but frankly not necessary, especially not with the new interpretation.

So I think that you can just change that produce code to only take MatrixBase objects. So we drop support for special matrix classes. In the new interpretation of Transform products that we're coming to, it's not useful anyway. Then you don't have any conflict anymore with DiagonalMatrix, so you can add a separate operator* taking (Transform, DiagonalMatrix).

Benoit

Hi,

first of al let me say l I agree with the changes proposed in the other thread.

However, DiagonalMatrix is used to represent non uniform scaling. For

instance Scaling(1,2) returns a 2x2 DiagonalMatrix. So this time I

think that

T * DiagonalMatrix

should really be interpreted as T * scaling, whence the following rules:

Affine = Isometry * diagonal

Affine = Affine * diagonal

Projective = Projective * diagonal

Who want to represent points using a diagonal matrix....

Otherwise we would have to add an explicit NonUniformScaling class,

but this is exactly what a DiagonalMatrix is....

Also, the old (current) code in Transform is taking any EigenBase, i.e. either a dense matrix or another kind of special matrix, in particular a DiagonalMatrix.

That was nice when products were interpreted in the old way (interpreting the EigenBase as a Transform) but frankly not necessary, especially not with the new interpretation.

So I think that you can just change that produce code to only take MatrixBase objects. So we drop support for special matrix classes. In the new interpretation of Transform products that we're coming to, it's not useful anyway. Then you don't have any conflict anymore with DiagonalMatrix, so you can add a separate operator* taking (Transform, DiagonalMatrix).

Benoit

gael

On Thu, Aug 19, 2010 at 1:00 PM, Hauke Heibel

<hauke.heibel@xxxxxxxxxxxxxx> wrote:

> Ok, I am starting a new thread.. The other one was way too long.

>

> I started with the RHS product refactoring. Everything went well,

> until I hit the case, where the RHS is a DiagonalMatrix. The patch is

> attached but I will anyways past the most important piece of code over

> here because it is hard to find in the diff.

>

> The function causing troubles is ei_transform_right_product_impl::run

> for Affine and Isometry. I tried to make a one fits all implementation

> and in theory, it works well. Even the inlining on MSVC seems to be

> working for fixed size types. For DiagonalMatrix, I am hitting the

> issue, that there is now way to extract a block.

>

> Here is how I tried to implement run:

>

> EIGEN_STRONG_INLINE static ResultType run(const

> Transform<Scalar,Dim,Mode>& T, const Other& other)

> {

> EIGEN_STATIC_ASSERT(OtherRows==Dim || OtherRows==HDim,

> YOU_MIXED_MATRICES_OF_DIFFERENT_SIZES);

>

> typedef Block<ResultType, Dim, OtherCols> TopLeftLhs;

> typedef Block<Other, Dim, OtherCols> TopLeftRhs;

>

> ResultType res(other.rows(),other.cols());

>

> TopLeftLhs(res, 0, 0, Dim, other.cols()) =

> ( T.linear() * TopLeftRhs(other, 0, 0, Dim, other.cols()) )..colwise() +

> T.translation();

>

> // This is a hack to circumvent the fact that DiagonalMatrix does

> not expose ::row(int)

> // In fact, the code below will not even be used in release mode,

> but it will still be compiled.

> if (OtherRows==HDim)

> Block<ResultType, 1, OtherCols>(res,0,0,1,other.cols()) =

> Block<Other, 1, OtherCols>(other.derived(),0,0,1,other.cols());

>

> return res;

> }

>

> I used Block in the end, because row() does not exist for

> DiagonalMatrix. Any Ideas? I would be too sad, if I were required to

> write a specialization just because of this since in general, the if

> clause is already removed by the compiler -- though it is still

> compiled.

>

> On the other hand side, who want to do something like this because now

> everything on the RHS is interpreted as points. If the RHS is 3x3 it

> is not any more interpreted as [S 0; 0 1]. The correct move would be

>

> A.linear() *= S;

>

> and even this works

>

> A * (S * pts_33)

>

> though this does not

>

> (A * S) * pts_33

>

> I could even go back to row() instead of block().

>

> I am pausing the patching for now, until I get feedback.

>

> - Hauke

>

**Follow-Ups**:**Re: [eigen] Transform products***From:*Hauke Heibel

**References**:**[eigen] Transform products***From:*Hauke Heibel

**Re: [eigen] Transform products***From:*Gael Guennebaud

**Messages sorted by:**[ date | thread ]- Prev by Date:
**Re: [eigen] problem with selfadjointView** - Next by Date:
**Re: [eigen] gcc 4.2.4 warning on type-punned pointers** - Previous by thread:
**Re: [eigen] Transform products** - Next by thread:
**Re: [eigen] Transform products**

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