Re: [eigen] Diagonal matrices diff

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


Out of curiosity i did it:
http://codereview.appspot.com/62071/show
It's a great tool indeed when reviewing a diff is needed. I'd like to
encourage people sending us diffs to use it.

But frankly in this particular case the question is more whether Gael
agrees with the principle, he doesn't really need to review carefully
my diff...

Benoit

2009/5/9, Keir Mierle <mierle@xxxxxxxxx>:
> A suggestion: Instead of mailing a diff, use Rietveld to post
> something for review. It's much easier to review that way instead of
> the raw diff; you don't need to have eigen checked out to see the
> context.
>
> http://codereview.appspot.com/
>
> If you have a gmail account, you can already use it. It's very easy to
> post new reviews.
>
> Keir
>
> On Fri, May 8, 2009 at 9:17 PM, Benoit Jacob <jacob.benoit.1@xxxxxxxxx>
> wrote:
>> Hi,
>>
>> before i can do the svd i need Banded matrices, for which i needed
>> some improvements in DiagonalCoeffs.
>> So here's a patch but as it is changing several things i thought i'd
>> ask for your opinion (you know who you are) before committing.
>>
>> * rename DiagonalCoeffs (clumsy name) to Band. I tried first Diagonal,
>> but that name was already taken by a constant Diagonal which is used
>> for various things already.
>> * rename the template parameter DiagId to Index.
>> * keep diagonal() unchanged, returns a Band<Derived, 0>
>> * rename diagonal<int>() to band<int>()
>> * introduce diagonal(int), returning a Band<Derived, Dynamic>. The
>> trick here is as we already do in Block : use ei_int_if_dynamic to
>> store the Index in Band.
>>   ---> that is the improvement that i need in order to code
>> BandedMatrix. No need to unroll an outer loop there, and that also
>> will reduce compilation times (only one Band type instantiated).
>>
>> Cheers,
>> Benoit
>>
>
>
>



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