lapack icon indicating copy to clipboard operation
lapack copied to clipboard

LAPACKE_[cz]tgexc doesn't pass ifst and ilst as outputs

Open mgates3 opened this issue 3 years ago • 2 comments

Description Per LAPACK docs, in [sd]tgexc, both ifst and ilst are [in,out]: while in [cz]tgexc, ifst is [in] and ilst is [in,out]: https://github.com/Reference-LAPACK/lapack/blob/master/SRC/stgexc.f#L142-L147 https://github.com/Reference-LAPACK/lapack/blob/master/SRC/ctgexc.f#L133-L138

In LAPACKE_[sd]tgexc, both ifst and ilst are passed as pointers to be outputs, while in LAPACKE_[cz]tgexc, both ifst and ilst are erroneously passed as scalars to be inputs only: https://github.com/Reference-LAPACK/lapack/blob/master/LAPACKE/src/lapacke_stgexc.c#L39 https://github.com/Reference-LAPACK/lapack/blob/master/LAPACKE/src/lapacke_ctgexc.c#L41

Minimally, LAPACKE_[cz]tgexc should be passed ilst as pointer to be an output. For consistency across all 4 precisions, I would also pass ifst as pointer — it won't get changed, so no harm done if passed as pointer. (This is what LAPACK++ will do.) Inconsistencies across the 4 precisions make templates or auto code generation more cumbersome.

I don't know how important having ilst as output is. Do users depend on it? The inconsistency showed up when implementing the LAPACK++ wrapper, and LAPACK++ output differed from LAPACKE. I can submit a PR to fix it, but how do we feel about changing the API to LAPACKE_[cz]tgexc, which would break compiling user codes? Probably not a commonly used routine, though.

In LAPACK++, currently I detect the LAPACKE issue but otherwise ignore it (tgexc not yet pushed):

lapackpp/test> ./tester --type s,d,c,z --dim 100 tgexc
LAPACK++ version 2022.07.00, id c878e31
input: ./tester --type 's,d,c,z' --dim 100 tgexc
                                                                       
type  jobvl  jobvr       n     error   time (s)  ref time (s)  status  
   s  novec  novec     100  0.00e+00    0.00164       0.00136  pass    

   d  novec  novec     100  0.00e+00    0.00139       0.00185  pass    

Note: ifst (33 & 33) or ilst (65 & 66) differ between (LAPACK++ & LAPACKE) due to LAPACKE issue.
   c  novec  novec     100  0.00e+00   4.70e-05      5.70e-05  pass    

Note: ifst (33 & 33) or ilst (65 & 66) differ between (LAPACK++ & LAPACKE) due to LAPACKE issue.
   z  novec  novec     100  0.00e+00   3.90e-05      6.20e-05  pass    
All tests passed for tgexc.

Checklist

  • [x] I've included a minimal example to reproduce the issue
  • [x] I'd be willing to make a PR to solve this issue

mgates3 avatar Dec 01 '22 21:12 mgates3

I don't know how important having ilst as output is. Do users depend on it?

It is only important in case of swap failures. To the extent of my knowledge, swap failures in the complex code are essentially impossible as of PR #420

thijssteel avatar Mar 14 '23 11:03 thijssteel

Daan Camps has a proof in his PhD thesis that the swap is backward stable (appendix B for those interested), but only seems to consider weak stability. It might be the strong stability that fails

thijssteel avatar Mar 14 '23 13:03 thijssteel