Re: [eigen] [patch] LDLt decomposition with rank-deficient matrices

[Manoj Rajagopalan <rmanoj@xxxxxxxxx>]

On Monday 28 June 2010 02:00:50 pm Ben Goodrich wrote:
> Hi Benoit,
>
> On Mon, Jun 28, 2010 at 8:51 AM, Benoit Jacob <jacob.benoit.1@xxxxxxxxx> 
wrote:
> > Hi Ben,
> >
> > Let's get this sorted :) So far I didn't participate much in the
> > discussion so I may be missing a lot of things.
> >
> > When it was question of making pivoting/nopivoting an option of the
> > LDLT class, I thought we were talking about a template parameter. If
> > fact, in your patch, it is a runtime option. In all other
> > decompositions in Eigen, we have made this be separate classes, e.g.
> > HouseholderQR vs ColPivHouseholderQR. Frankly I would like LDLT to do
> > the same. Keep LDLT pivoting, and add a NoPivLDLT class. This will
> > actually make your NoPivLDLT be more optimized (the biggest advantage
> > will be smaller executable code). The only advantage of having that as
> > runtime param is to share code if a user is using both flavors of
> > LDLT. I believe that the most common case is that a program will use
> > at most one flavor of LDLT.
> >
> > If you send a patch with this change, I promise I'll review it :)
> >
> > Benoit
>
> That is all fine with me; I don't have any strong reason to prefer a
> runtime option. But I interpreted Gael's email on June 16 as asking
> for a runtime option and then later he asked about the possibly of a
> separate class. So, it seems we now have a consensus on the separate
> class at least. :)
>
> Ben
>


Would it make sense to add an extra boolean template parameter to the LDLT 
template that defaults to 'true' with 'false' being the no-pivot case? Since 
partial specialization is allowed with class templates this should cause no 
problems with existing code that uses LDLT, AFAIK ...

-- Manoj