On Wed, Oct 05, 2011 at 10:18:19AM -0500, Greg Sterijevski wrote:
> Hi Gilles,
>
> There are currently [at least] two qr decompositions in existence in cm.
> There is QRDecomposition in package linear and there is embedded in
> LevenbergMarquardtOptimizer the guts of a pivoting QR decomposition. So
> adding a PivotingQRDecomposition class in linear would initially push that
> to 3 implementations. Here are a couple of scenarios:
>
> 1. Eliminate QRDecomposition from linear. Eliminate LevenbergMarquardt's
> embed qr decompositon and have the PivotingQRDecomposition become the de
> jure standard.
>
> 2. Eliminate just QRDecomposition, not touching Marquardt-Levenberg. (Don't
> play with something that works)
>
> 3. Keep all three until the next major release, marking QRDecomposition (the
> class) as deprecated.
4. Implement the best algorithm as "QRDecomposition" in ("linear"), and
remove duplicate code ("LevenbergMarquardt" will call the implementation
in "linear"; if replacing the LM currently internal version makes some
test fail, we should worry and look for the bug either in LM or in the
new QR or in the tests).
Gilles
> -Greg
>
> On Wed, Oct 5, 2011 at 9:39 AM, Gilles Sadowski <
> gil...@harfang.homelinux.org> wrote:
>
> > On Wed, Oct 05, 2011 at 08:46:55AM -0500, Greg Sterijevski wrote:
> > > Hi Gilles,
> > >
> > > The class passes all current tests for QRDecomposition. Are you
> > suggesting I
> > > rip out the QRDecomposition from OLSMultipleRegression...etc and then
> > make
> > > sure there are no test failures? Or are you suggestion a new set of
> > tests?
> >
> > None of the above, I guess (IIUC).
> > I'm just saying that if "QR decomposition" is a well defined concept, it
> > should be implemented in the class "QRDecomposition" in package "linear",
> > and classes that need the functionality should "import" it from there.
> > I.e.
> > there should not be duplicate code (which from what I read in this thread,
> > seems to exist in "OLSMultipleRegression" and
> > "LevenbergMarquardtOptimizer").
> >
> >
> > Gilles
> >
> > >
> > > -Greg
> > >
> > > On Wed, Oct 5, 2011 at 5:16 AM, Gilles Sadowski <
> > > gil...@harfang.homelinux.org> wrote:
> > >
> > > > Hi.
> > > >
> > > > > > A while back I was interested in being able to do pivoting qr
> > > > > > decomposition. I noticed that Chris Nix submitted a patch, but he
> > > > indicated
> > > > > > that he had more work to do (testing and filling in functionality).
> > In
> > > > the
> > > > > > discussion around this, Luc suggested that I look at the QR
> > > > decomposition in
> > > > > > Levenberg-Marquardt. I did just that a few days ago. The code was
> > very
> > > > clear
> > > > > > and nicely written (kudos to Luc). So, I copied the routine and
> > made a
> > > > new
> > > > > > PivotingQRDecomposition class. The class is intended as a "drop in
> > > > > > replacement" for QRDecomposition. I also copied the QRSolverTest
> > and
> > > > > > QRDecompositionTest. With the exception of testUnderdetermined in
> > the
> > > > solver
> > > > > > test and testAEqualQR in QRDecompositionTest, the tests are
> > unchanged
> > > > (and
> > > > > > all pass!). With testUndertermined, the "zeroed" rows of the
> > solution
> > > > matrix
> > > > > > are interspersed throughout the matrix (because of pivoting). So I
> > > > change
> > > > > > the test to count all the rows that have zero norms, and check that
> > it
> > > > is
> > > > > > the correct number. In testAEqualQR, I added a multiplication by
> > the
> > > > > > permutation matrix.
> > > > > >
> > > > > > What is the best way to proceed? I don't want to trounce the
> > additions
> > > > that
> > > > > > Chris made, but it looks like Chris has more sophisticated classes
> > in
> > > > mind.
> > > > > > I don't see this proposed change competing with his. Does it make
> > sense
> > > > to
> > > > > > bring back QRDecomposition interface (sorry Sebastien)? We can then
> > > > keep
> > > > > > both implementations until we are satisfied the pivoting one works.
> > > >
> > > > -1
> > > > Please do the required testing. Only the "current best" implementation
> > > > should remain.
> > > >
> > > >
> > > > Regards,
> > > > Gilles
> > > >
> > > > ---------------------------------------------------------------------
> > > > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> > > > For additional commands, e-mail: dev-h...@commons.apache.org
> > > >
> > > >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> > For additional commands, e-mail: dev-h...@commons.apache.org
> >
> >
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
