-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Check CHOLMOD version at runtime #10362
Conversation
ccall((:cholmod_version, :libcholmod), Cint, (Ptr{Cint},), version_array) | ||
ccall((:jl_cholmod_version, :libsuitesparse_wrapper), Cint, (Ptr{Cint},), tmp) | ||
if tmp != version_array | ||
throw(CHOLMODException("your version of CHOLMOD differ from the version used when Julia was build. These versions must match.")) |
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.
differs from the version used when Julia was built
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.
Thanks.
a8ca62d
to
297b698
Compare
Does throwing exceptions in |
Just checked and the result right now is Hej
Warning: error initializing module CHOLMOD:
Base.SparseMatrix.CHOLMOD.CHOLMODException(msg="your version of CHOLMOD differs from the version used when Julia was built. These versions must match.")
_
_ _ _(_)_ | A fresh approach to technical computing
(_) | (_) (_) | Documentation: http://docs.julialang.org
_ _ _| |_ __ _ | Type "help()" for help.
| | | | | | |/ _` | |
| | |_| | | | (_| | | Version 0.4.0-dev+3625 (2015-02-28 19:55 UTC)
_/ |\__'_|_|_|\__'_| | anj/cholmod/297b698* (fork: 1 commits, 0 days)
|__/ | x86_64-linux-gnu though I plan to remove the "Hej". So you can still use Julia even if you have an old or wrong version of CHOLMOD. |
297b698
to
7225a48
Compare
This error will likely be encountered by people using various distributions, so there is a high probability they will not know what I will also award a few bonus points for including the compiled version, and the current version. It's usually a struggle to make users find such information. eg:
|
It is a good idea to mention the version numbers in the error message. They are calculated anyway. However, real users shouldn't see this error. It is mainly meant as a help for packagers to make the right dependency requirements. Julia is broken and can segfault when the versions of CHOLMOD differ so we should really try to avoid mismatch. |
Real users will see this error message when the packagers mess up (which they do). |
I'm fine with adding to the error message that the sparse solvers are broken when the versions don't match, but I feel sad that the packaging model of Linux distributions makes it so likely that Julia will end up broken. I'd really prefer that we shipped our own copies of the dependencies. I have space enough left on my harddisk to over 5000 copies of SuiteSparse, so I'd really prefer a couple of extra copies to a potentially broken application. |
Encourage people to use the generic linux binaries instead of from their package manager then. You lose system-managed in-place updates that way, but you gain self-contained installation without needing root. For distribution packaging we need to either declare more explicit version requirements on dependencies (#6742 is part of that), or register sysimg rebuild hooks. The static compilation work might make this a little less painful if the Julia sysimg could be broken up into more independent pieces, so you wouldn't have to rebuild the whole thing. |
This shouldn't happen in standard distro packages. Normally, packagers are expected to warn maintainers of all packages that depend on their library when they update to a backward-incompatible version so that they can rebuild their package. So I'd say the problem comes from the fact that Julia moves very fast, and it's not easy for us to handle stable and development packages without sometimes breaking things. Maybe @staticfloat would better create a separate repo for the dependencies of stable releases, were things are more calm and users can expect more working packages. |
I agree with @ivarne that we should make the error as user-facing as possible. With all the moving parts we now have with the Julia builds, it's difficult to ensure everything works when problems are silent until the moment of critical failure. The first step toward ensuring that problems like this don't arise is preemptively checking for them, so this is a nice improvement. |
BTW, @staticfloat, it looks like the policy for Debian packages is to include the SOVERSION in their name, so normally you cannot install Julia with an unsupported dependency version. Sounds like a sane approach to me, that Fedora unfortunately does not follow AFAICT. |
This message should spell out what the consequences of this problem mean. Otherwise it'll confuse people. Is the exact version number what we actually want to be checking here, or should we specifically be checking as many features as we can? SuiteSparse can be configured in different ways by different packagers... |
7225a48
to
cefdb2c
Compare
The warnings is tremendously more useful now! Thanks! A few esthetic remarks. My terminal doesn't do a great job with breaking such long lines at word boundries, so I'd appreciate a few newlines at appropriate places. The begin
begin
begin
warn("""
CHOLMOD version incompatibility
Julia was compiled with a version of CHOLMOD that supported $(32length(IndexTypes))
bit integers, but is currently linked with version that supports $(8intsize) integers. This
might cause Julia to terminate when working with sparse matrices for operations involving
factorization of a matrix, e.g. solving systems of equations with \\.
This problem can be fixed by downloading the OS X or generic Linux binary from
www.julialang.org, which are shipped with the correct versions of all dependencies.
""")
end
end
end |
cefdb2c
to
74de89d
Compare
Check CHOLMOD version at runtime
@andreasnoack Somehow I missed that this commit added a warning even when only SuiteSparse's revision version ( This was raised in #12841, where a bugfix update of SuiteSparse in Fedora triggered the warning in Julia. Printing a scary warning all the time in a distro package isn't reasonable, and neither is updating Julia with every SuiteSparse update. I've never seen such a situation in established programs. |
@nalimilan Good point. The test could probably be loosened to only check for match on |
Ah, great to hear. That makes it much easier to follow SuiteSparse updates in distros. |
The check introduced in #10362 is actually too strict, as ABI break only happen with mismatches in the major version. Warning on minor version differences means distributions cannot update SuiteSparse without rebuiling Julia (#12841). Also fix the incorrect wording introuced in 25eb444: this message corresponds to the case where the version is different, not necessarily older.
@andreasnoack See #13000. |
The check introduced in #10362 is actually too strict, as ABI break only happen with mismatches in the major version. Warning on minor version differences means distributions cannot update SuiteSparse without rebuiling Julia (#12841). We also require at least libcholmod 2.1.1. Call dlsym_e instead of dlsym, which raises an error instead of returning C_NULL. Also fix the incorrect wording introuced in 25eb444: this message corresponds to the case where the version is different, not necessarily older.
This one should help to fix problem like #10342 where the version of CHOLMOD linked at runtime differs from the version used to create the shim suitesparse_wrapper. If the two versions don't match, an error is thrown.