-
Notifications
You must be signed in to change notification settings - Fork 95
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
Fix version checking for SUNDIALS #2922
Fix version checking for SUNDIALS #2922
Conversation
It seems the variables added to BOUT-dev/cmake/SetupBOUTThirdParty.cmake Line 306 in ed69fd6
|
@@ -297,7 +297,11 @@ if (BOUT_USE_SUNDIALS) | |||
FetchContent_MakeAvailable(sundials) | |||
message(STATUS "SUNDIALS done configuring") | |||
else() | |||
find_package(SUNDIALS 4 REQUIRED) | |||
ENABLE_LANGUAGE(C) | |||
find_package(SUNDIALS REQUIRED) |
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 take it:
find_package(SUNDIALS REQUIRED) | |
find_package(SUNDIALS 4.0.0 REQUIRED) |
doesn't work?
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.
No, that only seems to work for 4.x.x. For a newer version, I get a warning like
CMake Warning at cmake/FindSUNDIALS.cmake:34 (find_package):
Could not find a configuration file for package "SUNDIALS" that is
compatible with requested version "4".
The following configuration files were considered but not accepted:
.../SUNDIALSConfig.cmake, version: 7.0.0
Call Stack (most recent call first):
cmake/SetupBOUTThirdParty.cmake:301 (find_package)
CMakeLists.txt:479 (include)
from here (with QUIET removed). This leads to a linking error later.
With cmake 3.19, I think find_package(SUNDIALS 4...7 REQUIRED)
might work.
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.
Oh yes, pretty sure 4...7
is the correct syntax
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.
Turns out that the 4...7
syntax doesn't work with
https://github.com/LLNL/sundials/blob/b685654c6958699821fc68a822b66076fec8cae7/CMakeLists.txt#L267
so the current manual check is probably the best option.
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.
Ah, yes, I see now, that's very unfortunate. SameMajorVersion
means "the requested version must have exactly the same major version", and makes no allowances for requesting "any of these major versions".
Agreed, the manual check is the best option.
That line is used to set the linking flags that |
Thanks @Steven-Roberts ! |
Numpy 2.0 is out, and we're getting some incompatible versions in CI which is causing the job failures |
find_package
required version SUNDIALS 4.x.x as opposed to >=4.0.0.