New user self-registration is currently disabled. Please email eigen-core-team @ lists.tuxfamily.org if you need an account.
Bug 985 - RealQZ yields a wrong answer when there are zeros in the input matrices
RealQZ yields a wrong answer when there are zeros in the input matrices
Status: RESOLVED FIXED
Product: Eigen
Classification: Unclassified
Component: Eigenvalues
3.3 (current stable)
All All
: Normal Wrong Result
Assigned To: Nobody
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2015-03-30 17:40 UTC by Ben Goodrich
Modified: 2015-06-12 17:10 UTC (History)
5 users (show)



Attachments
test case (1.44 KB, text/x-c++src)
2015-03-30 17:40 UTC, Ben Goodrich
no flags Details
patch to fix bug (1.08 KB, patch)
2015-03-30 17:40 UTC, Ben Goodrich
no flags Details | Diff

Description Ben Goodrich 2015-03-30 17:40:00 UTC
Created attachment 557 [details]
test case

In the attached test, the case the norms are very large when the the product of the factors is subtracted from the original input matrices. In the hessenbergTriangular() method, the code tests for whether an element of the input matrix is zero. If it is zero, then the Givens rotation is not applied to S or T. However, if m_computeQZ is true, it applies the Givens rotation to Q or Z regardless of whether S or T was rotated. The solution in the patch is to move the rotation for Q or Z inside the if(coeff(i,j) != 0) block. This makes the norms zero to machine precision.
Comment 1 Ben Goodrich 2015-03-30 17:40:37 UTC
Created attachment 558 [details]
patch to fix bug
Comment 2 Christoph Hertzberg 2015-03-30 23:59:22 UTC
Thanks for the report and the patch.
Fixed in devel and 3.2 branch:
https://bitbucket.org/eigen/eigen/commits/387a190ed
https://bitbucket.org/eigen/eigen/commits/00932b53f
Comment 3 Ben Goodrich 2015-06-12 15:00:33 UTC
FYI: The changelog entry for bug 985 says "fix RealQZ when either matrix had zero rows or columns", which is not the correct description.
Comment 4 Christoph Hertzberg 2015-06-12 17:10:13 UTC
Well, that description was indeed very ambiguous. I meant to say "zero rows" as in "at least one zero row", i.e. a row containing only zeros -- having just single zeros was no problem, IIRC.
(This is also just FYI, changing the commit message now likely will mess up several things)

Note You need to log in before you can comment on or make changes to this bug.