Fix xGESVDQ's jobu and jobv support in LAPACKE#1146
Fix xGESVDQ's jobu and jobv support in LAPACKE#1146yizeyi18 wants to merge 1 commit intoReference-LAPACK:masterfrom
xGESVDQ's jobu and jobv support in LAPACKE#1146Conversation
xGESVDQ's jobu and jobv support in LAPACKE
angsch
left a comment
There was a problem hiding this comment.
the missing job options are good catch
| lapack_int nrows_v = API_SUFFIX(LAPACKE_lsame)( jobv, 'a' ) ? n : | ||
| ( API_SUFFIX(LAPACKE_lsame)( jobv, 's' ) ? MIN(m,n) : 1); | ||
| ( (API_SUFFIX(LAPACKE_lsame)( jobu, 's' ) || | ||
| API_SUFFIX(LAPACKE_lsame)( jobu, 'u' ) ) ? MIN(m,n) : 1); |
There was a problem hiding this comment.
Shouldn't jobu = 'f' and jobu = 'r' be included here too?
There was a problem hiding this comment.
Yes. I got it hard to understand the shape of U with jobu = 'f' and jobu = 'r', so these was remained for later implementation. For jobu = 'r', how many cols should be included(as numrank is determined inside lapack), or using the maxium(MIN(m,n)) is OK?
| goto exit_level_0; | ||
| } | ||
| if( API_SUFFIX(LAPACKE_lsame)( jobu, 'a' ) || API_SUFFIX(LAPACKE_lsame)( jobu, 's' ) ) { | ||
| if( API_SUFFIX(LAPACKE_lsame)( jobu, 'a' ) || API_SUFFIX(LAPACKE_lsame)( jobu, 's' ) || |
There was a problem hiding this comment.
Since the jobu = 'a', 's', 'u', 'r', 'f' conditional is repeated several times, it may be worth to declare an ancillary variable to wrap the status. An analogous comment applies to jobv.
|
One way to move forward would be to merge this PR and then, once merged, add the suggestions of @angsch |
Description
LAPACKE provides interface to
xGESVDQ, although not included inDOCS/lapacke.pdf. In the implementation of the interface,jobuandjobvare treated as the same as inxGESVD, causing"U","R","F"variant ofjobuand"V","R"variant ofjobvnot handled.This PR appends support of these variant of
jobuandjobvin LAPACKE's interface toxGESVDQ.For not founding the way to edit
DOCS/lapacke.pdf, the documentation remains unchanged.Checklist