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

[query] Fixed bug where hl.nd.qr didn't support 0 elements #10227

Merged
merged 3 commits into from
Mar 29, 2021

Conversation

johnc1231
Copy link
Contributor

I'm not sure this is the right fix. The segfault was coming from dgeqrf itself, and it was happening only when N is 0 (M being 0 was fine). The real issue is that when N is 0, LAPACK computes LWORK to be 0, meaning we don't actually allocate any memory for the WORK array and we segfault. To contrast, if I do a workspace size query for shape (0, 10), I get 320 for LWORK. This also seems wrong though, as you shouldn't really need any work space for an empty matrix?

Anyway, the segfault preventing fix was just to make sure LWORK was at least 1. I was hoping you (Patrick) could use Julia interface to play with the LAPACK functions and see what behavior you notice, and if it's consistent with the above.

@patrick-schultz
Copy link
Collaborator

LAPACK/MKL documentation says WORK must be an array of length max(1, LWORK). I think the "official" way to fix this is to leave LWORK as whatever the workspace query returns, and allocate the WORK arrray of size max(1, LWORK).

@patrick-schultz
Copy link
Collaborator

That's weird about the M=0 case. The LAPACK version (source here) returns LWORK=0 if either M or N is 0. But it doesn't seem worth complicating things over.

@johnc1231
Copy link
Contributor Author

Ah ok, I'll just change the allocation and leave LWORK alone.

@danking danking merged commit e0fff32 into hail-is:main Mar 29, 2021
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

Successfully merging this pull request may close these issues.

None yet

3 participants