Using HSL_jll instead of custom compile #263
Conversation
|
locally confirmed that HSL works on windows |
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. @@ Coverage Diff @@
## master #263 +/- ##
==========================================
+ Coverage 75.41% 76.51% +1.10%
==========================================
Files 40 40
Lines 4128 3811 -317
==========================================
- Hits 3113 2916 -197
+ Misses 1015 895 -120
... and 29 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
That looks good to me, it's good to see that HSL_jll is simplifying code a lot. Before merging this, I think it would be nice to run a benchmark, just to check the compilation flags are set properly in HSL_jll and we are not introducing any performance regression. |
|
@frapac Thanks for the quick review! Regarding the potential performance regression, I'd say we don't need to worry too much as the user can custom compile HSL_jll if they want to compile with specific compilation flags. Then, they can link it with Julia using cc @amontoison |
lib/MadNLPHSL/src/ma86.jl
Outdated
| ma86_set_num_threads(opt.ma86_num_threads) | ||
|
|
||
| # Note: the current version of HSL_jll does not support openmp. | ||
| # this will be reenabled once openmp is supported in HSL_jll. |
There was a problem hiding this comment.
OpenMP is not supported?!
Did I do a mistake when I compiled HSL_jll.jl?
There was a problem hiding this comment.
@sshin23 I fixed the issue upstream.
I linked the shared library with -fopenmp but I didn't compiled with -fopenmp...
There was a problem hiding this comment.
Thanks for the quick action @amontoison! Indeed we used to have -fopenmp flag in our previous compilation script
MadNLP.jl/lib/MadNLPHSL/deps/build.jl
Line 118 in 29e95a5
If you want to check it before releasing it, I'd be happy to test it with MadNLPHSL
There was a problem hiding this comment.
What is your platform with libgfortran?
Base.BinaryPlatforms.host_triplet()
There was a problem hiding this comment.
x86_64-linux-gnu-libgfortran5-cxx11-libstdcxx30-julia_version+1.9.1
|
For information, the file |
|
@sshin23 |
lib/MadNLPHSL/src/MadNLPHSL.jl
Outdated
| import LinearAlgebra, OpenBLAS32_jll | ||
|
|
||
| function __init__() | ||
| if VERSION ≥ v"1.7" |
There was a problem hiding this comment.
It's if VERSION ≥ v"1.9"
I need to fix it in HSL.jl.
This PR updates MadNLPHSL.jl to use HSL_jll instead of the custom-compiled library