DrTimothyAldenDavis / SuiteSparse

The official SuiteSparse library: a suite of sparse matrix algorithms authored or co-authored by Tim Davis, Texas A&M University.
https://people.engr.tamu.edu/davis/suitesparse.html
Other
1.14k stars 258 forks source link

SuiteSparse 7.6.0 #743

Closed DrTimothyAldenDavis closed 7 months ago

DrTimothyAldenDavis commented 7 months ago

various minor bug fixes:

remove `set ( package_USE_OPENMP OFF )` if `SUITESPARSE_USE_OPENMP` is set OFF.
resolve various compiler warnings
fix ABI compatibility for CHOLMOD
CI updates for additional testing
mmuetzel commented 7 months ago

Even if there aren't new features in that version, should it be called SuiteSparse 7.6.0 because we know there will be an ABI breaking change with respect to 7.5.1?

DrTimothyAldenDavis commented 7 months ago

I don't have a strong preference either way. @svillemot : what do you think about the version number for the ABI fix to CHOLMOD? CHOLMOD would go from 5.1.1 to 5.1.2 (an minor bug fix to repair the ABI problem), and then SuiteSparse itself would go from 7.5.1 to 7.5.2 or (as @mmuetzel suggests) to 7.6.0, to reflect the change to CHOLMOD.

And preference?

svillemot commented 7 months ago

I don’t have a preference for the version number of CHOLMOD and SuiteSparse. I guess it should be treated as any bugfix (unless you plan to introduce more changes in the next release).

DrTimothyAldenDavis commented 7 months ago

I'd prefer to use SuiteSparse 7.5.2 (where 7 is the main version and also the SO version), and CHOLMOD is 5.1.2 (5 being both the main version and SO version).

I view this as a serious bug fix so CHOLMOD 5.1.1 -> 5.1.2 seems fine to me, and thus SuiteSparse 7.5.1 -> 7.5.2 is my preference.

I realize the main version and SOVERSION don't have to match, but I would find that confusing to maintain. SuiteSparse has lots of packages, and I already have to keep track of 3 numbers x.y.z for each package. I don't want to add more numbers, so I prefer CHOLMOD 5.x.x to compile to libcholmod.so.5.

Does this sound OK?

If this is OK, I can release a stable SuiteSparse 7.5.2 today.

svillemot commented 7 months ago

Sounds good to me, thanks.

mmuetzel commented 7 months ago

If it is not too late, could you still consider #745 that would fix an issue for users which use a toolchain without OpenMP?

Independent of the SOVERSION, I was assuming that the major and minor version would indicate that libraries are ABI compatible. Like you wrote, the third version number would be used for non-ABI breaking bug fixes. This bug fix breaks the ABI (with respect to, e.g., SuiteSparse 7.5.1). So, would it be better to bump the minor version for it?

DrTimothyAldenDavis commented 7 months ago

I've revised the version numbers of CHOLMOD to 5.2.0, and SuiteSparse to 7.6.0. These changes propagate to CHOLMOD's dependencies (all packages in SuiteSparse that depend on CHOLMOD should look for CHOLMOD 5.2.0, not an earlier version, to make sure they all use a consistent ABI for CHOLMOD).

DrTimothyAldenDavis commented 7 months ago

If it is not too late, could you still consider #745 that would fix an issue for users which use a toolchain without OpenMP?

I've merged #745 for the OpenMP issue. @mmuetzel : Does that change resolve it, at least for SuiteSparse 7.6.0?

@svillemot : I think this PR will become the stable SuiteSparse 7.6.0. I will release a SuiteSparse 7.6.0.beta1 soon with these changes.