Repository navigation
Resolve build error when scalapack isn't installed - #228
Conversation
|
I don't think this PR is correct. The default should be to look for |
I see, I'll change the configuration logic as you mentioned. Btw, since Slate's goal to replace Sealapack, why it needs sealapack to build? |
|
SLATE doesn't need ScaLAPACK, but it's useful for comparison. Some routines (like norms) have no residual check, so the only way to check results is to compare with another implementation. Ironically, we've found bugs in ScaLAPACK by doing this. |
mgates3
left a comment
There was a problem hiding this comment.
Looks like a great addition. Just a minor change and a question.
There was a problem hiding this comment.
Pull request overview
This PR updates the CMake configuration for the test targets to better detect ScaLAPACK (and fall back to disabling it) so builds don’t fail on systems where ScaLAPACK isn’t installed.
Changes:
- Add ScaLAPACK detection logic in
test/CMakeLists.txt, falling back toSCALAPACK_LIBRARIES="none"when not found. - Enable
CMAKE_EXPORT_COMPILE_COMMANDSglobally in the top-levelCMakeLists.txt.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| test/CMakeLists.txt | Adds ScaLAPACK discovery via find_package/find_library and disables ScaLAPACK when not present. |
| CMakeLists.txt | Forces compile database generation via CMAKE_EXPORT_COMPILE_COMMANDS. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Removed the setting for CMAKE_EXPORT_COMPILE_COMMANDS.
Set SCALAPACK_LIBRARIES = "none" if ScaLapack isn't installed before building this library to prevent compiling error.