Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing parameter in calls to XERBLA #5

Open
martin-frbg opened this issue Apr 27, 2019 · 7 comments
Open

Missing parameter in calls to XERBLA #5

martin-frbg opened this issue Apr 27, 2019 · 7 comments

Comments

@martin-frbg
Copy link

Most routines (with the obvious exception of the f2c-converted xtrsyl_rec.c files) are missing the final (string length) argument in their call to XERBLA.

@elmar-peise
Copy link
Collaborator

Would you mind checking if #8 solves this?

@martin-frbg
Copy link
Author

Looks good to me but I may not have time to run the lapack testsuite with a pure netlib+relapack build until next weekend.

@elmar-peise
Copy link
Collaborator

No problem. I'm only maintaining this on the side, but if you find small things to change/fix, please let me know. Also, of course any pull requests are welcome :-)

@martin-frbg
Copy link
Author

Well, LAPACK testsuite suggests there are additional problems with the XSYTRF_ALLOW_MALLOC option OpenMathLib/OpenBLAS#2098 and with the recursion levels in GBTRF OpenMathLib/OpenBLAS#2099 but at the moment I barely find time to keep OpenBLAS going.

@elmar-peise
Copy link
Collaborator

elmar-peise commented Apr 30, 2019

Regarding GBTRF: For small bandwidth kl that function translates into O(n/kl) deep tail recursion (instead of the desired O(log(n))):

const int n1 = MIN(DREC_SPLIT(*n), *kl);

Instead of disabling the BR recursion, we might want to crossover to unblocked for small kl instead

ReLAPACK/src/dgbtrf.c

Lines 85 to 89 in d24495f

if (*n <= MAX(CROSSOVER_DGBTRF, 1)) {
// Unblocked
LAPACK(dgbtf2)(m, n, kl, ku, Ab, ldAb, ipiv, info);
return;
}

Since kl is constant for the recursion, we could move that check to the non-recursive RELAPACK_dgbtrf above.

It would be even better if we could avoid the tail recursion and get back to O(log(n)), but I would need to get back to a whiteboard to figure out how/if that's possible.

@elmar-peise
Copy link
Collaborator

Never mind my last suggestion: Since we want to do pivoting on the whole diagonal, we need to do tail recursion. But I suppose we could express that in a loop to avoid stack overflows.

@martin-frbg
Copy link
Author

Sorry for turning this into a general discussion ticket, but given that it is actually related to the originai topic - Reference-LAPACK/lapack#339 (and linked gcc bug ticket) suggests there is a more widespread problem with (omitted) string length arguments in calls from C to Fortran. At first glance, this seems to also affect single-character arguments like the ubiquitous "U"/"L" or "N"/"T" with potentially bad effects on the stack in recursive calls.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants