-
Notifications
You must be signed in to change notification settings - Fork 262
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
SPQR: Make GPUQREngine and SuiteSparse_GPURuntime subprojects #385
Conversation
This change could serve as a test-bed for how a root With the changes here, it should still be possible to build GPUQREngine as a stand-alone project (by configuring with the If we'd require that users need to use that potentially new root I haven't thought it through completely, but we'd probably need some additional logic if that use case should be supported. |
Rebased on a current head to resolve some merge conflicts. Following #408 (comment), we could also move SuiteSparse_GPURuntime to the SPQR folder. That could be a lower-impact first step on the way to absorbing those two libraries into SPQR_CUDA as proposed there. |
Yes, it would be a good idea to move GPUQREngine and SuiteSparse_GPURuntime into the SPQR folder as subprojects, just like SPQR_CUDA. The original intent was for SuiteSparse_GPURuntime to be a support library for any other SuiteSparse library that needed CUDA utilities, but it has turned out that I've needed it only for SPQR. In any case, if that changes in the future, I could go back to a stand alone library again. |
213e3cf
to
11914ff
Compare
I went ahead and moved SuiteSparse_GPURuntime to a subfolder of SPQR. I agree that it would still be possible to move SuiteSparse_GPURuntime back to a "proper" standalone library. But that should be done with some care to avoid including headers in other libraries that are private to this library. I believe that this PR should be ready to be merged now. However, I expect some merge conflicts with other open PRs (e.g. #411). It might be better to merge those PRs before this PR. |
Forgot to mention: This also fixes the part of #409 regarding those two libraries. |
c2b3684
to
c31c551
Compare
@DrTimothyAldenDavis: Could this PR be merged next? That would unblock the remaining changes for #409. |
da6dea4
into
DrTimothyAldenDavis:dev2
Done! I also released a minor fix: SuiteSparse v7.2.2, where I found a glitch when computing the # of OpenMP to use when assembling the last supernode in a superndal factorization. Likely no effect on performance, and no effect on accuracy of the result. I just might have gotten the wrong number of threads to use (but I always limit the # of threads to a sane number: 1 to the total # of threads available). |
This is an early test to check if it makes sense to make GPUQREngine a subproject of SPQR.
Feedback welcome.