Re: [eigen] Propagating Max*AtCompileTime template parameters to decompositions

[Benoit Jacob <jacob.benoit.1@xxxxxxxxx>]

2010/3/4 Adolfo Rodríguez Tsouroukdissian <dofo79@xxxxxxxxx>:
>
>
> On Thu, Mar 4, 2010 at 5:25 PM, Benoit Jacob <jacob.benoit.1@xxxxxxxxx>
> wrote:
>>
>> 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:
>>
>> http://eigen.tuxfamily.org/index.php?title=Developer%27s_Corner#Generating_a_patch
>>
>
> Ack.
>
> Adolfo
>
>>
>> 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.
>>
>> Benoit
>>
>>
>> 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).
>>
>> Thanks
>> Benoit
>>
>> >
>> > 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.+34.93.209.11.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.
>> >
>>
>>
>
>