lapack icon indicating copy to clipboard operation
lapack copied to clipboard

Add numerical tests for trevc3

Open angsch opened this issue 3 years ago • 2 comments

Description

Add some numerical tests for the so far completely untested trevc3

angsch avatar Jun 20 '22 16:06 angsch

If i'm not mistaken, trevc3 is tested because it is a part of geev, and geev is thoroughly tested, so I wouldn't say that trevc3 is completely untested (correct me if i'm wrong). However, separate tests for subroutines are always welcome.

Is this perhaps an indication that you will use the skills you used to write #651 to improve trevc3?

thijssteel avatar Jun 22 '22 11:06 thijssteel

@thijssteel Could you be so kind and have a look? Your suggestion to write the application of the Householder reflectors in a consistent way should be complete now; and, while I was already on it, the change is applied to the generalized problem as well. As far as I see it, that's all occurrences.

trevc3 is tested because it is a part of geev, and geev is thoroughly tested

The tests cover at least one more case that is not covered by geev: The backtransform is done in the tests, no via the eigenvector computation routine. There are a few more improvements that can be done in trevc3, so more tests are indeed good. :-)

angsch avatar Jul 06 '22 18:07 angsch

Just a late and general comment from me - wouldn't it make sense to update the PR title when a PR evolves to include a rewrite of the underlying algorithm in addition to the new tests ?

martin-frbg avatar Mar 20 '23 08:03 martin-frbg

It's actually a rewrite of a different algorithm. And yes, an even better update would be to split the PR.

thijssteel avatar Mar 20 '23 08:03 thijssteel