Move CUDA/CUDSS to weak deps and deduplicate GPU backend code - #596
Conversation
b982ec9 to
8d8f62c
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #596 +/- ##
==========================================
+ Coverage 84.16% 85.31% +1.14%
==========================================
Files 52 49 -3
Lines 4680 4311 -369
==========================================
- Hits 3939 3678 -261
+ Misses 741 633 -108 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
8d8f62c to
fc4a3ca
Compare
fc4a3ca to
7556ed7
Compare
|
Be aware of potential performance regression with CUDA that might have been introduced during generalization. If there's a benchmark I could run, let me know. |
21c0696 to
011184f
Compare
|
@sshin23 Can we merge this? The only addition to base MadNLP is |
sshin23
left a comment
There was a problem hiding this comment.
This is great. Thanks @michel2323 !
| CUDSS.cudss_set(solver, "deterministic_mode", 1) | ||
| end | ||
| if !isempty(opt.cudss_device_indices) | ||
| return if !isempty(opt.cudss_device_indices) |
There was a problem hiding this comment.
The return should be at the end of the function.
| # `buffer` contains the diagonal of the factorized matrix. | ||
|
|
||
| if M.opt.cudss_algorithm == MadNLP.CHOLESKY | ||
| return if M.opt.cudss_algorithm == MadNLP.CHOLESKY |
There was a problem hiding this comment.
why do we have a return here?
|
Please do a minor release of MadNLP.jl and then a breaking one for MadNLPGPU.jl after that you merged the PR. |
@sshin23, I think this is what you asked for in #476. It makes CUDA also just an extension. I would then rebase the oneAPI PR on this.