Add ability to compile OpenBLAS for windows. #3922
Conversation
* moved OpenBLAS compilation logic to compile_OpenBLAS macro.
|
I'm not sure if compiling OpenBLAS is in-scope for mlpack. It's a separate problem, as OpenBLAS is an external library. Adding the compilation step as part of mlpack installation would set up an unreasonable expectation for handling an external project. This adds to the maintenance burden, where this burden is unnecessary. Precompiled OpenBLAS binaries can always be obtained from:
Armadillo also comes with precompiled OpenBLAS for Windoze: PS. If really necessary, I think a better solution would be to simply include precompiled OpenBLAS as part of mlpack, just like Armadillo. There would be no brittleness with compilation and all the associated maintenance mess. |
|
It's necessary for simplifying the cross-compilation process at least---hence the auto-downloader and auto-OpenBLAS-compiler (it's been there since I think 2021?). That also means that we can't include a precompiled version, since we don't know what architecture we need it for. I haven't had a chance to fully review this PR yet (should be able to tomorrow morning) but the changes looked good to me. |
|
Maybe we can simply link blas from armadillo then? And no compilation needed at all... On x64 at least. |
rcurtin
left a comment
rcurtin
left a comment
There was a problem hiding this comment.
Thanks for the contribution @RedInJector! Just a couple very small comments from my side. This is definitely something that we should add as it will ease at least the cross-compilation process on Windows as well as some other situations.
To @conradsnicta's comment, @shrit originally wrote fetch_mlpack() as a way to automatically configure mlpack for a cross-compilation environment, where you would need a specifically-compiled BLAS version, and where bundling it wouldn't work.
In general I would suggest the use of the system's package manager, if possible (maybe I should have said this in #3920, sorry about that), since that is more well-suited for getting the correct dependencies for the correct architecture and so forth. On Windows this would be vcpkg I guess (they generally keep the mlpack package up to date). I wonder if a good solution here to minimize confusion but still keep fetch_mlpack() as an option that can be used outside the cross-compilation context is to:
- add documentation to
mlpack.cmakesuggesting that if possible, the user should use 'standard' installation methods using the package manager; - see if there are precompiled versions we could fetch instead, when relevant. I know that OpenBLAS releases x86/x64 precompiled Windows DLLs: https://github.com/OpenMathLib/OpenBLAS/releases; prebuilt Linux libraries is a little harder for a number of reasons though.
removed unnecessary generator checks
rcurtin
left a comment
rcurtin
left a comment
There was a problem hiding this comment.
Thanks @RedInJector! Feel free to add your name to COPYRIGHT.txt and add a note to HISTORY.md (I'll update HISTORY.md before merge if you don't get to it). I am trying to debug the github actions that is supposed to post this automatically for first-time contributors, but if you want mlpack stickers to put on your laptop, send a mailing address to [email protected] and we'll get some in the mail for you.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Thanks again @RedInJector! |
3 participants
How to properly use fetch_mlpack from mlpack.cmake? #3920