lapack
lapack copied to clipboard
Fixed usage of uninitialized variables in TESTING
Description
Thanks to @dklyuchinskiy, we were able to fix #956, which was caused by an initialized RESULT vector in the testing routines for the truncated QR routines.
Also, we changed the index calculation in one of the tests to use LDA instead of M. This has no impact on the test itself because for all variations that this test uses, it is always guaranteed that LDA == M, but it is a little bit more consistent.
Closes #956
Checklist
- [n/a] The documentation has been updated.
- [X] If the PR solves a specific issue, it is set to be closed on merge.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 0.00%. Comparing base (
db501d9) to head (f6355dc). Report is 135 commits behind head on master.
:exclamation: Current head f6355dc differs from pull request most recent head 7113caa
Please upload reports for the commit 7113caa to get more accurate results.
Additional details and impacted files
@@ Coverage Diff @@
## master #961 +/- ##
=======================================
Coverage 0.00% 0.00%
=======================================
Files 1930 1930
Lines 190421 190421
=======================================
Misses 190421 190421
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Btw. I'm wondering why the codecov.io report always shows 0%. If I create a report locally with gcovr it shows me 82%. Just in case anyone has an idea how to fix that. The pipeline shows nothing out of the ordinary.
@ACSimon33 thanks for the MR! LGTM!
I have one question: should we zero out RESULT on the bottom level of all cycles? I mean, that there is cycle
595 598 DO KMAX = 0, MIN(M,N)+1
and if test is failed for one KMAX, it will fail for all next ones at the moment?
@ACSimon33 thanks for the MR! LGTM!
I have one question: should we zero out
RESULTon the bottom level of all cycles? I mean, that there is cycle595 598 DO KMAX = 0, MIN(M,N)+1and if test is failed for one
KMAX, it will fail for all next ones at the moment?
@dklyuchinskiy Yes, you're right. I moved the initialization into the inner loop. I also refactored some of the indentation and put the error reporting at the end of the loop.
That seems not the only issue with uninitialized values in geqp3rk.
Regarding https://github.com/Reference-LAPACK/lapack/blob/d7ea9c52b9852cb39632aac36644602ee645de12/SRC/zgeqp3rk.f#L760-L766 the variable MAXC2NRM is used before set the first time.
This also affects cgeqp3rk at https://github.com/Reference-LAPACK/lapack/blob/d7ea9c52b9852cb39632aac36644602ee645de12/SRC/cgeqp3rk.f#L760-L767
But in its real counterparts we have
https://github.com/Reference-LAPACK/lapack/blob/d7ea9c52b9852cb39632aac36644602ee645de12/SRC/dgeqp3rk.f#L750-L760
and
https://github.com/Reference-LAPACK/lapack/blob/d7ea9c52b9852cb39632aac36644602ee645de12/SRC/sgeqp3rk.f#L750-L760
Hi @ACSimon33!
Let's finish this PR? I have found one typo inside TESTING/LIN/dchkqp3rk.f, but I cant push commits into your PR. Could you please fix it? Other changes looks great!
Problem, found by @grisuthedragon maybe is out of scope of this PR, but of course it is also important.
@dklyuchinskiy Fixed the typo. I think this is ready to be merged
@dklyuchinskiy Fixed the typo. I think this is ready to be merged
Thanks a lot! Agree.
@langou please review