-
Notifications
You must be signed in to change notification settings - Fork 25
add support for sparse-direct-mkl-pardiso solver #255
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
Conversation
|
I'm not a huge fan of adding this to SCS.jl. It seems like quite a heavy dependency. Can we use Requires similar to the GPU? |
|
I was thinking about it as well, but it pulls just two additional jlls: But yeah, probably you're right, I didn't take into account the downloaded libs, only loadtime ;) That's (@v1.7) pkg> add SCS
Installing known registries into `/tmp/julia_tmp`
Updating registry at `/tmp/julia_tmp/registries/General.toml`
Resolving package versions...
Installed Bzip2_jll ────────── v1.0.8+0
Installed Preferences ──────── v1.2.5
Installed SCS ──────────────── v1.1.1
Installed JSON ─────────────── v0.21.3
Installed CodecBzip2 ───────── v0.7.2
Installed Parsers ──────────── v2.2.4
Installed MutableArithmetics ─ v1.0.0
Installed BenchmarkTools ───── v1.3.1
Installed SCS_GPU_jll ──────── v3.2.0+0
Installed SCS_jll ──────────── v3.2.0+0
Installed OpenBLAS32_jll ───── v0.3.17+0
Installed CodecZlib ────────── v0.7.0
Installed Requires ─────────── v1.3.0
Installed OrderedCollections ─ v1.4.1
Installed TranscodingStreams ─ v0.9.6
Installed JLLWrappers ──────── v1.4.1
Installed MathOptInterface ─── v1.1.2
Downloaded artifact: Bzip2
Downloaded artifact: SCS_GPU
Downloaded artifact: OpenBLAS32
Downloaded artifact: SCS
Updating `/tmp/julia_tmp/environments/v1.7/Project.toml`
[c946c3f1] + SCS v1.1.1
Updating `/tmp/julia_tmp/environments/v1.7/Manifest.toml`
[6e4b80f9] + BenchmarkTools v1.3.1
[523fee87] + CodecBzip2 v0.7.2
[944b1d66] + CodecZlib v0.7.0
[692b3bcd] + JLLWrappers v1.4.1
[682c06a0] + JSON v0.21.3
[b8f27783] + MathOptInterface v1.1.2
[d8a4904e] + MutableArithmetics v1.0.0
[bac558e1] + OrderedCollections v1.4.1
[69de0a69] + Parsers v2.2.4
[21216c6a] + Preferences v1.2.5
[ae029012] + Requires v1.3.0
[c946c3f1] + SCS v1.1.1
[3bb67fe8] + TranscodingStreams v0.9.6
[6e34b625] + Bzip2_jll v1.0.8+0
[656ef2d0] + OpenBLAS32_jll v0.3.17+0
[af6e375f] + SCS_GPU_jll v3.2.0+0
[f4f2fc5b] + SCS_jll v3.2.0+0
[0dad84c5] + ArgTools
[56f22d72] + Artifacts
[2a0f44e3] + Base64
[ade2ca70] + Dates
[f43a241f] + Downloads
[b77e0a4c] + InteractiveUtils
[b27032c2] + LibCURL
[76f85450] + LibGit2
[8f399da3] + Libdl
[37e2e46d] + LinearAlgebra
[56ddb016] + Logging
[d6f4376e] + Markdown
[a63ad114] + Mmap
[ca575930] + NetworkOptions
[44cfe95a] + Pkg
[de0858da] + Printf
[9abbd945] + Profile
[3fa0cd96] + REPL
[9a3f8284] + Random
[ea8e919c] + SHA
[9e88b42a] + Serialization
[6462fe0b] + Sockets
[2f01184e] + SparseArrays
[10745b16] + Statistics
[fa267f1f] + TOML
[a4e569a6] + Tar
[8dfed614] + Test
[cf7118a7] + UUIDs
[4ec0a83e] + Unicode
[e66e0078] + CompilerSupportLibraries_jll
[deac9b47] + LibCURL_jll
[29816b5a] + LibSSH2_jll
[c8ffd9c3] + MbedTLS_jll
[14a3606d] + MozillaCACerts_jll
[4536629a] + OpenBLAS_jll
[83775a58] + Zlib_jll
[8e850b90] + libblastrampoline_jll
[8e850ede] + nghttp2_jll
[3f19e933] + p7zip_jll
Precompiling project...
23 dependencies successfully precompiled in 36 seconds
(@v1.7) pkg> add MKL_jll
Resolving package versions...
Installed MKL_jll ───────── v2022.0.0+0
Installed IntelOpenMP_jll ─ v2018.0.3+2
Downloaded artifact: IntelOpenMP
Updating `/tmp/julia_tmp/environments/v1.7/Project.toml`
[856f044c] + MKL_jll v2022.0.0+0
Updating `/tmp/julia_tmp/environments/v1.7/Manifest.toml`
[1d5cc7b8] + IntelOpenMP_jll v2018.0.3+2
[856f044c] + MKL_jll v2022.0.0+0
[4af54fe1] + LazyArtifacts
Precompiling project...
2 dependencies successfully precompiled in 1 seconds (23 already precompiled) |
Yes, this is what I meant by heavy. |
SCS_MKL_jll is build only for linux platforms, but MKL_jll is also available on Windows/Macos causing warnings on these platforms.
|
That's probably a problem with 32/64-bit interface on x86 linux: |
47d8588 to
757bb21
Compare
odow
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems okay. What are the performance differences?
| @@ -0,0 +1,61 @@ | |||
| struct MKLDirectSolver <: LinearSolver end | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs copyright header
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added;
out of curiosity: what is the rationale of adding those everywhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid people copy-pasting parts of the wrappers into other projects and ignoring the license implications 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but to fool copilot we'd need to scatter this notice in comments throughout the whole codebase 🤣
|
Numerically On a small problem I get
On a larger one the speed-up is comparable ( These might be atypical examples for showing off The original version (with large psd constraint and lots of sparse linear constraints) of the first (small) problem is:
so Tbh I hoped for something much better ;) But maybe those timings/findings are useful for @bodono as well. |
odow
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess one other thing: something should be added to the README?
Otherwise, squash+merge if you're happy.
this is a proof of concept, depends on JuliaPackaging/Yggdrasil#4773 but works locally ;)
Besides
MKLDirectSolverI've added runtimescs_versionfor each solver library;@odow technically it's a breaking change (there is argumentless version anymore), but it was just internal function that we didn't even test. So maybe we should ask do we actually need to query for version at runtime?