Refactor xGEBAL #808
Refactor xGEBAL #808
Conversation
|
As of latest (force) push all tests succeed fully on my machine. |
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@ ## master #808 +/- ## ======================================== Coverage 0.00% 0.00% ======================================== Files 1908 1908 Lines 187072 186968 -104 ======================================== + Misses 187072 186968 -104
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
| I seem to have missed a part of the code when translating my changes from the real to the complex case. Fixed as of now, sorry about missing that before opening the PR. |
There was a problem hiding this comment.
The original code is quite hard to read, so it's also hard to verify that this code does the same. That being said, I trust the tests for standard eigenvalue problems to be thorough enough. The scaling routine makes much more sense now. Good job @eprovst
3 participants
Description
This PR should improve the readability of xGEBAL, without changing it functionally. In my testing, this does give identical results to the original, however a second pair of eyes is needed to verify. Execution times also seem to be identical, if not ever so slightly better.
The first commit contains the refactor. The second is a slight change where the NaN check is taken outside the loop. As far as I know, multiplying and dividing by 2 should not introduce NaNs, else the latter change is of course incorrect.
I also noticed that although the 2-norm is used, the stopping criterion seems to be for the 1-norm (cf. James, R., Langou, J., & Lowery, B. R. (2014). On matrix balancing and eigenvector computation. arXiv:1401.5766.)? I'll leave that one to the experts.
(As some background: balancing seemed relevant to an issue I was facing, so I wanted to be able to tinker with DGEBAL, in the end it was irrelevant to my work, but maybe this refactor could be useful to someone else later.)
Checklist