|Re: [eigen] Propagating Max*AtCompileTime template parameters to decompositions|
[ Thread Index |
| More lists.tuxfamily.org/eigen Archives
- To: eigen@xxxxxxxxxxxxxxxxxxx
- Subject: Re: [eigen] Propagating Max*AtCompileTime template parameters to decompositions
- From: Benoit Jacob <jacob.benoit.1@xxxxxxxxx>
- Date: Thu, 4 Mar 2010 11:57:31 -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=t6Ubt66zCsJXNYfSZnAdFYFmnFkuYTE4TRKMrpeej9Y=; b=rSKsDTwPw7cdSx5+aPubWJkQexEeivMPgiZzLpkToOaFJ8iMK7CKFH24W1rlijJ9BH zd6rHJdyeACVoEbMW2dnGYnnx4Mn/wDjwwoOSbFjsxqx9fycFrVbZ3XkOEs5rAk79J6E oIuQzL+JZCuhYhFVDGUAvoSXCXTnwDzm+qF0w=
- 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=RkwMLkuMDG+/DDDBtRKq0hru8ApQTQv0WIAvCM1bb8+WR/JQ2Wtm/6EoZMinqYghVh 1048kolq0yuwo2zBpkgOby4Emkw3MSpKIPfHJK5rk+5YHO5rrfH6h2DCHo7X5VquBiSp wFEYyozaPfR32ileJSz6rHPZ/IgqsJSV79t2Y=
2010/3/4 Adolfo Rodríguez Tsouroukdissian <dofo79@xxxxxxxxx>:
> On Thu, Mar 4, 2010 at 5:25 PM, Benoit Jacob <jacob.benoit.1@xxxxxxxxx>
>> Hi Adolfo,
>> Many thanks for this patch. It's been sitting in the back of my mind
>> nagging me for a while :) These fixes are definitely wanted.
>> Just 2 things:
>> 1. All the MatrixType:: is a bit heavy so could you please introduce
>> enums like is being done in ColPivHouseholderQR.h, in other
>> decompositions? So MatrixType::MaxBlabla becomes just MaxBlaBla.
> OK, no problem. The enum approach is definitely much cleaner. I just didn't
> want to impose it if it was not used in some files :)
> One more thing, now that I'm on it, are there any other files that I might
> have overlooked that could benefit from similar changes?
Any decomposition is affected in the same way. Have you had a look at
the Eigenvalues directory?
>> 2. You generated this patch with "diff" so it doesn't credit you, and
>> you haven't written your commit msg. Could you please follow the
>> instructions given there:
>> Yes, this mailing list is the right place for such patches... the
>> issue tracker is useful to report bugs without being immediately able
>> to fix them.
>> 2010/3/4 Adolfo Rodríguez Tsouroukdissian <dofo79@xxxxxxxxx>:
>> > Hi all,
>> > I've been performing some tests with dynamic matrices that have a
>> > compile-time max size, and found out that for most matrix
>> > decompositions,
>> > the MaxRowsAtCompileTime and MaxColsAtCompileTime template parameters of
>> > the
>> > input matrix are not propagated to the decomposition's members and
>> > temporaries. This leads to undesired and preventable heap allocations.
>> > The
>> > following is a minimal example of what I mean:
>> > typedef Matrix<double, dynamic, dynamic, 0, 10, 10> Mat;
>> > Mat A(Mat::Random(5, 5));
>> > FullPivLU<Mat> ldlt(A); // Allocates 4 temps
>> > LDLT<Mat> ldlt(A); // Allocates 3 temps
>> > I'm attaching a patch against the devel branch (commit
>> > 2589:8a8f4e9e04bf)
>> > that deals with this issue for the Cholesky, LU, QR and SVD modules.
>> > Since
>> > most of these modules are experimental and undergoing heavy work, the
>> > style
>> > of the modifications changes form file to file (some use convenience
>> > enums,
>> > some not). However, I tried to stay consistent within the context of
>> > each
>> > file. Please let me know your thoughts on the matter.
>> Yes so actually, please introduce enums everywhere :)
>> > One very specific
>> > note: in line 65 of HouseHolderQR.h I left the Options template
>> > parameter
>> > as-is, but don't know if it should be changed to that of MatrixType.
>> Great catch! Please change that to Options (as defined on line 57).
>> > Cheers,
>> > Adolfo.
>> > P.S. Is there a better way to submit small patches, something like a
>> > bugtracker, or is this the currently recommended way?
>> > --
>> > Adolfo Rodríguez Tsouroukdissian, Ph. D.
>> > Robotics engineer
>> > PAL ROBOTICS S.L
>> > http://www.pal-robotics.com
>> > Tel. +34.93.414.53.47
>> > Fax.+184.108.40.206.09
>> > AVISO DE CONFIDENCIALIDAD: Este mensaje y sus documentos adjuntos,
>> > pueden
>> > contener información privilegiada y/o confidencial que está dirigida
>> > exclusivamente a su destinatario. Si usted recibe este mensaje y no es
>> > el
>> > destinatario indicado, o el empleado encargado de su entrega a dicha
>> > persona, por favor, notifíquelo inmediatamente y remita el mensaje
>> > original
>> > a la dirección de correo electrónico indicada. Cualquier copia, uso o
>> > distribución no autorizados de esta comunicación queda estrictamente
>> > prohibida.
>> > CONFIDENTIALITY NOTICE: This e-mail and the accompanying document(s) may
>> > contain confidential information which is privileged and intended only
>> > for
>> > the individual or entity to whom they are addressed. If you are not the
>> > intended recipient, you are hereby notified that any disclosure,
>> > copying,
>> > distribution or use of this e-mail and/or accompanying document(s) is
>> > strictly prohibited. If you have received this e-mail in error, please
>> > immediately notify the sender at the above e-mail address.