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
